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 places where assignable_expr should be used. #491

Closed
g-r-a-n-t opened this issue Jul 28, 2021 · 2 comments · Fixed by #493
Closed

Fix places where assignable_expr should be used. #491

g-r-a-n-t opened this issue Jul 28, 2021 · 2 comments · Fixed by #493

Comments

@g-r-a-n-t
Copy link
Member

g-r-a-n-t commented Jul 28, 2021

What is wrong?

There are a few places in the analyzer where we use expressions::expr to analyze expressions, when we should be using expressions::assignable_expr.

The name assignable_expr is not very clear, so we should consider changing that too.

The purpose of assignable_expr is to make sure expressions are in the default location of their type. The yulgen phase is then able to assume the location of a given value. If we don't do this, then we will have all sorts of undefined behavior.

How can it be fixed

@g-r-a-n-t g-r-a-n-t changed the title Fix places where assigable_expr should be used. Fix places where assignable_expr should be used. Jul 28, 2021
@g-r-a-n-t
Copy link
Member Author

g-r-a-n-t commented Jul 28, 2021

It might make sense to move all of this location business into the yulgen pass.

@g-r-a-n-t
Copy link
Member Author

g-r-a-n-t commented Jul 30, 2021

contract Foo:
    my_array: u256[3]

    pub def bar() -> u256:
        self.my_array = [1,2,3]
        sum: u256 = 0
        for i in self.my_array:
            sum = sum + i

        return sum

^ example of a feature that does not work properly at the moment

The yulgen phase expects the iterabe value to be in memory, but since we're not analyzing the expression with assignable_expr, there is no move_location attributed to it, so the array stays in storage. This results in OOG error during the execution of bar, since it attempts to access the array in memory using the very large storage pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant