argotorg / argotorg/solidity

The optimizer may completely remove all ``memoryguard``s preventing stack-to-memory.

Open
#12,913 0 comments 0 reactions 0 assignees View on GitHub
bug :bug: medium effort medium impact optimizer
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

Came up in https://github.com/ethereum/solidity/pull/12762.

We made ``memoryguard`` movable and removable to allow it to be CSE'd on the one hand and not to clutter the yul code with ``pop(memoryguard(...))`` on the other hand (also movable implies removable at the moment, so movable, but not *re*movable doesn't work).

Previously, the ``memoryguard``s would still never be removed entirely, since we'd always have a ``mstore(0x40, memoryguard(...))``, which depends on the actual value and could not be resolved.
Ever since the unused store eliminator, this ``mstore`` may be eliminated, though, which can result in the ``memoryguard`` to vanish, even though such code is succinctly memory-safe (these are cases that don't use any dynamic memory at all after optimization!).

With https://github.com/ethereum/solidity/pull/12762 such situations would become even more likely.

So we need a workaround for this. I have several options in mind, but none of them are overly nice. We can try to prevent the optimizer from removing the last ``memoryguard`` (hacky)... we could check for the memoryguard at the beginning of running the optimizer and then check again, if it has vanished and add it back as plain ``pop(memoryguard(...))`` (may be fine, but also a bit weird and kind of against the idea of ``memoryguard``s)... we could say that a vanishing ``memoryguard`` means no dynamic memory use and have the ``StackToMemoryMover`` still run and *introduce* a new ``memoryguard`` in these cases... not sure which is the least hacky option...

Contributor guide

Open the contributing guide

Research direction

Start by reading the optimizer behavior described here and the context in PR #12762, then trace the unused store eliminator and StackToMemoryMover around memoryguard handling. Compare the proposed workarounds and add regression coverage for optimized code where all memoryguards disappear; done means memory-safe code still receives the required stack-to-memory treatment.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, solidity
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.