Double Eval in experimental codegen without optimization

[ritzdorf]

Version Information

• vyper Version (output of vyper --version OR linkable commit hash vyperlang/vyper@commitish): 0.4.0rc6+commit.33719560

Issue description

In version 0.4.0rc6 it seems that #4030 fixed the issue #4071 for all tested compilation settings EXCEPT when compiling with experimental_codegen and no optimization. For that compilation setting incorrect bytecode with double evaluation is still being generated.

PoC

m: HashMap[uint256, String[33]]

@external
def foo() -> uint256:
    x: DynArray[uint256, 32] = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
    self.m[x.pop()] = "Hello world"
    return len(x)

And compile with

vyper --experimental-codegen -O none example.vy

Returns 30, but should return 31.

[harkal]

It appears that the latest master (commit 21f7172) fails to compile the above example. I have created a test case for it in #4130 to investigate for venom, but it seems to be a general issue.

[trocher]

It appears that the latest master (commit 21f7172) fails to compile the above example. I have created a test case for it in #4130 to investigate for venom, but it seems to be a general issue.

I think master failing to compile is expected given that #3514/#4071 have not been fixed yet, only a fence was added (#4030) to prevent incorrect bytecode generation by having the compiler fail, but the underlying issue is not fixed

[harkal]

so #4030 is not actually fixing the issue, instead it asserts to abort from producing erroneous bytecode?

[trocher]

Exact, #4030, together with #3835 it extend the usage of unique_symbol to more expressions that are known to have side effects. They make the compiler fail if the tagged node gets evaluated more than once by using unique_symbol:

Concerned (IR) nodes are:

• CALL (used in send, raw_call or regular external calls)
• CREATE/CREATE2 (used in all relevant builtin)
• SELFDESTRUCT
• LOGK (in raw_log)
• all STORE done by append and pop (storage, transient or mem)
• all TSTORE and SSTORE
• all internal calls

This means that for now, the compiler relies on that list to be exhaustive not to perform any double evaluation of the dst of an assignment that would lead to visible side effects

[charles-cooper]

yea we can probably demote this to "type 0" bug (panic instead of producing bad code). but we should still investigate since it indicates some kind of deeper issue

[charles-cooper]

ah never mind, i see what's happening -- -Onone --experimental-codegen bypasses the unique symbols check