Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sol->Yul] Optimizing delete struct. #9985

Merged
merged 1 commit into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions docs/ir/ir-breaking-changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rather say "that old code generated for delete before adding that struct" or something like that, but I'd also say it's fine for now - we'll need to refine this in the end when we make it "public" anyways I expect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you merged already anyways :-D.


::
// 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.
34 changes: 23 additions & 11 deletions libsolidity/codegen/YulUtilFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1266,19 +1266,31 @@ string YulUtilFunctions::clearStorageStructFunction(StructType const& _type)
MemberList::MemberMap structMembers = _type.nativeMembers(nullptr);
vector<map<string, string>> memberSetValues;

set<u256> 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"(
<setZero>(add(slot, <memberSlotDiff>), <memberStorageOffset>)
)")
("setZero", storageSetToZeroFunction(*member.type))
("memberSlotDiff", memberSlotDiff.str())
("memberStorageOffset", to_string(memberStorageOffset))
.render()
);
}
memberSetValues.emplace_back().emplace("clearMember", Whiskers(R"(
<setZero>(add(slot, <memberSlotDiff>), <memberStorageOffset>)
)")
("setZero", storageSetToZeroFunction(*member.type))
("memberSlotDiff", memberSlotDiff.str())
("memberStorageOffset", to_string(memberStorageOffset))
.render()
);
}

return Whiskers(R"(
function <functionName>(slot) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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