vyperlang / vyper

Pythonic Smart Contract Language for the EVM
https://vyperlang.org
Other
4.81k stars 788 forks source link

fix[codegen]: panic on potential eval order issue for some builtins #4157

Closed charles-cooper closed 2 weeks ago

charles-cooper commented 2 weeks ago

What I did

alternative to https://github.com/vyperlang/vyper/pull/4156; panic instead of changing codegen

How I did it

How to verify it

Commit message

`extract32()` and `slice()` have an evaluation order issue when
the arguments touch the same data. specifically, the length and data
evaluation are interleaved with the index/start/length evaluations. in
unusual situations (such as those in the included test cases), this
can result in "invalid" reads where the data and length reads appear
out of order. this commit conservatively blocks compilation if the
preconditions for the interleaved evaluation are detected.

---------

Co-authored-by: trocher <trooocher@proton.me>
Co-authored-by: cyberthirst <cyberthirst.eth@gmail.com>

Description for the changelog

Cute Animal Picture

![Put a link to a cute animal picture inside the parenthesis-->]()

cyberthirst commented 2 weeks ago

should we add a hint on how to avoid this issue? or link to the issue tracker?

charles-cooper commented 2 weeks ago

should we add a hint on how to avoid this issue? or link to the issue tracker?

no, i think it's pretty unlikely anybody actually runs into this. if they do, they should be encouraged to create an issue (as prompted) so we can get some real world usage data

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.08%. Comparing base (d92cd34) to head (a90f50d).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4157 +/- ## ========================================== - Coverage 91.32% 90.08% -1.24% ========================================== Files 109 109 Lines 15569 15573 +4 Branches 3418 3420 +2 ========================================== - Hits 14218 14029 -189 - Misses 923 1070 +147 - Partials 428 474 +46 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.