Skip to content

Remove RETURNDATACOPY elimination from UnusedStoreEliminator#16508

Open
rodiazet wants to merge 6 commits intodevelopfrom
remove-returndatacopy-elimination
Open

Remove RETURNDATACOPY elimination from UnusedStoreEliminator#16508
rodiazet wants to merge 6 commits intodevelopfrom
remove-returndatacopy-elimination

Conversation

@rodiazet
Copy link
Copy Markdown
Contributor

@rodiazet rodiazet commented Mar 9, 2026

As discussed here we decided to remove this optimisation as it has a minimal impact on generated code and make the implementation of the UnusedStoreEliminator complicated and bug prone. In this case pattern returndatacopy(0, 0, returndatasize()) will never be eliminated.

@rodiazet rodiazet requested review from cameel and nikola-matic and removed request for cameel March 9, 2026 16:11
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep these tests, but add a comment that we expect this not to be optimized.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extended comment added.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like removing this did not affect gas in any of our semantic tests?

Please check gas benchmarks from external tests though.

Copy link
Copy Markdown
Contributor Author

@rodiazet rodiazet Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relative

ir-no-optimize

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0%
euler
gnosis
gp2 0%
pool-together 0%
uniswap 0% +0% +0%
yield_liquidator 0% +0% +0%
zeppelin

ir-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
euler 0% -0% +0%
gnosis
gp2 0%
pool-together 0%
uniswap 0% +0% -0%
yield_liquidator 0% -0% 0%
zeppelin 0% -0% +0%

ir-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
euler
gnosis
gp2 0%
pool-together 0%
uniswap 0% 0% +0%
yield_liquidator 0% 0% 0%
zeppelin 0%

legacy-no-optimize

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0%
euler 0% +0% -0%
gnosis 0%
gp2 0%
pool-together 0%
uniswap 0% +0% +0%
yield_liquidator 0% -0% 0%
zeppelin

legacy-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
euler 0% -0% -0%
gnosis 0%
gp2 0%
pool-together 0%
uniswap 0% +0% -0%
yield_liquidator 0% -0% 0%
zeppelin 0% +0% +0%

legacy-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
euler 0% +0% -0%
gnosis 0%
gp2 0%
pool-together 0%
uniswap 0% -0% +0%
yield_liquidator 0% 0% 0%
zeppelin

!V = version mismatch
!B = no value in the "before" version
!A = no value in the "after" version
!T = one or both values were not numeric and could not be compared
-0 = very small negative value rounded to zero
+0 = very small positive value rounded to zero

absolute

ir-no-optimize

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0
euler
gnosis
gp2 0
pool-together 0
uniswap 0 36 82
yield_liquidator 0 12 72
zeppelin

ir-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0 0 0
euler 0 -24 5972
gnosis
gp2 0
pool-together 0
uniswap 0 48 -103
yield_liquidator 0 -36 0
zeppelin 0 -24 121

ir-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0 0 0
euler
gnosis
gp2 0
pool-together 0
uniswap 0 0 3955
yield_liquidator 0 0 0
zeppelin 0

legacy-no-optimize

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0
euler 0 12 -12110
gnosis 0
gp2 0
pool-together 0
uniswap 0 36 380
yield_liquidator 0 -12 0
zeppelin

legacy-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0 0 0
euler 0 -48 -18878
gnosis 0
gp2 0
pool-together 0
uniswap 0 48 -1614
yield_liquidator 0 -12 0
zeppelin 0 100 1426

legacy-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0
colony 0
elementfi 0
ens 0 0 0
euler 0 24 -10010
gnosis 0
gp2 0
pool-together 0
uniswap 0 -12 882
yield_liquidator 0 0 0
zeppelin

!V = version mismatch
!B = no value in the "before" version
!A = no value in the "after" version
!T = one or both values were not numeric and could not be compared
-0 = very small negative value rounded to zero
+0 = very small positive value rounded to zero

@rodiazet rodiazet force-pushed the remove-returndatacopy-elimination branch 2 times, most recently from 761080c to a3869e1 Compare March 10, 2026 18:37
Copy link
Copy Markdown
Member

@clonker clonker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me aside from the introduced error!

docs/bugs.json Outdated
"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 then 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 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.",
"link": "https://blog.soliditylang.org/2026/X/Y/Z/",
"introduced": "0.8.12",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"introduced": "0.8.12",
"introduced": "0.8.13",

It couldn't have been 0.8.12, there was no UnusedStoreEliminator. I think it's 0.8.13 but better double check :) this also changes the bugs_by_version.json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it was introduced with the USE 0.8.13 and only partially fixed in 0.8.15(https://github.com/argotorg/solidity/blame/v0.8.15/libyul/optimiser/UnusedStoreEliminator.cpp#L169-L185). I will leave 0.8.13 as you suggested

@rodiazet rodiazet force-pushed the remove-returndatacopy-elimination branch from a3869e1 to f817e10 Compare March 26, 2026 13:14
@rodiazet rodiazet requested review from cameel and clonker March 26, 2026 13:14
Copy link
Copy Markdown
Member

@clonker clonker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the nits

docs/bugs.json Outdated
"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 then 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 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.",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"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 then 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 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.",
"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.",

then -> than
code code -> code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to be sorry about. These are very annoying when reading the bug description.

@rodiazet rodiazet force-pushed the remove-returndatacopy-elimination branch from f817e10 to aca08f7 Compare March 27, 2026 11:31
@rodiazet rodiazet requested a review from clonker March 27, 2026 11:31
Copy link
Copy Markdown
Collaborator

@cameel cameel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main blocker for this is that it's missing regression tests for the bug. There are also some things to clarify about the buglist entry.

Changelog.md Outdated
* 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`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't normally don't include rationale to keep the changelog concise, so I'd remove this bit:

Suggested change
* 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.

You should put such explanations in UnusedStoreEliminator.cpp instead - currently there's nothing there saying why RETURNDATACOPY is treated specially. In fact, the extensive explanations you have in tests could be moved there too. In the tests themselves you can leave shorter remarks. This way you also don't have to repeat it in full in every test.

Comment on lines 165 to 169
*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;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meaning of this condition has diverged from the name. It's no longer just a list of instructions that write memory. I think we might be better off creating a variable like isBlacklisted and putting RETURNDATACOPY and MCOPY there. They're still memory writes so it would better reflect what we're going at here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, isMemoryOnlyWrite/isStorageOnlyWrite would be better names for these. The point is not only that these write memory but also that they are simple instructions that do not have other side effects.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we have two more tests for returndatacopy in the same dir. They were not updated, because they check that cases without returndatasize() are not modified, but they should have some comment in them too.

docs/bugs.json Outdated
{
"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.",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make up your mind on whether to add () after function names :)

Suggested change
"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.",
"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.",

Same problem in the changelog entries.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're also mixing single and double backticks in the description field below.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR should include a test case that would reproduce the bug before the fix to prove it's really fixed and to serve as a regression test. I'd also include the tricky case with a look that we discussed in #16436.

Comment on lines +11 to +13
"conditions": {
"yulOptimizer": true
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about viaIR? I think this should be possible to trigger via evmasm too through inline assembly. But then we should have a test case that proves this. Otherwise this needs viaIR: false.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also needs >=byzantium for evmVersion. RETURNDATACOPY did not exist in earlier EVM versions.

docs/bugs.json Outdated
"link": "https://blog.soliditylang.org/2026/X/Y/Z/",
"introduced": "0.8.13",
"fixed": "0.8.35",
"severity": "low",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we already decide on severity for this?

I would not give it more than very low, but we also need to agree on that as a team.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that there is something below low severity. Change it to very low. Can we just agreed here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity levels are listed in the docs: List of Known Bugs

To agree on one we normally discuss it on a call or on the security channel.

docs/bugs.json Outdated
"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.",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?

Suggested change
"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.",
"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).",
  • It was missing context for why a revert is expected here in the first place.
  • Use past tense when referring to the bug (the entry is published when the bug is already fixed) and present when referring to things that are still true after the fix.
  • This is a bug description, we don't normally mention the bugfix in these. The fact that the optimization was rarely applicable is notable, but the blog post is a better place to explain it.

"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.",
"link": "https://blog.soliditylang.org/2026/X/Y/Z/",
Copy link
Copy Markdown
Collaborator

@cameel cameel Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could already start working on the blog post for the bug. We won't really be able to merge the PR until we have a link to put here. This also depends on us scheduling a release that will include it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, this has never really been an issue since an important bug fix meant that a release followed almost immediately. Now, with the flood of those very low severity important bugs, this is not always the case. Maybe we should relax that.

My main concerns are that we may forget to update it and that we may end up with so many of these merged that work on the blog posts will delay the release even more. The first one could be addressed by having a release checklist step to make sure that the bug list entries added by the release are complete.

Changelog.md Outdated
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.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entries for important bugs have their own section (that we only add when such bugfixes are present). Check how it was done for earlier releases.

Copy link
Copy Markdown
Contributor Author

@rodiazet rodiazet Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved, but do we define this as important bug? How do we define them then?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it gets a bug list entry it's an important bug.

@rodiazet rodiazet force-pushed the remove-returndatacopy-elimination branch from aca08f7 to 9e623e9 Compare March 28, 2026 10:11
Comment on lines +161 to +162
bool isStorageOnlyWrite = (*instruction == Instruction::SSTORE);
bool isMemoryOnlyWrite =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const these.

Comment on lines +168 to +177
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 &&
!isBlacklisted &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants