Address PR review comments.

Author: rodiazet

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.
@@ -12,12 +15,11 @@ Compiler Features:
 * Metadata: Store the state of the experimental mode in JSON and CBOR metadata. In CBOR this broadens the meaning of the existing `experimental` field, which used to indicate only the presence of certain experimental pragmas in the source.
 * Standard JSON Interface: Introduce `settings.experimental` setting required for enabling the experimental mode.
 * Yul Optimizer: Improve performance of control flow side effects collector and function references resolver.
-* Yul Optimizer: Remove optimization that eliminated `returndatacopy` operations. This optimization was intentionally removed because it is very rarely used and complicates the implementation of `UnusedStoreEliminator`.
+* 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.
 * Yul EVM Code Transform: Improve stack shuffler performance by fixing a BFS deduplication issue.
-* Yul Optimizer: Fix `UnusedStoreEliminator` incorrectly removing `returndatacopy` operations when the length comes from a stale `returndatasize()` call that was invalidated by subsequent call opcodes.
 
 
 ### 0.8.34 (2026-02-18)

docs/bugs.json

@@ -2,12 +2,12 @@
     {
         "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. It has a special optimization for ``returndatacopy(0, 0, returndatasize())``, which copies the entire return data buffer to memory. This operation can be removed if the start offset is zero and the length equals `returndatasize()`, similar to other memory write operations. However, the check for this pattern did not account for `returndatasize()` values becoming stale. The size of the return data buffer is updated by ``CALL``, ``STATICCALL``, ``DELEGATECALL``, and ``CALLCODE`` opcodes. If a ``returndatasize()`` value is stored in a variable before such an operation and then used in a subsequent ``returndatacopy``, the stored size no longer reflects the actual return data buffer size. It should revert, if the stale return data size is greater than actual return data size. The optimizer would still consider it safe to remove, even though it may revert and should not be removed. This could lead to incorrect behavior when the code should revert but it does not. The bug only affects inline assembly or handwritten Yul code that uses optimizer sequences including the ``UnusedStoreEliminator`` step, which is a part of default optimizer sequence. The fix removes this optimisation as it is very rarely used.",
+        "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/X/Y/Z/",
         "introduced": "0.8.13",
         "fixed": "0.8.35",
-        "severity": "low",
+        "severity": "very low",
         "conditions": {
             "yulOptimizer": true
         }

libyul/optimiser/UnusedStoreEliminator.cpp

@@ -158,23 +158,28 @@ 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 isStorageOnlyWrite = (*instruction == Instruction::SSTORE);
+	bool isMemoryOnlyWrite =
 		*instruction == Instruction::EXTCODECOPY ||
 		*instruction == Instruction::CODECOPY ||
 		*instruction == Instruction::CALLDATACOPY ||
-		// 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 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 isCandidateForRemoval =
-		*instruction != Instruction::MCOPY &&
-		*instruction != Instruction::RETURNDATACOPY &&
+		!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)));
 	if (isCandidateForRemoval)
 	{
 		m_allStores.insert(&_statement);

test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize.yul

@@ -1,13 +1,4 @@
 // This test ensures that `returndatacopy` is NOT optimized away.
-// 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`.
-// ```
-// let x = returndatasize()
-// staticcall(...)
-// returndatacopy(0, 0, x) // Cannot be optimized away, because it can revert.
-// ```
 {
   returndatacopy(0,0,returndatasize())
 }

test/libyul/yulOptimizerTests/unusedStoreEliminator/returndatacopy_returndatasize_var.yul

@@ -1,13 +1,4 @@
 // This test ensures that `returndatacopy` is NOT optimized away.
-// 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`.
-// ```
-// let x = returndatasize()
-// staticcall(...)
-// returndatacopy(0, 0, x) // Cannot be optimized away, because it can revert.
-// ```
 {
   let s := returndatasize()
   returndatacopy(0,0,s)