Repository: vyperlang/vyper

Issue:
make_setter is incorrect for complex types when the RHS references the LHS
### 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].
```python

@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).

Discussion:
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang) Report
> Merging [#3410](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang) (a9d9b3e) into [master](https://app.codecov.io/gh/vyperlang/vyper/commit/97ff017c5e8d1aa5b0bcefbc93cc5cf9c131f71c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang) (97ff017) will **decrease** coverage by `36.21%`.
> The diff coverage is `83.87%`.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please [install the Github App Integration](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang).

```diff
@@             Coverage Diff             @@
##           master    #3410       +/-   ##
===========================================
- Coverage   88.96%   52.75%   -36.21%     
===========================================
  Files          86       86               
  Lines       10765    10789       +24     
  Branches     2445     2448        +3     
===========================================
- Hits         9577     5692     -3885     
- Misses        786     4542     +3756     
- Partials      402      555      +153     
```


| [Impacted Files](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang) | Coverage Δ | |
|---|---|---|
| [vyper/codegen/stmt.py](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang#diff-dnlwZXIvY29kZWdlbi9zdG10LnB5) | `44.48% <54.54%> (-44.99%)` | :arrow_down: |
| [vyper/codegen/context.py](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang#diff-dnlwZXIvY29kZWdlbi9jb250ZXh0LnB5) | `80.88% <100.00%> (-10.91%)` | :arrow_down: |
| [vyper/codegen/expr.py](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang#diff-dnlwZXIvY29kZWdlbi9leHByLnB5) | `31.62% <100.00%> (-55.88%)` | :arrow_down: |
| [vyper/codegen/ir\_node.py](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang#diff-dnlwZXIvY29kZWdlbi9pcl9ub2RlLnB5) | `86.19% <100.00%> (-4.84%)` | :arrow_down: |
| [vyper/semantics/analysis/base.py](https://app.codecov.io/gh/vyperlang/vyper/pull/3410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang#diff-dnlwZXIvc2VtYW50aWNzL2FuYWx5c2lzL2Jhc2UucHk=) | `67.12% <100.00%> (-14.13%)` | :arrow_down: |

... and [62 files with indirect coverage changes](https://app.codecov.io/gh/vyperlang/vyper/pull/3410/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vyperlang)


Patch:
diff --git a/vyper/codegen/context.py b/vyper/codegen/context.py
index 9902cd0c..696b81d1 100644
--- a/vyper/codegen/context.py
+++ b/vyper/codegen/context.py
@@ -30,6 +30,9 @@ class VariableRecord:
     is_immutable: bool = False
     data_offset: Optional[int] = None
 
+    def __hash__(self):
+        return hash(id(self))
+
     def __post_init__(self):
         if self.blockscopes is None:
             self.blockscopes = []
diff --git a/vyper/codegen/expr.py b/vyper/codegen/expr.py
index dd764fbe..6da3d950 100644
--- a/vyper/codegen/expr.py
+++ b/vyper/codegen/expr.py
@@ -166,7 +166,7 @@ class Expr:
             return IRnode.from_list(["address"], typ=AddressT())
         elif self.expr.id in self.context.vars:
             var = self.context.vars[self.expr.id]
-            return IRnode.from_list(
+            ret = IRnode.from_list(
                 var.pos,
                 typ=var.typ,
                 location=var.location,  # either 'memory' or 'calldata' storage is handled above.
@@ -174,6 +174,8 @@ class Expr:
                 annotation=self.expr.id,
                 mutable=var.mutable,
             )
+            ret._referenced_variables = {var}
+            return ret
 
         # TODO: use self.expr._expr_info
         elif self.expr.id in self.context.globals:
@@ -189,9 +191,11 @@ class Expr:
                 mutable = False
                 location = DATA
 
-            return IRnode.from_list(
+            ret = IRnode.from_list(
                 ofst, typ=varinfo.typ, location=location, annotation=self.expr.id, mutable=mutable
             )
+            ret._referenced_variables = {varinfo}
+            return ret
 
     # x.y or x[5]
     def parse_Attribute(self):
@@ -255,12 +259,16 @@ class Expr:
         # self.x: global attribute
         elif isinstance(self.expr.value, vy_ast.Name) and self.expr.value.id == "self":
             varinfo = self.context.globals[self.expr.attr]
-            return IRnode.from_list(
+            ret = IRnode.from_list(
                 varinfo.position.position,
                 typ=varinfo.typ,
                 location=STORAGE,
                 annotation="self." + self.expr.attr,
             )
+            ret._referenced_variables = {varinfo}
+
+            return ret
+
         # Reserved keywords
         elif (
             isinstance(self.expr.value, vy_ast.Name) and self.expr.value.id in ENVIRONMENT_VARIABLES
diff --git a/vyper/codegen/ir_node.py b/vyper/codegen/ir_node.py
index 1ba4122c..d36a18ec 100644
--- a/vyper/codegen/ir_node.py
+++ b/vyper/codegen/ir_node.py
@@ -397,6 +397,16 @@ class IRnode:
 
         return _WithBuilder(self, name, should_inline)
 
+    @cached_property
+    def referenced_variables(self):
+        ret = set()
+        for arg in self.args:
+            ret |= arg.referenced_variables
+
+        ret |= getattr(self, "_referenced_variables", set())
+
+        return ret
+
     @cached_property
     def contains_self_call(self):
         return getattr(self, "is_self_call", False) or any(x.contains_self_call for x in self.args)
diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py
index 01c1d5f1..e24c4296 100644
--- a/vyper/codegen/stmt.py
+++ b/vyper/codegen/stmt.py
@@ -73,11 +73,22 @@ class Stmt:
 
     def parse_Assign(self):
         # Assignment (e.g. x[4] = y)
-        sub = Expr(self.stmt.value, self.context).ir_node
-        target = self._get_target(self.stmt.target)
+        src = Expr(self.stmt.value, self.context).ir_node
+        dst = self._get_target(self.stmt.target)
 
-        ir_node = make_setter(target, sub)
-        return ir_node
+        ret = ["seq"]
+        overlap = len(dst.referenced_variables & src.referenced_variables) > 0
+        if overlap and not dst.typ._is_prim_word:
+            # there is overlap between the lhs and rhs, and the type is
+            # complex - i.e., it spans multiple words. for safety, we
+            # copy to a temporary buffer before copying to the destination.
+            tmp = self.context.new_internal_variable(src.typ)
+            tmp = IRnode.from_list(tmp, typ=src.typ, location=MEMORY)
+            ret.append(make_setter(tmp, src))
+            src = tmp
+
+        ret.append(make_setter(dst, src))
+        return IRnode.from_list(ret)
 
     def parse_If(self):
         if self.stmt.orelse:
@@ -336,8 +347,12 @@ class Stmt:
 
     def parse_AugAssign(self):
         target = self._get_target(self.stmt.target)
+
         sub = Expr.parse_value_expr(self.stmt.value, self.context)
         if not target.typ._is_prim_word:
+            # because of this check, we do not need to check for
+            # make_setter references lhs<->rhs as in parse_Assign -
+            # single word load/stores are atomic.
             return
 
         with target.cache_when_complex("_loc") as (b, target):
diff --git a/vyper/semantics/analysis/base.py b/vyper/semantics/analysis/base.py
index 5919c966..5065131f 100644
--- a/vyper/semantics/analysis/base.py
+++ b/vyper/semantics/analysis/base.py
@@ -165,6 +165,9 @@ class VarInfo:
     is_local_var: bool = False
     decl_node: Optional[vy_ast.VyperNode] = None
 
+    def __hash__(self):
+        return hash(id(self))
+
     def __post_init__(self):
         self._modification_count = 0
 
diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py
index 22ac8cec..3907882e 100644
--- a/vyper/semantics/analysis/module.py
+++ b/vyper/semantics/analysis/module.py
@@ -226,6 +226,17 @@ class ModuleAnalyzer(VyperNodeVisitorBase):
             except VyperException as exc:
                 raise exc.with_annotation(node) from None
 
+        def _validate_self_namespace():
+            # block globals if storage variable already exists
+            try:
+                if name in self.namespace["self"].typ.members:
+                    raise NamespaceCollision(
+                        f"Value '{name}' has already been declared", node
+                    ) from None
+                self.namespace[name] = var_info
+            except VyperException as exc:
+                raise exc.with_annotation(node) from None
+
         if node.is_constant:
             if not node.value:
                 raise VariableDeclarationException("Constant must be declared with a value", node)
@@ -233,10 +244,7 @@ class ModuleAnalyzer(VyperNodeVisitorBase):
                 raise StateAccessViolation("Value must be a literal", node.value)
 
             validate_expected_type(node.value, type_)
-            try:
-                self.namespace[name] = var_info
-            except VyperException as exc:
-                raise exc.with_annotation(node) from None
+            _validate_self_namespace()
 
             return _finalize()
 
@@ -247,16 +255,7 @@ class ModuleAnalyzer(VyperNodeVisitorBase):
             )
 
         if node.is_immutable:
-            try:
-                # block immutable if storage variable already exists
-                if name in self.namespace["self"].typ.members:
-                    raise NamespaceCollision(
-                        f"Value '{name}' has already been declared", node
-                    ) from None
-                self.namespace[name] = var_info
-            except VyperException as exc:
-                raise exc.with_annotation(node) from None
-
+            _validate_self_namespace()
             return _finalize()
 
         try:
diff --git a/vyper/semantics/analysis/utils.py b/vyper/semantics/analysis/utils.py
index 368f590f..26f3fd18 100644
--- a/vyper/semantics/analysis/utils.py
+++ b/vyper/semantics/analysis/utils.py
@@ -334,7 +334,11 @@ class _ExprAnalyser:
     def types_from_Name(self, node):
         # variable name, e.g. `foo`
         name = node.id
-        if name not in self.namespace and name in self.namespace["self"].typ.members:
+        if (
+            name not in self.namespace
+            and "self" in self.namespace
+            and name in self.namespace["self"].typ.members
+        ):
             raise InvalidReference(
                 f"'{name}' is a storage variable, access it as self.{name}", node
             )


Test patch:
diff --git a/tests/parser/features/test_assignment.py b/tests/parser/features/test_assignment.py
index 65fb3a7a..0dd63a0d 100644
--- a/tests/parser/features/test_assignment.py
+++ b/tests/parser/features/test_assignment.py
@@ -255,3 +255,63 @@ def foo():
     ret : bool = self.bar()
 """
     assert_compile_failed(lambda: get_contract_with_gas_estimation(code), InvalidType)
+
+
+def test_assign_rhs_lhs_overlap(get_contract):
+    # GH issue 2418
+    code = """
+@external
+def bug(xs: uint256[2]) -> uint256[2]:
+    # Initial value
+    ys: uint256[2] = xs
+    ys = [ys[1], ys[0]]
+    return ys
+    """
+    c = get_contract(code)
+
+    assert c.bug([1, 2]) == [2, 1]
+
+
+def test_assign_rhs_lhs_partial_overlap(get_contract):
+    # GH issue 2418, generalize when lhs is not only dependency of rhs.
+    code = """
+@external
+def bug(xs: uint256[2]) -> uint256[2]:
+    # Initial value
+    ys: uint256[2] = xs
+    ys = [xs[1], ys[0]]
+    return ys
+    """
+    c = get_contract(code)
+
+    assert c.bug([1, 2]) == [2, 1]
+
+
+def test_assign_rhs_lhs_overlap_dynarray(get_contract):
+    # GH issue 2418, generalize to dynarrays
+    code = """
+@external
+def bug(xs: DynArray[uint256, 2]) -> DynArray[uint256, 2]:
+    ys: DynArray[uint256, 2] = xs
+    ys = [ys[1], ys[0]]
+    return ys
+    """
+    c = get_contract(code)
+    assert c.bug([1, 2]) == [2, 1]
+
+
+def test_assign_rhs_lhs_overlap_struct(get_contract):
+    # GH issue 2418, generalize to structs
+    code = """
+struct Point:
+    x: uint256
+    y: uint256
+
+@external
+def bug(p: Point) -> Point:
+    t: Point = p
+    t = Point({x: t.y, y: t.x})
+    return t
+    """
+    c = get_contract(code)
+    assert c.bug((1, 2)) == (2, 1)
diff --git a/tests/parser/syntax/test_constants.py b/tests/parser/syntax/test_constants.py
index 15546672..ffd2f1fa 100644
--- a/tests/parser/syntax/test_constants.py
+++ b/tests/parser/syntax/test_constants.py
@@ -69,6 +69,46 @@ VAL: constant(Bytes[4]) = b"testtest"
         """
 VAL: constant(Bytes[4]) = b"t"
 VAL: uint256
+    """,
+        NamespaceCollision,
+    ),
+    # global with same type and name
+    (
+        """
+VAL: constant(uint256) = 1
+VAL: uint256
+    """,
+        NamespaceCollision,
+    ),
+    # global with same type and name, different order
+    (
+        """
+VAL: uint256
+VAL: constant(uint256) = 1
+    """,
+        NamespaceCollision,
+    ),
+    # global with same type and name
+    (
+        """
+VAL: immutable(uint256)
+VAL: uint256
+
+@external
+def __init__():
+    VAL = 1
+    """,
+        NamespaceCollision,
+    ),
+    # global with same type and name, different order
+    (
+        """
+VAL: uint256
+VAL: immutable(uint256)
+
+@external
+def __init__():
+    VAL = 1
     """,
         NamespaceCollision,
     ),