Adding better error message for bytecode verifier ability check, plus…

… displaying message in compiler pipeline. This also demonstrates the problem with bug #14813
aptos-labs · Oct 16, 2024 · 5bb29c3 · 5bb29c3
1 parent 09427b2
commit 5bb29c3
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 2 deletions.
diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs
@@ -812,7 +812,13 @@ impl<'a, const N: usize> SignatureChecker<'a, N> {
         for (offset, instr) in code.code.iter().enumerate() {
             let map_err = |res: PartialVMResult<()>| {
                 res.map_err(|err| {
-                    err.append_message_with_separator(' ', format!("at offset {}", offset))
+                    err.append_message_with_separator(
+                        ' ',
+                        format!(
+                            "missing abilities for `{:?}` at code offset {}",
+                            instr, offset
+                        ),
+                    )
                 })
             };
             match instr {

diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs
@@ -570,7 +570,10 @@ fn report_bytecode_verification_error(
         let debug_info = if command_line::get_move_compiler_backtrace_from_env() {
             format!("\n{:#?}", e)
         } else {
-            "".to_string()
+            format!(
+                "\nError message: {}",
+                e.message().cloned().unwrap_or_else(|| "none".to_string())
+            )
         };
         env.diag(
             Severity::Bug,

diff --git a/third_party/move/move-compiler-v2/tests/bytecode-verify-failure/equality.exp b/third_party/move/move-compiler-v2/tests/bytecode-verify-failure/equality.exp
@@ -16,6 +16,7 @@ B0:
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `EQUALITY_OP_TYPE_MISMATCH_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
   ┌─ tests/bytecode-verify-failure/equality.move:3:9
   │
 3 │         x == y

diff --git a/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.exp b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.exp
@@ -0,0 +1,46 @@
+
+Diagnostics:
+warning: Unused parameter `key`. Consider removing or prefixing with an underscore: `_key`
+  ┌─ tests/file-format-generator/bug_14813_fun_call_ability.move:9:40
+  │
+9 │     fun search<V: drop>(self: &Map<V>, key: u256): bool {
+  │                                        ^^^
+
+
+============ disassembled file-format ==================
+// Move bytecode v7
+module 815.red_black_map {
+struct Map<phantom Ty0> has drop {
+	dummy_field: bool
+}
+
+public contains<Ty0>(Arg0: &Map<Ty0>, Arg1: u256): bool /* def_idx: 0 */ {
+B0:
+	0: MoveLoc[0](Arg0: &Map<Ty0>)
+	1: MoveLoc[1](Arg1: u256)
+	2: Call search<Ty0>(&Map<Ty0>, u256): bool
+	3: Ret
+}
+search<Ty0: drop>(Arg0: &Map<Ty0>, Arg1: u256): bool /* def_idx: 1 */ {
+B0:
+	0: LdTrue
+	1: MoveLoc[0](Arg0: &Map<Ty0>)
+	2: Pop
+	3: Ret
+}
+}
+============ bytecode verification failed ========
+
+Diagnostics:
+bug: bytecode verification failed with unexpected status code `CONSTRAINT_NOT_SATISFIED`. This is a compiler bug, consider reporting it.
+Error message: missing abilities for `CallGeneric(0)` at code offset 2
+   ┌─ tests/file-format-generator/bug_14813_fun_call_ability.move:1:1
+   │
+ 1 │ ╭ module 0x815::red_black_map {
+ 2 │ │
+ 3 │ │     struct Map<phantom V> has drop {}
+ 4 │ │
+   · │
+11 │ │     }
+12 │ │ }
+   │ ╰─^
diff --git a/...d_party/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.move b/...d_party/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.move
@@ -0,0 +1,12 @@
+module 0x815::red_black_map {
+
+    struct Map<phantom V> has drop {}
+
+    public fun contains<V>(self: &Map<V>, key: u256): bool {
+        self.search(key)
+    }
+
+    fun search<V: drop>(self: &Map<V>, key: u256): bool {
+        true
+    }
+}
diff --git a/...arty/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.opt.exp b/...arty/move/move-compiler-v2/tests/file-format-generator/bug_14813_fun_call_ability.opt.exp
@@ -0,0 +1,46 @@
+
+Diagnostics:
+warning: Unused parameter `key`. Consider removing or prefixing with an underscore: `_key`
+  ┌─ tests/file-format-generator/bug_14813_fun_call_ability.move:9:40
+  │
+9 │     fun search<V: drop>(self: &Map<V>, key: u256): bool {
+  │                                        ^^^
+
+
+============ disassembled file-format ==================
+// Move bytecode v7
+module 815.red_black_map {
+struct Map<phantom Ty0> has drop {
+	dummy_field: bool
+}
+
+public contains<Ty0>(Arg0: &Map<Ty0>, Arg1: u256): bool /* def_idx: 0 */ {
+B0:
+	0: MoveLoc[0](Arg0: &Map<Ty0>)
+	1: MoveLoc[1](Arg1: u256)
+	2: Call search<Ty0>(&Map<Ty0>, u256): bool
+	3: Ret
+}
+search<Ty0: drop>(Arg0: &Map<Ty0>, Arg1: u256): bool /* def_idx: 1 */ {
+B0:
+	0: LdTrue
+	1: MoveLoc[0](Arg0: &Map<Ty0>)
+	2: Pop
+	3: Ret
+}
+}
+============ bytecode verification failed ========
+
+Diagnostics:
+bug: bytecode verification failed with unexpected status code `CONSTRAINT_NOT_SATISFIED`. This is a compiler bug, consider reporting it.
+Error message: missing abilities for `CallGeneric(0)` at code offset 2
+   ┌─ tests/file-format-generator/bug_14813_fun_call_ability.move:1:1
+   │
+ 1 │ ╭ module 0x815::red_black_map {
+ 2 │ │
+ 3 │ │     struct Map<phantom V> has drop {}
+ 4 │ │
+   · │
+11 │ │     }
+12 │ │ }
+   │ ╰─^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_12817_order_dependency.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_12817_order_dependency.old.exp
@@ -3,6 +3,7 @@
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `READREF_EXISTS_MUTABLE_BORROW_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
    ┌─ tests/reference-safety/bug_12817_order_dependency.move:14:9
    │
 14 │         *r2 + *r1 // <- changed order here

diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_13149.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_13149.old.exp
@@ -3,6 +3,7 @@
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `WRITEREF_EXISTS_BORROW_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
   ┌─ tests/reference-safety/bug_13149.move:7:9
   │
 7 │         *r2 = 2;

diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/bug_13976.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/bug_13976.old.exp
@@ -3,6 +3,7 @@
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `CALL_BORROWED_MUTABLE_REFERENCE_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
    ┌─ tests/reference-safety/bug_13976.move:12:13
    │
 12 │             lifted_lambda(&mut a, z))

diff --git a/...iler-v2/tests/reference-safety/v1-borrow-tests/mutable_borrow_local_twice_invalid.old.exp b/...iler-v2/tests/reference-safety/v1-borrow-tests/mutable_borrow_local_twice_invalid.old.exp
@@ -3,6 +3,7 @@
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `WRITEREF_EXISTS_BORROW_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
   ┌─ tests/reference-safety/v1-borrow-tests/mutable_borrow_local_twice_invalid.move:6:9
   │
 6 │         *r2 = 2;

diff --git a/...e/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp b/...e/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp
@@ -3,6 +3,7 @@
 
 Diagnostics:
 bug: bytecode verification failed with unexpected status code `WRITEREF_EXISTS_BORROW_ERROR`. This is a compiler bug, consider reporting it.
+Error message: none
    ┌─ tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.move:10:18
    │
 10 │         *g_mut = G { v: 0 };