Skip to content

Comments

fix[venom]: uninitialized mem after mem2var#4841

Open
HodanPlodky wants to merge 5 commits intovyperlang:masterfrom
HodanPlodky:fix/venom/uninitialized-mem-after-mem2var
Open

fix[venom]: uninitialized mem after mem2var#4841
HodanPlodky wants to merge 5 commits intovyperlang:masterfrom
HodanPlodky:fix/venom/uninitialized-mem-after-mem2var

Conversation

@HodanPlodky
Copy link
Collaborator

What I did

Fixed problem with Mem2Var pass where if the value is read before written for the first time the semantics with memory would be to return 0. This was not reflected after the pass. To achieve this I added the initial assign to variable.

Before mem2var:

main:
    %x = source
    %ptr = alloca 1, 32
    jmp @loop_header
loop_header:
    %i = phi @main, %x, @loop_body, %nexti
    %cond = iszero %i
    jnz %i, @loop_body, @exit
loop_body:
    %inv = sload 0
    %y = add %i, %inv
    mstore %ptr, %y
    %nexti = add %i, 1
    jmp @loop_header
exit:
    ; if the loop body is not run
    ; then the result should be 0
    %res = mload %ptr 
    sink %res

After mem2var and makessa:

main:
    %x = source
    %ptr = alloca 1, 32

    ; by default set to zero to mimic 
    ; uninitialized memory
    %alloca_ptr_0 = 0
    jmp @loop_header
loop_header:
    %alloca_ptr_0:1 = phi @main, %alloca_ptr_0, @loop_body, %alloca_ptr_0:2
    %i:1 = phi @main, %x, @loop_body, %nexti
    %i = %i:1
    %cond = iszero %i:1
    jnz %i:1, @loop_body, @exit
loop_body:
    %inv = sload 0
    %y = add %i:1, %inv
    %alloca_ptr_0:2 = %y
    %nexti = add %i:1, 1
    jmp @loop_header
exit:
    %res = %alloca_ptr_0:1
    sink %res

How I did it

How to verify it

Commit message

Added initial assign to the Mem2Var pass for cases where it would be necessary to read from uninitialized memory.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 91.78082% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.88%. Comparing base (2fd9dc4) to head (2832fde).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
vyper/venom/analysis/defined_mem.py 94.11% 1 Missing and 2 partials ⚠️
vyper/venom/passes/mem2var.py 87.50% 1 Missing and 1 partial ⚠️
vyper/venom/passes/machinery/inst_updater.py 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4841      +/-   ##
==========================================
- Coverage   92.89%   92.88%   -0.01%     
==========================================
  Files         155      157       +2     
  Lines       21744    21912     +168     
  Branches     3885     3929      +44     
==========================================
+ Hits        20199    20353     +154     
- Misses       1024     1029       +5     
- Partials      521      530       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@charles-cooper
Copy link
Member

is this generated by vyper code? or is the example handcrafted venom

i think that for unallocated memory, the assumption that the first read returns 0 is not true, if the allocator has given it a memory space which was previously written to already

@github-actions
Copy link

github-actions bot commented Feb 7, 2026

📊 Bytecode Size Changes (venom)

No changes detected.

Full bytecode sizes

Contract legacy-O2 legacy-Os -O2 -O3 -Os
curvefi/amm/stableswap/implementation/implementation_v_700.vy 24962 23769 19721 19510 17925
curvefi/legacy/CurveStableSwapMetaNG.vy 24952 23578 19339 19195 17604
curvefi/legacy/CurveStableSwapNG.vy 24473 23298 19184 18958 17405
curvefi/amm/stableswap/meta_implementation/meta_implementation_v_700.vy 23610 22805 18609 18276 17259
yearnfi/VaultV3.vy 19972 19063 16658 16460 14050
curvefi/amm/tricryptoswap/implementation/implementation_v_200.vy 20590 19825 16461 16212 15282
curvefi/amm/twocryptoswap/implementation/implementation_v_210.vy 18090 17350 14668 14420 13456
curvefi/legacy/CurveCryptoSwap2.vy 18947 18382 14468 14181 13532
yearnfi/VaultV2.vy 16676 15763 13303 13241 11954
curvefi/amm/stableswap/factory/factory_v_100.vy 14558 13978 12923 12366 11423
curvefi/gauge/child_gauge/implementation/implementation_v_110.vy 12338 11561 10014 9881 8983
curvefi/gauge/child_gauge/implementation/implementation_v_100.vy 12017 11249 9733 9600 8712
curvefi/amm/stableswap/views/views_v_120.vy 12784 12368 9596 9365 8767
curvefi/amm/tricryptoswap/math/math_v_200.vy 11055 10992 9226 8736 7964
curvefi/legacy/CurveCryptoMathOptimized3.vy 11054 10991 9225 8735 7964
curvefi/gauge/child_gauge/implementation/implementation_v_020.vy 10665 9947 8688 8512 7711
curvefi/registries/metaregistry/metaregistry_v_110.vy 7590 6732 6325 5854 5343
curvefi/amm/tricryptoswap/views/views_v_200.vy 7821 7776 6170 6035 5935
curvefi/helpers/stable_swap_meta_zap/stable_swap_meta_zap_v_100.vy 7302 7067 5909 5765 5454
curvefi/helpers/router/router_v_110.vy 6717 6717 5816 5476 5444
curvefi/amm/twocryptoswap/views/views_v_200.vy 6991 6946 5698 5574 5463
curvefi/registries/metaregistry/registry_handlers/stableswap/handler_v_110.vy 6633 6259 5661 5276 5216
curvefi/amm/twocryptoswap/factory/factory_v_200.vy 5540 5252 5268 4626 4222
curvefi/amm/twocryptoswap/math/math_v_210.vy 6666 6666 5221 5221 4647
curvefi/amm/tricryptoswap/factory/factory_v_200.vy 5246 5021 4829 4323 4081
curvefi/gauge/child_gauge/factory/factory_v_201.vy 4844 4547 3980 3962 3525
yearnfi/VaultFactory.vy 3765 3617 3909 2919 2928
curvefi/registries/metaregistry/registry_handlers/tricryptoswap/handler_v_110.vy 4241 3939 3577 3300 3203
curvefi/registries/metaregistry/registry_handlers/twocryptoswap/handler_v_110.vy 4186 3884 3464 3199 3065
curvefi/gauge/child_gauge/factory/factory_v_100.vy 4183 3914 3354 3336 2963
curvefi/registries/address_provider/address_provider_v_201.vy 2973 2782 2652 2647 2365
curvefi/helpers/rate_provider/rate_provider_v_101.vy 3260 3260 2471 2449 2292
curvefi/amm/stableswap/math/math_v_100.vy 3067 3046 2442 2441 2298
curvefi/helpers/rate_provider/rate_provider_v_100.vy 2847 2841 2368 2360 2107
curvefi/helpers/deposit_and_stake_zap/deposit_and_stake_zap_v_100.vy 2322 2316 2036 2007 1799
curvefi/governance/relayer/taiko/relayer_v_001.vy 2068 2064 1795 1788 1669
curvefi/governance/relayer/polygon_cdk/relayer_v_101.vy 1556 1523 1460 1453 1323
curvefi/governance/relayer/arb_orbit/relayer_v_101.vy 1266 1262 1172 1158 1093
curvefi/governance/relayer/op_stack/relayer_v_101.vy 1186 1182 1109 1098 1030
curvefi/governance/relayer/not_rollup/relayer_v_100.vy 1168 1153 1103 1098 1012
curvefi/governance/vault/vault_v_100.vy 964 941 889 889 820
curvefi/governance/agent/agent_v_100.vy 541 541 517 517 463
curvefi/governance/agent/agent_v_101.vy 541 541 517 517 463
curvefi/governance/relayer/relayer_v_100.vy 496 496 465 460 465

@charles-cooper
Copy link
Member

like it should be an error to try to read alloca space before it has been written to, no?


try:
checker.run_passes(pre, pre)
except CompilerPanic:

Check notice

Code scanning / CodeQL

Empty except Note test

'except' clause does nothing but pass and there is no explanatory comment.
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.

2 participants