From 6f7947cfa54fa740b5e012c0e7aa6c9889f33373 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Wed, 7 Oct 2020 19:23:08 +0200 Subject: [PATCH] [Sol->Yul] Optimizing delete struct. Co-authored-by: Daniel Kirchner --- docs/ir/ir-breaking-changes.rst | 29 +++++++++++++++ libsolidity/codegen/YulUtilFunctions.cpp | 34 ++++++++++++------ .../struct_delete_storage_nested_small.sol | 35 +++++++++++++++++++ .../structs/struct_delete_storage_small.sol | 24 +++++++++++++ ...truct_delete_storage_with_arrays_small.sol | 29 +++++++++++++++ 5 files changed, 140 insertions(+), 11 deletions(-) create mode 100644 test/libsolidity/semanticTests/structs/struct_delete_storage_nested_small.sol create mode 100644 test/libsolidity/semanticTests/structs/struct_delete_storage_small.sol create mode 100644 test/libsolidity/semanticTests/structs/struct_delete_storage_with_arrays_small.sol diff --git a/docs/ir/ir-breaking-changes.rst b/docs/ir/ir-breaking-changes.rst index c39c63e23..7ffa49508 100644 --- a/docs/ir/ir-breaking-changes.rst +++ b/docs/ir/ir-breaking-changes.rst @@ -4,3 +4,32 @@ Solidity IR-based Codegen Changes This section highlights the main differences between the old and the IR-based codegen, along with the reasoning behind the changes and how to update affected code. + +Semantic Only Changes +===================== + +This section lists the changes that are semantic-only, thus potentially +hiding new and different behavior in existing code. + + * When storage structs are deleted, every storage slot that contains a member of the struct is set to zero entirely. Formally, padding space was left untouched. +Consequently, if the padding space within a struct is used to store data (e.g. in the context of a contract upgrade), you have to be aware that ``delete`` will now also clear the added member (while it wouldn't have been cleared in the past). + +:: + // SPDX-License-Identifier: GPL-3.0 + pragma solidity >0.7.0; + + contract C { + struct S { + uint64 y; + uint64 z; + } + S s; + function f() public { + // ... + delete s; + // s occupies only first 16 bytes of the 32 bytes slot + // delete will write zero to the full slot + } + } + +We have the same behavior for implicit delete, for example when array of structs is shortened. diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index b9a8533f8..14c907f49 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1266,19 +1266,31 @@ string YulUtilFunctions::clearStorageStructFunction(StructType const& _type) MemberList::MemberMap structMembers = _type.nativeMembers(nullptr); vector> memberSetValues; + set slotsCleared; for (auto const& member: structMembers) - { - auto const& [memberSlotDiff, memberStorageOffset] = _type.storageOffsetsOfMember(member.name); + if (member.type->storageBytes() < 32) + { + auto const& slotDiff = _type.storageOffsetsOfMember(member.name).first; + if (!slotsCleared.count(slotDiff)) + { + memberSetValues.emplace_back().emplace("clearMember", "sstore(add(slot, " + slotDiff.str() + "), 0)"); + slotsCleared.emplace(slotDiff); + } + } + else + { + auto const& [memberSlotDiff, memberStorageOffset] = _type.storageOffsetsOfMember(member.name); + solAssert(memberStorageOffset == 0, ""); - memberSetValues.emplace_back().emplace("clearMember", Whiskers(R"( - (add(slot, ), ) - )") - ("setZero", storageSetToZeroFunction(*member.type)) - ("memberSlotDiff", memberSlotDiff.str()) - ("memberStorageOffset", to_string(memberStorageOffset)) - .render() - ); - } + memberSetValues.emplace_back().emplace("clearMember", Whiskers(R"( + (add(slot, ), ) + )") + ("setZero", storageSetToZeroFunction(*member.type)) + ("memberSlotDiff", memberSlotDiff.str()) + ("memberStorageOffset", to_string(memberStorageOffset)) + .render() + ); + } return Whiskers(R"( function (slot) { diff --git a/test/libsolidity/semanticTests/structs/struct_delete_storage_nested_small.sol b/test/libsolidity/semanticTests/structs/struct_delete_storage_nested_small.sol new file mode 100644 index 000000000..441c299c3 --- /dev/null +++ b/test/libsolidity/semanticTests/structs/struct_delete_storage_nested_small.sol @@ -0,0 +1,35 @@ +contract C { + struct S { + uint32 a; + S[] x; + } + S s; + function f() public returns (uint256 r1, uint256 r2, uint256 r3) { + assembly { + // 2 ** 150 - 1 + sstore(s.slot, 1427247692705959881058285969449495136382746623) + } + s.a = 1; + s.x.push(); s.x.push(); + S storage ptr1 = s.x[0]; + S storage ptr2 = s.x[1]; + assembly { + // 2 ** 150 - 1 + sstore(ptr1.slot, 1427247692705959881058285969449495136382746623) + sstore(ptr2.slot, 1427247692705959881058285969449495136382746623) + } + s.x[0].a = 2; s.x[1].a = 3; + delete s; + assert(s.a == 0); + assert(s.x.length == 0); + assembly { + r1 := sload(s.slot) + r2 := sload(ptr1.slot) + r3 := sload(ptr2.slot) + } + } +} +// ==== +// compileViaYul: true +// ---- +// f() -> 0, 0, 0 diff --git a/test/libsolidity/semanticTests/structs/struct_delete_storage_small.sol b/test/libsolidity/semanticTests/structs/struct_delete_storage_small.sol new file mode 100644 index 000000000..046e4ec6d --- /dev/null +++ b/test/libsolidity/semanticTests/structs/struct_delete_storage_small.sol @@ -0,0 +1,24 @@ +contract C { + struct S { + uint64 y; + uint64 z; + } + S s; + function f() public returns (uint256 ret) { + assembly { + // 2 ** 150 - 1 + sstore(s.slot, 1427247692705959881058285969449495136382746623) + } + s.y = 1; s.z = 2; + delete s; + assert(s.y == 0); + assert(s.z == 0); + assembly { + ret := sload(s.slot) + } + } +} +// ==== +// compileViaYul: true +// ---- +// f() -> 0 diff --git a/test/libsolidity/semanticTests/structs/struct_delete_storage_with_arrays_small.sol b/test/libsolidity/semanticTests/structs/struct_delete_storage_with_arrays_small.sol new file mode 100644 index 000000000..65fb74187 --- /dev/null +++ b/test/libsolidity/semanticTests/structs/struct_delete_storage_with_arrays_small.sol @@ -0,0 +1,29 @@ +contract C { + struct S { + uint32 a; + uint32[3] b; + uint32[] x; + } + S s; + function f() public returns (uint256 ret) { + assembly { + // 2 ** 150 - 1 + sstore(s.slot, 1427247692705959881058285969449495136382746623) + } + s.a = 1; + s.b[0] = 2; s.b[1] = 3; + s.x.push(4); s.x.push(5); + delete s; + assert(s.a == 0); + assert(s.b[0] == 0); + assert(s.b[1] == 0); + assert(s.x.length == 0); + assembly { + ret := sload(s.slot) + } + } +} +// ==== +// compileViaYul: true +// ---- +// f() -> 0