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

fix[codegen]: fix make_setter overlap with internal calls #4037

Merged

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented May 20, 2024

What I did

fix #3503

How I did it

How to verify it

Commit message

fix overlap analysis for `make_setter` when the RHS contains an internal
call. this represents a bug which was partially fixed in 1c8349e;
this commit fixes two variants:

- where the RHS contains an internal call, the analysis needs to take
  into account all storage variables touched by the called function.

- where the RHS contains an external call, the analysis needs to assume
  any storage variable could be touched (this is a conservative
  analysis; a more advanced analysis could limit the assumption to
  storage variables touched by reentrant functions)

---------

Co-authored-by: cyberthirst <cyberthirst.eth@gmail.com>
Co-authored-by: trocher <rocher14@live.fr>

Description for the changelog

Cute Animal Picture

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

fix overlap analysis for `make_setter` when the RHS contains an internal
call. this represents a bug which was partially fixed in 1c8349e;
this commit fixes a variant where the RHS contains an internal call.
vyper/codegen/ir_node.py Show resolved Hide resolved
@charles-cooper charles-cooper added this to the v0.4.0 milestone May 25, 2024
@cyberthirst
Copy link
Collaborator

recursion missing:

def test_make_setter_external_call2(get_contract):
    # variant of GH #3503
    code = """
interface A:
   def boo(): nonpayable

a: DynArray[uint256, 10]

@external
def foo() -> DynArray[uint256, 10]:
    self.a = [1, 2, self.baz(), 4]
    return self.a # returns [11, 12, 3, 4]


@internal
def baz() -> uint256:
    extcall A(self).boo()
    return 3

@external
def boo():
    self.a = [11, 12, 13, 14, 15, 16]
    self.a = []
    # it should now be impossible to read any of [11, 12, 13, 14, 15, 16]
    """
    c = get_contract(code)

    assert c.foo() == [1, 2, 3, 4
>       assert c.foo() == [1, 2, 3, 4]
E       assert [11, 12, 3, 4] == [1, 2, 3, 4]
E         
E         At index 0 diff: 11 != 1
E         Use -v to get more diff

@@ -56,6 +57,10 @@ def set_frame_info(self, frame_info: FrameInfo) -> None:
else:
self.frame_info = frame_info

def set_func_ir(self, func_ir: "InternalFuncIR") -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?

@charles-cooper
Copy link
Member Author

charles-cooper commented May 26, 2024

thanks to @trocher for the test case contributed in 7fc2426

@charles-cooper charles-cooper merged commit ad9c10b into vyperlang:master May 26, 2024
157 checks passed
@charles-cooper charles-cooper deleted the fix/function-call-make-setter branch May 26, 2024 21:14
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.

bug: make_setter is incorrect for complex types when RHS references LHS with a function call
3 participants