Merge pull request #13100 from ethereum/fixUnusedStoreInlineAssembly

Fix unused store inline assembly

Author: ekpyron

Changelog.md

@@ -1,5 +1,9 @@
 ### 0.8.15 (unreleased)
 
+Important Bugfixes:
+ * Yul Optimizer: Keep all memory side-effects of inline assembly blocks.
+
+
 Language Features:
  * Add `E.selector` for a non-anonymous event `E` to access the 32-byte selector topic.
  * Errors and Events allow qualified access from other contracts.

docs/bugs.json

@@ -1,4 +1,17 @@
 [
+    {
+        "uid": "SOL-2022-4",
+        "name": "InlineAssemblyMemorySideEffects",
+        "summary": "The Yul optimizer may incorrectly remove memory writes from inline assembly blocks, that do not access solidity variables.",
+        "description": "The Yul optimizer considers all memory writes in the outermost Yul block that are never read from as unused and removes them. This is valid when that Yul block is the entire Yul program, which is always the case for the Yul code generated by the new via-IR pipeline. Inline assembly blocks are never optimized in isolation when using that pipeline. Instead they are optimized as a part of the whole Yul input. However, the legacy code generation pipeline (which is still the default) runs the Yul optimizer individually on an inline assembly block if the block does not refer to any local variables defined in the surrounding Solidity code. Consequently, memory writes in such inline assembly blocks are removed as well, if the written memory is never read from in the same assembly block, even if the written memory is accessed later, for example by a subsequent inline assembly block.",
+        "link": "https://blog.soliditylang.org/2022/06/15/inline-assembly-memory-side-effects-bug/",
+        "introduced": "0.8.13",
+        "fixed": "0.8.15",
+        "severity": "low/medium",
+        "conditions": {
+            "yulOptimizer": true
+        }
+    },
     {
         "uid": "SOL-2022-3",
         "name": "DataLocationChangeInInternalOverride",
@@ -8,7 +21,6 @@
         "introduced": "0.6.9",
         "fixed": "0.8.14",
         "severity": "very low"
-
     },
     {
         "uid": "SOL-2022-2",

docs/bugs_by_version.json

@@ -1614,13 +1614,16 @@
     },
     "0.8.13": {
         "bugs": [
+            "InlineAssemblyMemorySideEffects",
             "DataLocationChangeInInternalOverride",
             "NestedCallataArrayAbiReencodingSizeValidation"
         ],
         "released": "2022-03-16"
     },
     "0.8.14": {
-        "bugs": [],
+        "bugs": [
+            "InlineAssemblyMemorySideEffects"
+        ],
         "released": "2022-05-17"
     },
     "0.8.2": {

libsolidity/codegen/CompilerContext.cpp

@@ -485,6 +485,7 @@ void CompilerContext::appendInlineAssembly(
 		obj.code = parserResult;
 		obj.analysisInfo = make_shared<yul::AsmAnalysisInfo>(analysisInfo);
 
+		solAssert(!dialect.providesObjectAccess());
 		optimizeYul(obj, dialect, _optimiserSettings, externallyUsedIdentifiers);
 
 		if (_system)

libyul/optimiser/UnusedStoreEliminator.cpp

@@ -31,6 +31,8 @@
 #include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/AST.h>
 
+#include <libyul/backends/evm/EVMDialect.h>
+
 #include <libsolutil/CommonData.h>
 
 #include <libevmasm/Instruction.h>
@@ -76,7 +78,13 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast)
 		ignoreMemory
 	};
 	rse(_ast);
-	rse.changeUndecidedTo(State::Unused, Location::Memory);
+	if (
+		auto evmDialect = dynamic_cast<EVMDialect const*>(&_context.dialect);
+		evmDialect && evmDialect->providesObjectAccess()
+	)
+		rse.changeUndecidedTo(State::Unused, Location::Memory);
+	else
+		rse.changeUndecidedTo(State::Used, Location::Memory);
 	rse.changeUndecidedTo(State::Used, Location::Storage);
 	rse.scheduleUnusedForDeletion();
 

test/libsolidity/semanticTests/array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol

@@ -17,5 +17,5 @@ contract c {
 // test() ->
 // gas irOptimized: 142639
 // gas legacy: 164430
-// gas legacyOptimized: 157898
+// gas legacyOptimized: 158513
 // storageEmpty -> 1

test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol

@@ -0,0 +1,29 @@
+contract C {
+	function f() external returns (uint256 x) {
+		assembly {
+			mstore(0, 0x42)
+		}
+		assembly {
+			x := mload(0)
+		}
+	}
+	function g() external returns (bool) {
+		uint initialFreeMemoryPointer;
+		assembly {
+			initialFreeMemoryPointer := mload(0x40)
+		}
+		assembly {
+			let ptr := mload(0x40)
+			mstore(0x40, add(ptr, 0x20))
+		}
+		uint finalFreeMemoryPointer;
+		assembly {
+			finalFreeMemoryPointer := mload(0x40)
+		}
+		assert(initialFreeMemoryPointer != finalFreeMemoryPointer);
+		return true;
+	}
+}
+// ----
+// f() -> 0x42
+// g() -> true

test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block_bugreport.sol

@@ -0,0 +1,23 @@
+contract Test {
+	uint256 x;
+
+	function test() public returns (uint256) {
+		uint256 a = myGetX();
+		x = 5;
+		uint256 b = myGetX();
+		assembly {
+			log0(0, 64)
+		}
+		return a + b + myGetX();
+	}
+
+	function myGetX() internal view returns (uint256) {
+		assembly {
+			mstore(1, 0x123456789abcdef)
+		}
+		return x;
+	}
+}
+// ----
+// test() -> 10
+// ~ emit <anonymous>: 0x0123456789abcd, 0xef00000000000000000000000000000000000000000000000000000000000000