[Sol->Yul] Optimizing delete struct.

ethereum · Oct 12, 2020 · ec4945b · ec4945b
1 parent 1c142c5
commit ec4945b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 11 deletions.
diff --git a/docs/ir/ir-breaking-changes.rst b/docs/ir/ir-breaking-changes.rst
@@ -4,3 +4,34 @@ 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 deleting storage struct, if struct occupies less than one slot, whole slot will be cleared.
+ It is dangerous to use reminder of a slot for storing any other data as it is going to be set to zero after delete.
+
+::
+    // 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;
+            uint256 slotValue;
+            assembly {
+                slotValue := sload(s.slot)
+            }
+            // slotValue is always zero here, even though s occupies only half of the slot and the rest of the slot
+            // could be set via inline assembly to some other value
+        }
+    }
diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp
@@ -1156,19 +1156,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) {