Repository: aptos-labs/aptos-core (language: python)

Issue:
[Bug][move-compiler-v2] Copy mutable ref twice for ==
# 🐛 Bug

```
module 0xCAFE::Module0 {
    public fun function2() {
        let x = &mut 0u8;
        (copy x == copy x);
    }
}
```

This compiles with V1, but V2 will report:
```
error: mutable reference in local `x` requires exclusive access but is borrowed
  ┌─ ~/debug_package/sources/repro.move:4:20
  │
4 │         (copy x == copy x);
  │         -----------^^^^^^-
  │         │          │
  │         │          requirement enforced here
  │         conflicting reference used here

error: same mutable reference in value is also used in other value in argument list
  ┌─ ~/debug_package/sources/repro.move:4:9
  │
3 │         let x = &mut 0u8;
  │                 -------- previous mutable local borrow
4 │         (copy x == copy x);
  │         ^^^^^^^^^^^^^^^^^^ requirement enforced here
```

The operations are safe so probably just a difference in V1/V2's ref safety algorithm.


Discussion / hints:

Fix patch:
diff --git a/third_party/move/move-compiler-v2/src/bytecode_generator.rs b/third_party/move/move-compiler-v2/src/bytecode_generator.rs
index 785fa528b3396..f09f82a1ae9f7 100644
--- a/third_party/move/move-compiler-v2/src/bytecode_generator.rs
+++ b/third_party/move/move-compiler-v2/src/bytecode_generator.rs
@@ -791,12 +791,14 @@ impl<'env> Generator<'env> {
             Operation::Shr => self.gen_op_call(targets, id, BytecodeOperation::Shr, args),
             Operation::And => self.gen_logical_shortcut(true, targets, id, args),
             Operation::Or => self.gen_logical_shortcut(false, targets, id, args),
-            Operation::Eq => self.gen_op_call(targets, id, BytecodeOperation::Eq, args),
-            Operation::Neq => self.gen_op_call(targets, id, BytecodeOperation::Neq, args),
-            Operation::Lt => self.gen_op_call(targets, id, BytecodeOperation::Lt, args),
-            Operation::Gt => self.gen_op_call(targets, id, BytecodeOperation::Gt, args),
-            Operation::Le => self.gen_op_call(targets, id, BytecodeOperation::Le, args),
-            Operation::Ge => self.gen_op_call(targets, id, BytecodeOperation::Ge, args),
+            Operation::Eq => self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Eq, args),
+            Operation::Neq => {
+                self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Neq, args)
+            },
+            Operation::Lt => self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Lt, args),
+            Operation::Gt => self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Gt, args),
+            Operation::Le => self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Le, args),
+            Operation::Ge => self.gen_op_call_auto_freeze(targets, id, BytecodeOperation::Ge, args),
             Operation::Not => self.gen_op_call(targets, id, BytecodeOperation::Not, args),
 
             Operation::NoOp => {}, // do nothing
@@ -896,6 +898,28 @@ impl<'env> Generator<'env> {
         self.gen_op_call(targets, id, bytecode_op, args)
     }
 
+    fn gen_op_call_auto_freeze(
+        &mut self,
+        targets: Vec<TempIndex>,
+        id: NodeId,
+        op: BytecodeOperation,
+        args: &[Exp],
+    ) {
+        // Freeze arguments if needed
+        let args = args
+            .iter()
+            .map(|e| {
+                let ty = self.env().get_node_type(e.node_id());
+                if let Type::Reference(ReferenceKind::Mutable, elem_ty) = ty {
+                    self.maybe_convert(e, &Type::Reference(ReferenceKind::Immutable, elem_ty))
+                } else {
+                    e.clone()
+                }
+            })
+            .collect::<Vec<_>>();
+        self.gen_op_call(targets, id, op, &args)
+    }
+
     fn gen_op_call(
         &mut self,
         targets: Vec<TempIndex>,


Test patch:
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.exp
deleted file mode 100644
index 36f5599b0bc89..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.exp
+++ /dev/null
@@ -1,9 +0,0 @@
-
-============ bytecode verification failed ========
-
-Diagnostics:
-bug: bytecode verification failed with unexpected status code `EQUALITY_OP_TYPE_MISMATCH_ERROR`. This is a compiler bug, consider reporting it.
-  ┌─ tests/reference-safety/bug_14060.move:6:9
-  │
-6 │         x != copy y;
-  │         ^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.no-opt.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.no-opt.exp
deleted file mode 100644
index 36f5599b0bc89..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.no-opt.exp
+++ /dev/null
@@ -1,9 +0,0 @@
-
-============ bytecode verification failed ========
-
-Diagnostics:
-bug: bytecode verification failed with unexpected status code `EQUALITY_OP_TYPE_MISMATCH_ERROR`. This is a compiler bug, consider reporting it.
-  ┌─ tests/reference-safety/bug_14060.move:6:9
-  │
-6 │         x != copy y;
-  │         ^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.old.exp
deleted file mode 100644
index 36f5599b0bc89..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.old.exp
+++ /dev/null
@@ -1,9 +0,0 @@
-
-============ bytecode verification failed ========
-
-Diagnostics:
-bug: bytecode verification failed with unexpected status code `EQUALITY_OP_TYPE_MISMATCH_ERROR`. This is a compiler bug, consider reporting it.
-  ┌─ tests/reference-safety/bug_14060.move:6:9
-  │
-6 │         x != copy y;
-  │         ^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.exp
new file mode 100644
index 0000000000000..90b32906711cc
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.exp
@@ -0,0 +1,2 @@
+
+============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.move b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.move
similarity index 55%
rename from third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.move
rename to third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.move
index 909fb37ee994c..89fcf3ab500a4 100644
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060.move
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.move
@@ -1,4 +1,3 @@
-// TODO(#14060): after fix, rename file to reflect issue (`bug_nnn.move` to `bug_nnn_<issue>.move`)
 module 0xCAFE::Module0 {
     public fun f() {
         let x = &mut 0u8;
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.no-opt.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.no-opt.exp
new file mode 100644
index 0000000000000..90b32906711cc
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.no-opt.exp
@@ -0,0 +1,2 @@
+
+============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.old.exp
new file mode 100644
index 0000000000000..90b32906711cc
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14060_relation_op_freeze.old.exp
@@ -0,0 +1,2 @@
+
+============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.exp
deleted file mode 100644
index 9eec85cc70714..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.exp
+++ /dev/null
@@ -1,9 +0,0 @@
-
-============ bytecode verification failed ========
-
-Diagnostics:
-bug: bytecode verification failed with unexpected status code `READREF_EXISTS_MUTABLE_BORROW_ERROR`. This is a compiler bug, consider reporting it.
-  ┌─ tests/reference-safety/bug_14096.move:6:9
-  │
-6 │         (copy x == copy x);
-  │         ^^^^^^^^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.no-opt.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.no-opt.exp
deleted file mode 100644
index 9eec85cc70714..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.no-opt.exp
+++ /dev/null
@@ -1,9 +0,0 @@
-
-============ bytecode verification failed ========
-
-Diagnostics:
-bug: bytecode verification failed with unexpected status code `READREF_EXISTS_MUTABLE_BORROW_ERROR`. This is a compiler bug, consider reporting it.
-  ┌─ tests/reference-safety/bug_14096.move:6:9
-  │
-6 │         (copy x == copy x);
-  │         ^^^^^^^^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.old.exp
deleted file mode 100644
index e946e2fed7f03..0000000000000
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.old.exp
+++ /dev/null
@@ -1,18 +0,0 @@
-
-Diagnostics:
-error: mutable reference in local `x` requires exclusive access but is borrowed
-  ┌─ tests/reference-safety/bug_14096.move:6:20
-  │
-6 │         (copy x == copy x);
-  │         -----------^^^^^^-
-  │         │          │
-  │         │          requirement enforced here
-  │         conflicting reference used here
-
-error: same mutable reference in value is also used in other value in argument list
-  ┌─ tests/reference-safety/bug_14096.move:6:9
-  │
-5 │         let x = &mut 0u8;
-  │                 -------- previous mutable local borrow
-6 │         (copy x == copy x);
-  │         ^^^^^^^^^^^^^^^^^^ requirement enforced here
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.exp
new file mode 100644
index 0000000000000..90b32906711cc
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.exp
@@ -0,0 +1,2 @@
+
+============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.move b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.move
similarity index 61%
rename from third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.move
rename to third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.move
index f72038e107a5b..925cf3ab7b6eb 100644
--- a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096.move
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.move
@@ -1,4 +1,3 @@
-// TODO(#14096): after fix, rename file to reflect issue (`bug_nnn.move` to `bug_nnn_<issue>.move`)
 module 0xCAFE::Module0 {
     // Expected to compile without errors
     public fun function2() {
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.no-opt.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.no-opt.exp
new file mode 100644
index 0000000000000..90b32906711cc
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.no-opt.exp
@@ -0,0 +1,2 @@
+
+============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.old.exp
new file mode 100644
index 0000000000000..5923dd2a15039
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/reference-safety/bug_14096_relation_op_freeze.old.exp
@@ -0,0 +1,11 @@
+
+Diagnostics:
+error: mutable reference in local `x` requires exclusive access but is borrowed
+  ┌─ tests/reference-safety/bug_14096_relation_op_freeze.move:5:20
+  │
+5 │         (copy x == copy x);
+  │         -----------^^^^^^-
+  │         ││         │
+  │         ││         requirement enforced here
+  │         │previous freeze
+  │         conflicting reference used here