[move] Remove unnecessary code (MystenLabs#11087)

## Description 

- Remove unnecessary checks in interpreter

## Test Plan 

- Added tests

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

crates/sui-adapter-transactional-tests/tests/runtime_behavior/versioned_check_swap_loc_new.exp

@@ -0,0 +1,14 @@
+processed 4 tasks
+
+task 1 'publish'. lines 8-42:
+created: object(1,0)
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 4491600,  storage_rebate: 0, non_refundable_storage_fee: 0
+
+task 2 'run'. lines 44-44:
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 988000,  storage_rebate: 978120, non_refundable_storage_fee: 9880
+
+task 3 'run'. lines 46-46:
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 988000,  storage_rebate: 978120, non_refundable_storage_fee: 9880

crates/sui-adapter-transactional-tests/tests/runtime_behavior/versioned_check_swap_loc_new.move

@@ -0,0 +1,46 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+// test old behavior of invariant violation
+
+//# init --addresses test=0x0
+
+//# publish
+module test::m {
+    public fun t1(cond: bool) {
+        let x: vector<u64>;
+        let r: &vector<u64>;
+        if (cond) {
+            x = vector[];
+            r = &x;
+            // use r in ways to disable optimizations or moving
+            id_ref(r);
+            id_ref(copy r);
+        };
+        x = vector[];
+        // use x in ways to disable optimizations or moving
+        id(x);
+        id(x);
+        return
+    }
+
+    public fun t2(cond: bool) {
+        let x: vector<u64> = vector[];
+        let r: &vector<u64>;
+        if (cond) {
+            r = &x;
+            // use r in ways to disable optimizations or moving
+            id_ref(r);
+            id_ref(copy r);
+        };
+        _ = move x;
+        return
+    }
+
+    fun id<T>(x: T): T { x }
+    fun id_ref<T>(x: &T): &T { x }
+}
+
+//# run test::m::t1 --args true
+
+//# run test::m::t2 --args true

crates/sui-adapter-transactional-tests/tests/runtime_behavior/versioned_check_swap_loc_old.exp

@@ -0,0 +1,14 @@
+processed 4 tasks
+
+task 1 'publish'. lines 8-42:
+created: object(1,0)
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 4491600,  storage_rebate: 0, non_refundable_storage_fee: 0
+
+task 2 'run'. lines 44-44:
+Error: Transaction Effects Status: MOVE VM INVARIANT VIOLATION.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMInvariantViolation, source: Some(VMError { major_status: UNKNOWN_INVARIANT_VIOLATION_ERROR, sub_status: None, message: Some("moving container with dangling references"), exec_state: Some(ExecutionState { stack_trace: [] }), location: Module(ModuleId { address: test, name: Identifier("m") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 13)] }), command: Some(0) } }
+
+task 3 'run'. lines 46-46:
+Error: Transaction Effects Status: MOVE VM INVARIANT VIOLATION.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMInvariantViolation, source: Some(VMError { major_status: UNKNOWN_INVARIANT_VIOLATION_ERROR, sub_status: None, message: Some("moving container with dangling references"), exec_state: Some(ExecutionState { stack_trace: [] }), location: Module(ModuleId { address: test, name: Identifier("m") }), indices: [], offsets: [(FunctionDefinitionIndex(1), 12)] }), command: Some(0) } }

crates/sui-adapter-transactional-tests/tests/runtime_behavior/versioned_check_swap_loc_old.move

@@ -0,0 +1,46 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+// test old behavior of invariant violation
+
+//# init --protocol_version 6 --addresses test=0x0
+
+//# publish
+module test::m {
+    public fun t1(cond: bool) {
+        let x: vector<u64>;
+        let r: &vector<u64>;
+        if (cond) {
+            x = vector[];
+            r = &x;
+            // use r in ways to disable optimizations or moving
+            id_ref(r);
+            id_ref(copy r);
+        };
+        x = vector[];
+        // use x in ways to disable optimizations or moving
+        id(x);
+        id(x);
+        return
+    }
+
+    public fun t2(cond: bool) {
+        let x: vector<u64> = vector[];
+        let r: &vector<u64>;
+        if (cond) {
+            r = &x;
+            // use r in ways to disable optimizations or moving
+            id_ref(r);
+            id_ref(copy r);
+        };
+        _ = move x;
+        return
+    }
+
+    fun id<T>(x: T): T { x }
+    fun id_ref<T>(x: &T): &T { x }
+}
+
+//# run test::m::t1 --args true
+
+//# run test::m::t2 --args true

crates/sui-adapter/src/adapter.rs

@@ -110,6 +110,8 @@ pub fn new_move_vm(
             runtime_limits_config: VMRuntimeLimitsConfig {
                 vector_len_max: protocol_config.max_move_vector_len(),
             },
+            enable_invariant_violation_check_in_swap_loc:
+                !protocol_config.disable_invariant_violation_check_in_swap_loc(),
         },
     )
     .map_err(|_| SuiError::ExecutionInvariantViolation)

crates/sui-protocol-config/src/lib.rs

@@ -148,6 +148,9 @@ struct FeatureFlags {
     // Disallow adding `key` ability to types during package upgrades.
     #[serde(skip_serializing_if = "is_false")]
     disallow_adding_key_ability: bool,
+    // Disables unnecessary invariant check in the Move VM when swapping the value out of a local
+    #[serde(skip_serializing_if = "is_false")]
+    disable_invariant_violation_check_in_swap_loc: bool,
 }
 
 fn is_false(b: &bool) -> bool {
@@ -641,6 +644,11 @@ impl ProtocolConfig {
     pub fn disallow_adding_key_ability(&self) -> bool {
         self.feature_flags.disallow_adding_key_ability
     }
+
+    pub fn disable_invariant_violation_check_in_swap_loc(&self) -> bool {
+        self.feature_flags
+            .disable_invariant_violation_check_in_swap_loc
+    }
 }
 
 // Special getters
@@ -1042,6 +1050,8 @@ impl ProtocolConfig {
             7 => {
                 let mut cfg = Self::get_for_version_impl(version - 1);
                 cfg.feature_flags.disallow_adding_key_ability = true;
+                cfg.feature_flags
+                    .disable_invariant_violation_check_in_swap_loc = true;
                 cfg
             }
             // Use this template when making changes:

crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_7.snap

@@ -11,6 +11,7 @@ feature_flags:
   scoring_decision_with_validity_cutoff: true
   consensus_order_end_of_epoch_last: true
   disallow_adding_key_ability: true
+  disable_invariant_violation_check_in_swap_loc: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

crates/sui-transactional-test-runner/src/test_adapter.rs

@@ -372,7 +372,6 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter<'a> {
             upgradeable,
             dependencies,
         } = extra;
-
         let named_addr_opt = modules.first().unwrap().0;
         let first_module_name = modules.first().unwrap().1.self_id().name().to_string();
         let modules_bytes = modules

external-crates/move/move-vm/runtime/src/config.rs

@@ -12,6 +12,8 @@ pub struct VMConfig {
     // execution to ensure that type safety cannot be violated at runtime.
     pub paranoid_type_checks: bool,
     pub runtime_limits_config: VMRuntimeLimitsConfig,
+    // When this flag is set to true, MoveVM will check invariant violation in swap_loc
+    pub enable_invariant_violation_check_in_swap_loc: bool,
 }
 
 impl Default for VMConfig {
@@ -21,6 +23,7 @@ impl Default for VMConfig {
             max_binary_format_version: VERSION_MAX,
             paranoid_type_checks: false,
             runtime_limits_config: VMRuntimeLimitsConfig::default(),
+            enable_invariant_violation_check_in_swap_loc: true,
         }
     }
 }

external-crates/move/move-vm/runtime/src/interpreter.rs

@@ -163,7 +163,13 @@ impl Interpreter {
         let mut locals = Locals::new(function.local_count());
         for (i, value) in args.into_iter().enumerate() {
             locals
-                .store_loc(i, value)
+                .store_loc(
+                    i,
+                    value,
+                    loader
+                        .vm_config()
+                        .enable_invariant_violation_check_in_swap_loc,
+                )
                 .map_err(|e| self.set_location(e))?;
         }
 
@@ -317,7 +323,13 @@ impl Interpreter {
         let arg_count = func.arg_count();
         let is_generic = !ty_args.is_empty();
         for i in 0..arg_count {
-            locals.store_loc(arg_count - i - 1, self.operand_stack.pop()?)?;
+            locals.store_loc(
+                arg_count - i - 1,
+                self.operand_stack.pop()?,
+                loader
+                    .vm_config()
+                    .enable_invariant_violation_check_in_swap_loc,
+            )?;
 
             if self.paranoid_type_checks {
                 let ty = self.operand_stack.pop_ty()?;
@@ -1841,15 +1853,28 @@ impl Frame {
                         interpreter.operand_stack.push(local)?;
                     }
                     Bytecode::MoveLoc(idx) => {
-                        let local = self.locals.move_loc(*idx as usize)?;
+                        let local = self.locals.move_loc(
+                            *idx as usize,
+                            resolver
+                                .loader()
+                                .vm_config()
+                                .enable_invariant_violation_check_in_swap_loc,
+                        )?;
                         gas_meter.charge_move_loc(&local)?;
 
                         interpreter.operand_stack.push(local)?;
                     }
                     Bytecode::StLoc(idx) => {
                         let value_to_store = interpreter.operand_stack.pop()?;
                         gas_meter.charge_store_loc(&value_to_store)?;
-                        self.locals.store_loc(*idx as usize, value_to_store)?;
+                        self.locals.store_loc(
+                            *idx as usize,
+                            value_to_store,
+                            resolver
+                                .loader()
+                                .vm_config()
+                                .enable_invariant_violation_check_in_swap_loc,
+                        )?;
                     }
                     Bytecode::Call(idx) => {
                         return Ok(ExitCode::Call(*idx));

external-crates/move/move-vm/runtime/src/runtime.rs

@@ -232,7 +232,13 @@ impl VMRuntime {
             .enumerate()
             .map(|(idx, (arg_ty, arg_bytes))| match &arg_ty {
                 Type::MutableReference(inner_t) | Type::Reference(inner_t) => {
-                    dummy_locals.store_loc(idx, self.deserialize_value(inner_t, arg_bytes)?)?;
+                    dummy_locals.store_loc(
+                        idx,
+                        self.deserialize_value(inner_t, arg_bytes)?,
+                        self.loader
+                            .vm_config()
+                            .enable_invariant_violation_check_in_swap_loc,
+                    )?;
                     dummy_locals.borrow_loc(idx)
                 }
                 _ => self.deserialize_value(&arg_ty, arg_bytes),
@@ -345,7 +351,12 @@ impl VMRuntime {
             .into_iter()
             .map(|(idx, ty)| {
                 // serialize return values first in the case that a value points into this local
-                let local_val = dummy_locals.move_loc(idx)?;
+                let local_val = dummy_locals.move_loc(
+                    idx,
+                    self.loader
+                        .vm_config()
+                        .enable_invariant_violation_check_in_swap_loc,
+                )?;
                 let (bytes, layout) = self.serialize_return_value(&ty, local_val)?;
                 Ok((idx as LocalIndex, bytes, layout))
             })

external-crates/move/move-vm/transactional-tests/tests/instructions/inaccessible_borrowed_local.exp

@@ -0,0 +1 @@
+processed 2 tasks

external-crates/move/move-vm/transactional-tests/tests/instructions/inaccessible_borrowed_local.mvir

@@ -0,0 +1,27 @@
+//# run
+main() {
+    let x: vector<u64>;
+    let r: &vector<u64>;
+    label start:
+        jump_if_false (true) end;
+    label if_true:
+        x = vec_pack_0<u64>();
+        r = &x;
+    label end:
+        x = vec_pack_0<u64>();
+        return;
+}
+
+//# run
+main() {
+    let x: vector<u64>;
+    let r: &vector<u64>;
+    label start:
+        x = vec_pack_0<u64>();
+        jump_if_false (true) end;
+    label if_true:
+        r = &x;
+    label end:
+        _ = move(x);
+        return;
+}

external-crates/move/move-vm/types/src/values/values_impl.rs

@@ -995,16 +995,20 @@ impl Locals {
         }
     }
 
-    fn swap_loc(&mut self, idx: usize, x: Value) -> PartialVMResult<Value> {
+    fn swap_loc(&mut self, idx: usize, x: Value, violation_check: bool) -> PartialVMResult<Value> {
         let mut v = self.0.borrow_mut();
         match v.get_mut(idx) {
             Some(v) => {
-                if let ValueImpl::Container(c) = v {
-                    if c.rc_count() > 1 {
-                        return Err(PartialVMError::new(
-                            StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
-                        )
-                        .with_message("moving container with dangling references".to_string()));
+                if violation_check {
+                    if let ValueImpl::Container(c) = v {
+                        if c.rc_count() > 1 {
+                            return Err(PartialVMError::new(
+                                StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
+                            )
+                            .with_message(
+                                "moving container with dangling references".to_string(),
+                            ));
+                        }
                     }
                 }
                 Ok(Value(std::mem::replace(v, x.0)))
@@ -1017,8 +1021,8 @@ impl Locals {
         }
     }
 
-    pub fn move_loc(&mut self, idx: usize) -> PartialVMResult<Value> {
-        match self.swap_loc(idx, Value(ValueImpl::Invalid))? {
+    pub fn move_loc(&mut self, idx: usize, violation_check: bool) -> PartialVMResult<Value> {
+        match self.swap_loc(idx, Value(ValueImpl::Invalid), violation_check)? {
             Value(ValueImpl::Invalid) => Err(PartialVMError::new(
                 StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
             )
@@ -1027,8 +1031,13 @@ impl Locals {
         }
     }
 
-    pub fn store_loc(&mut self, idx: usize, x: Value) -> PartialVMResult<()> {
-        self.swap_loc(idx, x)?;
+    pub fn store_loc(
+        &mut self,
+        idx: usize,
+        x: Value,
+        violation_check: bool,
+    ) -> PartialVMResult<()> {
+        self.swap_loc(idx, x, violation_check)?;
         Ok(())
     }
 

external-crates/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs

@@ -416,6 +416,7 @@ impl From<AdapterExecuteArgs> for VMConfig {
     fn from(arg: AdapterExecuteArgs) -> VMConfig {
         VMConfig {
             paranoid_type_checks: arg.check_runtime_types,
+            enable_invariant_violation_check_in_swap_loc: false,
             ..Default::default()
         }
     }