From 14cdb76e4b16376484dd33ce9a050925c663dabc Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 7 Jun 2022 12:42:50 +0200 Subject: [PATCH 1/4] Add failing test --- .../optimize_memory_store_multi_block.sol | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol diff --git a/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol b/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol new file mode 100644 index 000000000..023d178a9 --- /dev/null +++ b/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.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 : 0x0123456789abcd, 0xef00000000000000000000000000000000000000000000000000000000000000 From 51ef6a62dadd31733a8401fa32919fb4cff2666d Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 7 Jun 2022 12:30:13 +0200 Subject: [PATCH 2/4] Fix removal of memory stores in inline assembly blocks. --- Changelog.md | 4 ++++ libsolidity/codegen/CompilerContext.cpp | 1 + libyul/optimiser/UnusedStoreEliminator.cpp | 10 +++++++++- .../byte_array_pop_long_storage_empty_garbage_ref.sol | 2 +- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 4151d533a..2a3f34bbd 100644 --- a/Changelog.md +++ b/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. diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 55089dde1..dbd1a1730 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -485,6 +485,7 @@ void CompilerContext::appendInlineAssembly( obj.code = parserResult; obj.analysisInfo = make_shared(analysisInfo); + solAssert(!dialect.providesObjectAccess()); optimizeYul(obj, dialect, _optimiserSettings, externallyUsedIdentifiers); if (_system) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 976c8c842..0e6acdeea 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -31,6 +31,8 @@ #include #include +#include + #include #include @@ -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(&_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(); diff --git a/test/libsolidity/semanticTests/array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol b/test/libsolidity/semanticTests/array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol index bef07f55b..66fb16775 100644 --- a/test/libsolidity/semanticTests/array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol +++ b/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 From 94dd6d0671cc32b3f2b545ec80bfcf84190aec34 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 10 Jun 2022 18:31:49 +0200 Subject: [PATCH 3/4] Bug list entry draft. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- docs/bugs.json | 14 +++++++++++++- docs/bugs_by_version.json | 5 ++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/docs/bugs.json b/docs/bugs.json index 7bee4d10f..30b1769fd 100644 --- a/docs/bugs.json +++ b/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", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 86203a72b..1b0f7d1ca 100644 --- a/docs/bugs_by_version.json +++ b/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": { From aa7e4e02bbfa5b8a0798f31ae59381cb123c366b Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 10 Jun 2022 20:12:09 +0200 Subject: [PATCH 4/4] A few more tests. --- .../optimize_memory_store_multi_block.sol | 36 +++++++++++-------- ...ize_memory_store_multi_block_bugreport.sol | 23 ++++++++++++ 2 files changed, 44 insertions(+), 15 deletions(-) create mode 100644 test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block_bugreport.sol diff --git a/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol b/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol index 023d178a9..1a72925d6 100644 --- a/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol +++ b/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block.sol @@ -1,23 +1,29 @@ -contract Test { - uint256 x; - - function test() public returns (uint256) { - uint256 a = myGetX(); - x = 5; - uint256 b = myGetX(); +contract C { + function f() external returns (uint256 x) { assembly { - log0(0, 64) + mstore(0, 0x42) + } + assembly { + x := mload(0) } - return a + b + myGetX(); } - - function myGetX() internal view returns (uint256) { + function g() external returns (bool) { + uint initialFreeMemoryPointer; assembly { - mstore(1, 0x123456789abcdef) + initialFreeMemoryPointer := mload(0x40) } - return x; + assembly { + let ptr := mload(0x40) + mstore(0x40, add(ptr, 0x20)) + } + uint finalFreeMemoryPointer; + assembly { + finalFreeMemoryPointer := mload(0x40) + } + assert(initialFreeMemoryPointer != finalFreeMemoryPointer); + return true; } } // ---- -// test() -> 10 -// ~ emit : 0x0123456789abcd, 0xef00000000000000000000000000000000000000000000000000000000000000 +// f() -> 0x42 +// g() -> true diff --git a/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block_bugreport.sol b/test/libsolidity/semanticTests/inlineAssembly/optimize_memory_store_multi_block_bugreport.sol new file mode 100644 index 000000000..023d178a9 --- /dev/null +++ b/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 : 0x0123456789abcd, 0xef00000000000000000000000000000000000000000000000000000000000000