Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_setter is incorrect for complex types when the RHS references the LHS #2418

Closed
charles-cooper opened this issue Aug 14, 2021 · 1 comment · Fixed by #3410
Closed

make_setter is incorrect for complex types when the RHS references the LHS #2418

charles-cooper opened this issue Aug 14, 2021 · 1 comment · Fixed by #3410
Labels
bug - codegen bug - type 1 bug which results in incorrect codegen codegen issue with codegen
Milestone

Comments

@charles-cooper
Copy link
Member

Version Information

  • vyper Version (output of vyper --version): 0.2.15

What's your issue about?

This causes the memory location of x[0] to be overwritten before it gets copied to x[1].

@public
def bug(x_unsorted: uint256[2]) -> uint256:
    # Initial value
    x: uint256[2] = x_unsorted
    x = [x[1], x[0]] # bug!
    # x is now equal to [ x_unsorted[1], x_unsorted[1] ]
    return x[0]
# But when it's x = [x_unsorted[1], x_unsorted[0]] - all good

On 0.2.15 this produces code like

[seq,
  [mstore, 416, [mload, 448 <32+x>]],
  [mstore, 448, [mload, 416 <0+x>]]

This has been there for awhile, even in 0.2.0 we get

[with, _L, 416 <x>,
  [seq,
    [mstore, _L, [mload, 448 <32+x>]], <- still bug
    [mstore, [add, 32, _L], [mload, 416 <0+x>]]]],

I'm guessing a similar issue exists for contract variables.

How can it be fixed?

Well, fix make_setter, but it's unclear what the best approach is. The first obvious fix is just to process all the loads before all the stores, but this breaks for byte arrays (because those don't use the stack at all, they use memcpy macros).

@charles-cooper charles-cooper added the bug Bug that shouldn't change language semantics when fixed. label Aug 14, 2021
@charles-cooper
Copy link
Member Author

probably the most sane thing to do here is, first detect if the RHS depends on the LHS. if there is a dependency, then make_setter to a temporary buffer and then run make_setter on that to the LHS.

@charles-cooper charles-cooper added codegen issue with codegen bug - codegen bug - type 1 bug which results in incorrect codegen and removed bug Bug that shouldn't change language semantics when fixed. labels Apr 15, 2022
@charles-cooper charles-cooper added this to the v0.3.8 milestone May 14, 2023
@charles-cooper charles-cooper changed the title make_setter is incorrect for complex types when the RHS references the LHS assignment is incorrect for complex types when the RHS references the LHS May 14, 2023
@charles-cooper charles-cooper changed the title assignment is incorrect for complex types when the RHS references the LHS make_setter is incorrect for complex types when the RHS references the LHS May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug - codegen bug - type 1 bug which results in incorrect codegen codegen issue with codegen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant