From 4a250a1fb0ec4dabe4e7d4a569fd20dcbfb8c1f6 Mon Sep 17 00:00:00 2001 From: rodiazet Date: Wed, 22 Apr 2026 17:42:35 +0200 Subject: [PATCH 1/5] Add `returndataload` elimination repro test. --- ...urndatacopy_usign_stale_returndatasize.sol | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 test/libsolidity/semanticTests/inlineAssembly/returndatacopy_usign_stale_returndatasize.sol diff --git a/test/libsolidity/semanticTests/inlineAssembly/returndatacopy_usign_stale_returndatasize.sol b/test/libsolidity/semanticTests/inlineAssembly/returndatacopy_usign_stale_returndatasize.sol new file mode 100644 index 000000000000..45e0fe4b76c8 --- /dev/null +++ b/test/libsolidity/semanticTests/inlineAssembly/returndatacopy_usign_stale_returndatasize.sol @@ -0,0 +1,30 @@ +contract C { + function e1() external returns(uint256) { + return 1; + } + + function e() external {} + + // This test verifies that UnusedStoreEliminator does not incorrectly eliminate + // a returndatacopy whose size argument is a stale snapshot of returndatasize() + // taken before a subsequent call that resets the return data buffer. + // + // The test only detects the bug when run with --optimize. + // + // The destination offset 96 is chosen so that the written region [96, 128) + // is provably disjoint from the ABI encoder's mload(64) at [64, 96). + function f() public { + assembly { + mstore(0x00, shl(0xe0, 0xa2c2d666)) // bytes4(keccak256("e1()")) + pop(staticcall(gas(), address(), 0x00, 0x04, 0x00, 0x00)) + let rds := returndatasize() // rds == 32 (return value of e1()) + mstore(0x00, shl(0xe0, 0xffae15ba)) // bytes4(keccak256("e()")) + pop(staticcall(gas(), address(), 0x00, 0x04, 0x00, 0x00)) + // After e(), returndatasize() == 0, but rds is stale (== 32). + // The out-of-bounds access reverts. + returndatacopy(96, 0x00, rds) + } + } +} +// ---- +// f() -> FAILURE From 45a5a295e92420843a1621165ed71a20c1034757 Mon Sep 17 00:00:00 2001 From: rodiazet Date: Fri, 17 Apr 2026 13:03:08 +0200 Subject: [PATCH 2/5] Add evmasm test --- .../args | 1 + .../input.sol | 12 +++++++ .../output | 34 +++++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/args create mode 100644 test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/input.sol create mode 100644 test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output diff --git a/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/args b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/args new file mode 100644 index 000000000000..2b5be19f4d79 --- /dev/null +++ b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/args @@ -0,0 +1 @@ +--ir-optimized --optimize --debug-info none --no-cbor-metadata diff --git a/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/input.sol b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/input.sol new file mode 100644 index 000000000000..2cae4b096490 --- /dev/null +++ b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/input.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity >=0.8.0; + +contract C { + function f() public pure { + assembly { + // Destination offset 96 is chosen to make the written memory region [96, 96+returndatasize()) + // provably disjoint from the ABI encoder's mload(64) (which reads [64, 96)). + returndatacopy(96, 0, returndatasize()) + } + } +} diff --git a/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output new file mode 100644 index 000000000000..4ae01e4f3a64 --- /dev/null +++ b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output @@ -0,0 +1,34 @@ +Optimized IR: +/// @use-src 0:"input.sol" +object "C_7" { + code { + { + let _1 := memoryguard(0x80) + mstore(64, _1) + if callvalue() { revert(0, 0) } + let _2 := datasize("C_7_deployed") + codecopy(_1, dataoffset("C_7_deployed"), _2) + return(_1, _2) + } + } + /// @use-src 0:"input.sol" + object "C_7_deployed" { + code { + { + mstore(64, 128) + if iszero(lt(calldatasize(), 4)) + { + if eq(0x26121ff0, shr(224, calldataload(0))) + { + if callvalue() { revert(0, 0) } + if slt(add(calldatasize(), not(3)), 0) { revert(0, 0) } + return(0, 0) + } + } + revert(0, 0) + } + } + data ".metadata" hex"" + } +} + From 0c9dcf19881755daf90efdac21a7a45f0cb9b3fe Mon Sep 17 00:00:00 2001 From: rodiazet Date: Fri, 17 Apr 2026 13:03:21 +0200 Subject: [PATCH 3/5] Apply fix --- libyul/optimiser/UnusedStoreEliminator.cpp | 48 ++++++++-------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 6f2f1ee938f8..aac554d74d54 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -158,48 +158,32 @@ void UnusedStoreEliminator::visit(Statement const& _statement) // both by querying a combination of semantic information and by listing the instructions. // This way the assert below should be triggered on any change. using evmasm::SemanticInformation; - bool isStorageWrite = (*instruction == Instruction::SSTORE); - bool isMemoryWrite = + bool const isStorageOnlyWrite = (*instruction == Instruction::SSTORE); + bool const isMemoryOnlyWrite = *instruction == Instruction::EXTCODECOPY || *instruction == Instruction::CODECOPY || *instruction == Instruction::CALLDATACOPY || - *instruction == Instruction::RETURNDATACOPY || - // TODO: Removing MCOPY is complicated because it's not just a store but also a load. - //*instruction == Instruction::MCOPY || *instruction == Instruction::MSTORE || - *instruction == Instruction::MSTORE8; - bool isCandidateForRemoval = - *instruction != Instruction::MCOPY && + *instruction == Instruction::MSTORE8 || + *instruction == Instruction::MCOPY || + *instruction == Instruction::RETURNDATACOPY; + bool const isBlacklisted = + // TODO: Removing MCOPY is complicated because it's not just a store but also a load. + *instruction == Instruction::MCOPY || + // The optimization that eliminated `returndatacopy` operations + // was intentionally removed because it is very rarely used and complicates the implementation + // of UnusedStoreEliminator. I.e. a variable which is initialized to `returndatasize()` becomes + // stale when a call changing the size is used in between the variable initialization and `returndatacopy`. + *instruction == Instruction::RETURNDATACOPY; + bool const isCandidateForRemoval = + !isBlacklisted && SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( SemanticInformation::storage(*instruction) == SemanticInformation::Write || (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) ); - yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); + yulAssert(isCandidateForRemoval == (isStorageOnlyWrite || (!m_ignoreMemory && isMemoryOnlyWrite && !isBlacklisted))); if (isCandidateForRemoval) { - if (*instruction == Instruction::RETURNDATACOPY) - { - // Out-of-bounds access to the returndata buffer results in a revert, - // so we are careful not to remove a potentially reverting call to a builtin. - // The only way the Solidity compiler uses `returndatacopy` is - // `returndatacopy(X, 0, returndatasize())`, so we only allow to remove this pattern - // (which is guaranteed to never cause an out-of-bounds revert). - bool allowReturndatacopyToBeRemoved = false; - auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); - auto length = identifierNameIfSSA(funCall->arguments.at(2)); - if (length && startOffset) - { - FunctionCall const* lengthCall = std::get_if(m_ssaValues.at(*length).value); - if ( - m_knowledgeBase.knownToBeZero(*startOffset) && - lengthCall && - toEVMInstruction(m_dialect, lengthCall->functionName) == Instruction::RETURNDATASIZE - ) - allowReturndatacopyToBeRemoved = true; - } - if (!allowReturndatacopyToBeRemoved) - return; - } m_allStores.insert(&_statement); std::vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); From 84a019dd0474c99505285f32e54d88b8f642bbc9 Mon Sep 17 00:00:00 2001 From: rodiazet Date: Fri, 17 Apr 2026 13:04:34 +0200 Subject: [PATCH 4/5] Update tests --- .../output | 2 +- .../unusedStoreEliminator/returndatacopy_fixed_size.yul | 1 + .../unusedStoreEliminator/returndatacopy_returndatasize.yul | 5 ++--- .../returndatacopy_returndatasize_nonzero_start.yul | 1 + .../returndatacopy_returndatasize_var.yul | 5 ++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output index 4ae01e4f3a64..d726034b4772 100644 --- a/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output +++ b/test/cmdlineTests/unusedStoreEliminator_returndatacopy_returndatasize/output @@ -22,6 +22,7 @@ object "C_7" { { if callvalue() { revert(0, 0) } if slt(add(calldatasize(), not(3)), 0) { revert(0, 0) } + returndatacopy(96, 0, returndatasize()) return(0, 0) } } @@ -31,4 +32,3 @@ object "C_7" { data ".metadata" hex"" } } - diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_fixed_size.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_fixed_size.yul index 50c05e8260bd..139d817b2a86 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_fixed_size.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_fixed_size.yul @@ -1,3 +1,4 @@ +// This optimisation step is removed and `returndatacopy` is never removed by `unusedStoreEliminator`. { returndatacopy(0,0,32) } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize.yul index e0d5cbf1c2d7..75d752a7e978 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize.yul @@ -1,3 +1,4 @@ +// This test ensures that `returndatacopy` is NOT optimized away. { returndatacopy(0,0,returndatasize()) } @@ -8,8 +9,6 @@ // // { // { -// let _1 := returndatasize() -// let _2 := 0 -// let _3 := 0 +// returndatacopy(0, 0, returndatasize()) // } // } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_nonzero_start.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_nonzero_start.yul index a40e76955b3f..ec827020f11e 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_nonzero_start.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_nonzero_start.yul @@ -1,3 +1,4 @@ +// This optimisation step is removed and `returndatacopy` is never removed by `unusedStoreEliminator`. { returndatacopy(0,1,returndatasize()) } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_var.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_var.yul index eac2070f7411..58498311b72c 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_var.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_var.yul @@ -1,3 +1,4 @@ +// This test ensures that `returndatacopy` is NOT optimized away. { let s := returndatasize() returndatacopy(0,0,s) @@ -9,8 +10,6 @@ // // { // { -// let s := returndatasize() -// let _1 := 0 -// let _2 := 0 +// returndatacopy(0, 0, returndatasize()) // } // } From 257d0fc4a49c812ff21196911563556210231a65 Mon Sep 17 00:00:00 2001 From: rodiazet Date: Fri, 17 Apr 2026 13:04:53 +0200 Subject: [PATCH 5/5] Add changelog and bug list entry. --- Changelog.md | 4 ++++ docs/bugs.json | 14 ++++++++++++++ docs/bugs_by_version.json | 25 ++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 5c1420ca346c..8cefd7508f42 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,9 @@ Language Features: * General: Add a builtin that computes the base slot of a storage namespace using the `erc7201` formula from ERC-7201. +Important Bugfixes: +* Yul Optimizer: Fix `UnusedStoreEliminator` incorrectly removing `returndatacopy` operations when the length comes from a stale `returndatasize()` call that was invalidated by subsequent call opcodes. + Compiler Features: * Commandline Interface: Disallow selecting the deprecated assembly input mode that was only accessible via `--assemble` instead of treating it as equivalent to `--strict-assembly`. * Commandline Interface: Introduce `--experimental` flag required for enabling the experimental mode. @@ -15,6 +18,7 @@ Compiler Features: * Standard JSON Interface: Introduce `settings.experimental` setting required for enabling the experimental mode. * Standard JSON Interface: Replace the top-level ``ethdebug`` output with ``ethdebug.resources`` and ``ethdebug.compilation``. Decouple ethdebug outputs from binary compilation so that global ethdebug outputs can be produced without generating bytecode. * Yul Optimizer: Improve performance of control flow side effects collector and function references resolver. +* Yul Optimizer: Remove optimization that eliminated `returndatacopy` operations. Bugfixes: * Yul: Fix incorrect serialization of Yul object names containing double quotes and escape sequences, producing output that could not be parsed as valid Yul. diff --git a/docs/bugs.json b/docs/bugs.json index 5f8d1a162732..3c61e26381e6 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,18 @@ [ + { + "uid": "SOL-2026-2", + "name": "UnusedStoreEliminatorStaleReturnDataSize", + "summary": "The Yul optimizer's ``UnusedStoreEliminator`` may incorrectly remove ``returndatacopy(...)`` operations when using a stale value from ``returndatasize()`` that was invalidated by subsequent call operations.", + "description": "The ``UnusedStoreEliminator`` is a Yul optimizer step that removes redundant memory and storage writes. One of the operations eligible for removal is ``returndatacopy(...)``. This particular operation has a quirk - unlike any other instruction for bulk memory copying it reverts on out-of-bounds access. A revert is one of the side-effects that the optimizer guarantees to preserve so the operation can only be removed when it is certain that it cannot revert. This is the case when the entire return data buffer is copied to memory, i.e. when the start offset is zero and the length equals ``returndatasize()``. The optimizer was special-cased to detect and optimize only this specific pattern, since it matches the code produced by the code generator for external calls. However, the check did not account for the possibility of ``returndatasize()`` values becoming stale. The size of the return data buffer is updated by ``call()``, ``staticcall()``, ``delegatecall()``, and ``callcode()``. If a ``returndatasize()`` value is stored in a variable before such an operation and then used in a subsequent ``returndatacopy(...)``, the stored size may no longer reflect the actual return data buffer size. Despite this, the optimizer would consider it safe to remove, bypassing the revert and allowing the code to continue, possibly leading to unexpected behavior. Since the code generator never produces code that interleaves multiple calls and access to their return data, the bug only affected inline assembly or handwritten Yul code. The necessary condition is the use of an optimizer sequence including the ``UnusedStoreEliminator`` step (which is the default).", + "link": "https://blog.soliditylang.org/2026/04/21/unusedstore-eliminator-stale-returndatasize-bug/", + "introduced": "0.8.13", + "fixed": "0.8.35", + "severity": "very low", + "conditions": { + "yulOptimizer": true, + "evmVersion": ">=byzantium" + } + }, { "uid": "SOL-2026-1", "name": "TransientStorageClearingHelperCollision", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index c5576d4b298e..f0cca78f37b2 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1869,6 +1869,7 @@ }, "0.8.13": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1884,6 +1885,7 @@ }, "0.8.14": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1897,6 +1899,7 @@ }, "0.8.15": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1908,6 +1911,7 @@ }, "0.8.16": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1918,6 +1922,7 @@ }, "0.8.17": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1927,6 +1932,7 @@ }, "0.8.18": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1936,6 +1942,7 @@ }, "0.8.19": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1960,6 +1967,7 @@ }, "0.8.20": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication", "FullInlinerNonExpressionSplitArgumentEvaluationOrder", @@ -1969,6 +1977,7 @@ }, "0.8.21": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication" ], @@ -1976,6 +1985,7 @@ }, "0.8.22": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow", "VerbatimInvalidDeduplication" ], @@ -1983,36 +1993,42 @@ }, "0.8.23": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow" ], "released": "2023-11-08" }, "0.8.24": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow" ], "released": "2024-01-25" }, "0.8.25": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow" ], "released": "2024-03-14" }, "0.8.26": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow" ], "released": "2024-05-21" }, "0.8.27": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "LostStorageArrayWriteOnSlotOverflow" ], "released": "2024-09-04" }, "0.8.28": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision", "LostStorageArrayWriteOnSlotOverflow" ], @@ -2020,6 +2036,7 @@ }, "0.8.29": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision", "LostStorageArrayWriteOnSlotOverflow" ], @@ -2041,6 +2058,7 @@ }, "0.8.30": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision", "LostStorageArrayWriteOnSlotOverflow" ], @@ -2048,6 +2066,7 @@ }, "0.8.31": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision", "LostStorageArrayWriteOnSlotOverflow" ], @@ -2055,18 +2074,22 @@ }, "0.8.32": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision" ], "released": "2025-12-18" }, "0.8.33": { "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize", "TransientStorageClearingHelperCollision" ], "released": "2025-12-18" }, "0.8.34": { - "bugs": [], + "bugs": [ + "UnusedStoreEliminatorStaleReturnDataSize" + ], "released": "2026-02-18" }, "0.8.4": {