Stop stackless bytecode pipeline when errors are found by a processor

aptos-labs · Sep 5, 2024 · 4558e27 · 4558e27
1 parent 9eb867d
commit 4558e27
Show file tree

Hide file tree

Showing 41 changed files with 112 additions and 232 deletions.
diff --git a/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/diamond_clicker.v2_exp b/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/diamond_clicker.v2_exp
@@ -33,26 +33,26 @@ B0:
 	9: MutBorrowField[0](OuterStruct.any_field: vector<InnerStruct>)
 	10: StLoc[2](loc1: &mut vector<InnerStruct>)
 	11: LdU64(0)
-	12: StLoc[3](loc2: u64)
-	13: CopyLoc[2](loc1: &mut vector<InnerStruct>)
-	14: FreezeRef
-	15: VecLen(3)
+	12: CopyLoc[2](loc1: &mut vector<InnerStruct>)
+	13: FreezeRef
+	14: VecLen(3)
+	15: StLoc[3](loc2: u64)
 	16: StLoc[4](loc3: u64)
 B1:
-	17: CopyLoc[3](loc2: u64)
-	18: CopyLoc[4](loc3: u64)
+	17: CopyLoc[4](loc3: u64)
+	18: CopyLoc[3](loc2: u64)
 	19: Lt
 	20: BrFalse(31)
 B2:
 	21: CopyLoc[2](loc1: &mut vector<InnerStruct>)
-	22: CopyLoc[3](loc2: u64)
+	22: CopyLoc[4](loc3: u64)
 	23: VecMutBorrow(3)
 	24: FreezeRef
 	25: Call debug::print<InnerStruct>(&InnerStruct)
-	26: MoveLoc[3](loc2: u64)
+	26: MoveLoc[4](loc3: u64)
 	27: LdU64(1)
 	28: Add
-	29: StLoc[3](loc2: u64)
+	29: StLoc[4](loc3: u64)
 	30: Branch(34)
 B3:
 	31: MoveLoc[2](loc1: &mut vector<InnerStruct>)

diff --git a/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/table.v2_exp b/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/table.v2_exp
@@ -52,7 +52,7 @@ Events:
 }
 {
     type:    0x1::transaction_fee::FeeStatement
-    data:    "9c0000000000000006000000000000000400000000000000c83a0200000000000000000000000000"
+    data:    "9c0000000000000007000000000000000400000000000000c83a0200000000000000000000000000"
 }mutable inputs after call: local#0: 0
 return values: 0
 

diff --git a/third_party/move/evm/move-to-yul/src/context.rs b/third_party/move/evm/move-to-yul/src/context.rs
@@ -225,7 +225,7 @@ impl<'a> Context<'a> {
         pipeline.add_processor(ReachingDefProcessor::new());
         pipeline.add_processor(LiveVarAnalysisProcessor::new());
         if options.dump_bytecode {
-            pipeline.run_with_dump(env, &mut targets, &options.output, false, &|_| {})
+            pipeline.run_with_dump(env, &mut targets, &options.output, false, &|_| {}, || true)
         } else {
             pipeline.run(env, &mut targets);
         }

diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs
@@ -124,9 +124,10 @@ where
             &dump_base_name,
             false,
             &pipeline::register_formatters,
+            || !env.has_errors(),
         )
     } else {
-        pipeline.run(&env, &mut targets)
+        pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors())
     }
     check_errors(&env, error_writer, "stackless-bytecode analysis errors")?;
 

diff --git a/...arty/move/move-compiler-v2/src/pipeline/reference_safety/reference_safety_processor_v3.rs b/...arty/move/move-compiler-v2/src/pipeline/reference_safety/reference_safety_processor_v3.rs
@@ -16,6 +16,8 @@
 //! in the stack machine they are put on the stack in independent instructions.
 //! Another difference is the need for generating good error messages which
 //! the bytecode verifier has not.
+//!
+//! Prerequisites: there are no uninitialized locals.
 
 use crate::pipeline::{
     livevar_analysis_processor::{LiveVarAnnotation, LiveVarInfoAtCodeOffset},

diff --git a/third_party/move/move-compiler-v2/tests/ability-check/v1-borrow-tests/no_drop.exp b/third_party/move/move-compiler-v2/tests/ability-check/v1-borrow-tests/no_drop.exp
@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: local `x` of type `M::X` does not have the `drop` ability
+  ┌─ tests/ability-check/v1-borrow-tests/no_drop.move:6:9
+  │
+6 │         &x;
+  │         ^^ implicitly dropped here since it is no longer used
diff --git a/third_party/move/move-compiler-v2/tests/ability-check/v1-borrow-tests/no_drop.move b/third_party/move/move-compiler-v2/tests/ability-check/v1-borrow-tests/no_drop.move
@@ -0,0 +1,9 @@
+module 0x8675309::M {
+    struct X {}
+
+    fun t1(): bool {
+        let x = X {};
+        &x;
+        false
+    }
+}
diff --git a/...piler-v2/tests/ability-check/v1-borrow-tests/return_type_mismatch_and_unused_resource.exp b/...piler-v2/tests/ability-check/v1-borrow-tests/return_type_mismatch_and_unused_resource.exp
@@ -7,11 +7,3 @@ error: cannot return a reference derived from value since it is not based on a p
    │                 -- value previously borrowed here
 13 │         r
    │         ^ return attempted here
-
-
-Diagnostics:
-error: local `x` of type `M::X` does not have the `drop` ability
-  ┌─ tests/ability-check/v1-borrow-tests/return_type_mismatch_and_unused_resource.move:6:9
-  │
-6 │         &x;
-  │         ^^ implicitly dropped here since it is no longer used
diff --git a/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_2.exp b/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_2.exp
@@ -43,27 +43,3 @@ error: local `q` of type `m::S` does not have the `drop` ability
    │
 13 │         q.a
    │         ^ still borrowed but will be implicitly dropped later since it is no longer used
-
-============ after DeadStoreElimination: ================
-
-[variant baseline]
-fun m::test($t0: m::S): u64 {
-     var $t1: u64
-     var $t2: m::S
-     var $t3: m::S
-     var $t4: &mut u64
-     var $t5: &mut m::S
-     var $t6: u64
-     var $t7: &m::S
-     var $t8: &u64
-  0: $t2 := move($t0)
-  1: $t3 := copy($t2)
-  2: $t5 := borrow_local($t2)
-  3: $t4 := borrow_field<m::S>.a($t5)
-  4: $t6 := 0
-  5: write_ref($t4, $t6)
-  6: $t7 := borrow_local($t3)
-  7: $t8 := borrow_field<m::S>.a($t7)
-  8: $t1 := read_ref($t8)
-  9: return $t1
-}
diff --git a/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_3.exp b/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_3.exp
@@ -28,21 +28,3 @@ error: cannot write local `a` since it is borrowed
   │         ^^^^^^ write attempted here
 8 │         *c
   │         -- conflicting reference `c` used here
-
-============ after DeadStoreElimination: ================
-
-[variant baseline]
-fun m::test($t0: u64): u64 {
-     var $t1: u64
-     var $t2: &mut u64
-     var $t3: &mut u64
-     var $t4: &mut u64
-     var $t5: u64
-  0: $t2 := borrow_local($t0)
-  1: $t3 := copy($t2)
-  2: $t4 := move($t3)
-  3: $t5 := 0
-  4: write_ref($t2, $t5)
-  5: $t1 := read_ref($t4)
-  6: return $t1
-}
diff --git a/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_4.exp b/third_party/move/move-compiler-v2/tests/copy-propagation/mut_refs_4.exp
@@ -30,23 +30,3 @@ error: cannot copy local `p` which is still mutably borrowed
   ·
 8 │         *a = 0;
   │         ------ conflicting reference `a` used here
-
-============ after DeadStoreElimination: ================
-
-[variant baseline]
-fun m::test($t0: u64): u64 {
-     var $t1: u64
-     var $t2: &mut u64
-     var $t3: u64
-     var $t4: u64
-     var $t5: u64
-     var $t6: u64
-  0: $t2 := borrow_local($t0)
-  1: $t3 := copy($t0)
-  2: $t4 := move($t3)
-  3: $t5 := move($t4)
-  4: $t6 := 0
-  5: write_ref($t2, $t6)
-  6: $t1 := move($t5)
-  7: return $t1
-}
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.exp b/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.exp
@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: use of unassigned local `y`
+  ┌─ tests/reference-safety/unused_reference.move:6:9
+  │
+6 │         *y;
+  │         ^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.move b/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.move
@@ -0,0 +1,8 @@
+module 0xCAFE::Module0 {
+    struct S has copy, drop {f:u64}
+
+    public fun function0() {
+        let y: &S;
+        *y;
+    }
+}
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.no-opt.exp b/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.no-opt.exp
@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: use of unassigned local `y`
+  ┌─ tests/reference-safety/unused_reference.move:6:9
+  │
+6 │         *y;
+  │         ^^
diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/unused_reference.old.exp
@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: use of unassigned local `y`
+  ┌─ tests/reference-safety/unused_reference.move:6:9
+  │
+6 │         *y;
+  │         ^^
diff --git a/...ler-v2/tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.exp b/...ler-v2/tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.exp
@@ -19,11 +19,3 @@ error: cannot drop local `s` which is still borrowed
    │         ^^^^^^^^^^^^^^ dropped here
 17 │         z;
    │         - conflicting reference `z` used here
-
-
-Diagnostics:
-error: local `x` of type `M::X` does not have the `drop` ability
-  ┌─ tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.move:6:17
-  │
-6 │         let y = &x;
-  │                 ^^ still borrowed but will be implicitly dropped later since it is no longer used
diff --git a/...tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.no-opt.exp b/...tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.no-opt.exp
@@ -19,11 +19,3 @@ error: cannot drop local `s` which is still borrowed
    │         ^^^^^^^^^^^^^^ dropped here
 17 │         z;
    │         - conflicting reference `z` used here
-
-
-Diagnostics:
-error: local `x` of type `M::X` does not have the `drop` ability
-  ┌─ tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.move:6:17
-  │
-6 │         let y = &x;
-  │                 ^^ still borrowed but will be implicitly dropped later since it is no longer used
diff --git a/...v2/tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.old.exp b/...v2/tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.old.exp
@@ -21,11 +21,3 @@ error: cannot assign to borrowed local `s`
    │         ^^^^^^^^^^^^^^ attempted to assign here
 17 │         z;
    │         - conflicting reference `z` used here
-
-
-Diagnostics:
-error: local `x` of type `M::X` does not have the `drop` ability
-  ┌─ tests/reference-safety/v1-borrow-tests/mutate_with_borrowed_loc_struct_invalid.move:6:17
-  │
-6 │         let y = &x;
-  │                 ^^ still borrowed but will be implicitly dropped later since it is no longer used
diff --git a/third_party/move/move-compiler-v2/tests/simplifier-elimination/use_before_assign.exp b/third_party/move/move-compiler-v2/tests/simplifier-elimination/use_before_assign.exp
@@ -26,11 +26,3 @@ error: use of unassigned local `x`
   │
 4 │     let y = x;
   │             ^
-
-
-Diagnostics:
-warning: Unused assignment to `y`. Consider removing or prefixing with an underscore: `_y`
-  ┌─ tests/simplifier-elimination/use_before_assign.move:4:13
-  │
-4 │     let y = x;
-  │             ^
diff --git a/third_party/move/move-compiler-v2/tests/simplifier-elimination/use_before_assign_while.exp b/third_party/move/move-compiler-v2/tests/simplifier-elimination/use_before_assign_while.exp
@@ -103,22 +103,3 @@ error: use of unassigned local `x`
    │
 19 │         while (cond) { let y = &x; _ = move y; if (cond) { x = 0 }; break }
    │                                ^^
-
-
-Diagnostics:
-warning: Unused assignment to `x`. Consider removing or prefixing with an underscore: `_x`
-   ┌─ tests/simplifier-elimination/use_before_assign_while.move:19:60
-   │
-19 │         while (cond) { let y = &x; _ = move y; if (cond) { x = 0 }; break }
-   │                                                            ^^^^^
-
-
-Diagnostics:
-error: cannot move local `x` since it is still in use
-  ┌─ tests/simplifier-elimination/use_before_assign_while.move:9:32
-  │
-9 │         while (cond) { let y = move x + 1; if (cond) { continue }; x = 0; y; }
-  │                                ^^^^^^
-  │                                │
-  │                                attempted to move here
-  │                                used here
diff --git a/third_party/move/move-compiler-v2/tests/testsuite.rs b/third_party/move/move-compiler-v2/tests/testsuite.rs
@@ -745,7 +745,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> {
         }
     }
 
-    if options.compile_test_code {
+    if ok && options.compile_test_code {
         // Build the test plan here to parse and validate any test-related attributes in the AST.
         // In real use, this is run outside of the compilation process, but the needed info is
         // available in `env` once we finish the AST.
@@ -810,6 +810,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> {
                         out.push_str(dump);
                         debug!("{}", dump);
                     }
+                    *ok.borrow()
                 },
             );
             if *ok.borrow() && config.stop_after == StopAfter::FileFormat {

diff --git a/third_party/move/move-compiler-v2/tests/uninit-use-checker/unused_reference.exp b/third_party/move/move-compiler-v2/tests/uninit-use-checker/unused_reference.exp
@@ -0,0 +1,29 @@
+============ initial bytecode ================
+
+[variant baseline]
+public fun Module0::function0() {
+     var $t0: &Module0::S
+     var $t1: Module0::S
+  0: $t1 := read_ref($t0)
+  1: return ()
+}
+
+
+Diagnostics:
+error: use of unassigned local `y`
+  ┌─ tests/uninit-use-checker/unused_reference.move:6:9
+  │
+6 │         *y;
+  │         ^^
+
+============ after uninitialized_use_checker: ================
+
+[variant baseline]
+public fun Module0::function0() {
+     var $t0: &Module0::S
+     var $t1: Module0::S
+     # before: { no: $t0, $t1 }, after: { no: $t0 }
+  0: $t1 := read_ref($t0)
+     # before: { no: $t0 }, after: { no: $t0 }
+  1: return ()
+}
diff --git a/third_party/move/move-compiler-v2/tests/uninit-use-checker/unused_reference.move b/third_party/move/move-compiler-v2/tests/uninit-use-checker/unused_reference.move
@@ -0,0 +1,8 @@
+module 0xCAFE::Module0 {
+    struct S has copy, drop {f:u64}
+
+    public fun function0() {
+        let y: &S;
+        *y;
+    }
+}
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/attribute_location_invalid.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/attribute_location_invalid.exp
@@ -23,6 +23,3 @@ error: invalid attribute
    │       │
    │       Attribute 'expected_failure' is not expected with a struct
    │       Expected to be used with one of the following: function
-
-
-============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/expected_failure_on_non_function.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/expected_failure_on_non_function.exp
@@ -26,6 +26,3 @@ error: invalid attribute
   │       │
   │       Attribute 'expected_failure' is not expected with a constant
   │       Expected to be used with one of the following: function
-
-
-============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/extra_attributes2.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/extra_attributes2.exp
@@ -23,6 +23,3 @@ error: invalid attribute
    │
 21 │     #[expected_failure(verify_only)]
    │                        ^^^^^^^^^^^ Attribute 'verify_only' is not expected in a nested attribute position
-
-
-============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/function_with_spec.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/function_with_spec.exp
@@ -5,6 +5,3 @@ error: unresolved spec target
    │
 18 │     spec baz {
    │          ^^^
-
-
-============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/multiple_errors.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/multiple_errors.exp
@@ -15,6 +15,3 @@ error: duplicate declaration, item, or annotation
    │       ---------------- Attribute previously given here
 42 │     #[expected_failure]
    │       ^^^^^^^^^^^^^^^^ Duplicate attribute 'expected_failure' attached to the same item
-
-
-============ bytecode verification succeeded ========
diff --git a/third_party/move/move-compiler-v2/tests/unit_test/test/multiple_test_annotations.exp b/third_party/move/move-compiler-v2/tests/unit_test/test/multiple_test_annotations.exp
@@ -15,6 +15,3 @@ error: duplicate declaration, item, or annotation
    │       ---- Attribute previously given here
 10 │     #[test(_a=@0x1, _b=@0x2)]
    │       ^^^^^^^^^^^^^^^^^^^^^^ Duplicate attribute 'test' attached to the same item
-
-
-============ bytecode verification succeeded ========
diff --git a/...party/move/move-compiler-v2/tests/unit_test/test/named_address_no_value_in_annotation.exp b/...party/move/move-compiler-v2/tests/unit_test/test/named_address_no_value_in_annotation.exp
@@ -5,6 +5,3 @@ error: address with no value
   │
 2 │     #[test(_a = @UnboundAddr)]
   │                  ^^^^^^^^^^^ address 'UnboundAddr' is not assigned a value. Try assigning it a value when calling the compiler
-
-
-============ bytecode verification succeeded ========
diff --git a/...rty/move/move-compiler-v2/tests/unit_test/test/other_failures_invalid_location_module.exp b/...rty/move/move-compiler-v2/tests/unit_test/test/other_failures_invalid_location_module.exp
@@ -5,6 +5,3 @@ error: unbound module
   │
 6 │     #[expected_failure(arithmetic_error, location=0x2::m)]
   │                                                   ^^^^^^ Unbound module '0x2::m'
-
-
-============ bytecode verification succeeded ========