[move-vm] Removed most RCs from runtime

- RCs are no longer necessary due to Move's reference safety checks - Some RCs are still needed to update clean/dirty status or hold onto external memory safely
diem · Mar 29, 2022 · 4163634 · 4163634
1 parent ac094f2
commit 4163634
Show file tree

Hide file tree

Showing 7 changed files with 534 additions and 770 deletions.
diff --git a/language/move-stdlib/src/natives/bcs.rs b/language/move-stdlib/src/natives/bcs.rs
@@ -29,7 +29,7 @@ pub fn native_to_bytes(
     // delegate to the BCS serialization for `Value`
     let serialized_value_opt = match context.type_to_type_layout(&arg_type)? {
         None => None,
-        Some(layout) => ref_to_val.read_ref()?.simple_serialize(&layout),
+        Some(layout) => ref_to_val.read_ref().simple_serialize(&layout),
     };
     let serialized_value = match serialized_value_opt {
         None => {

diff --git a/language/move-vm/runtime/src/interpreter.rs b/language/move-vm/runtime/src/interpreter.rs
@@ -906,15 +906,19 @@ impl Frame {
                     }
                     Bytecode::ReadRef => {
                         let reference = interpreter.operand_stack.pop_as::<Reference>()?;
-                        let value = reference.read_ref()?;
+                        let value = reference.read_ref();
                         gas_status.charge_instr_with_size(Opcodes::READ_REF, value.size())?;
                         interpreter.operand_stack.push(value)?;
                     }
                     Bytecode::WriteRef => {
                         let reference = interpreter.operand_stack.pop_as::<Reference>()?;
                         let value = interpreter.operand_stack.pop()?;
                         gas_status.charge_instr_with_size(Opcodes::WRITE_REF, value.size())?;
-                        reference.write_ref(value)?;
+                        // safe due as long as the code being executed passed the reference
+                        // safety checks
+                        unsafe {
+                            reference.write_ref(value)?;
+                        }
                     }
                     Bytecode::CastU8 => {
                         gas_status.charge_instr(Opcodes::CAST_U8)?;
@@ -1099,7 +1103,7 @@ impl Frame {
                         let addr = signer_reference
                             .borrow_field(0)?
                             .value_as::<Reference>()?
-                            .read_ref()?
+                            .read_ref()
                             .value_as::<AccountAddress>()?;
                         let ty = resolver.get_struct_type(*sd_idx);
                         // REVIEW: Can we simplify Interpreter::move_to?
@@ -1112,7 +1116,7 @@ impl Frame {
                         let addr = signer_reference
                             .borrow_field(0)?
                             .value_as::<Reference>()?
-                            .read_ref()?
+                            .read_ref()
                             .value_as::<AccountAddress>()?;
                         let ty = resolver.instantiate_generic_type(*si_idx, self.ty_args())?;
                         let size = interpreter.move_to(data_store, addr, &ty, resource)?;

diff --git a/language/move-vm/runtime/src/lib.rs b/language/move-vm/runtime/src/lib.rs
@@ -1,8 +1,6 @@
 // Copyright (c) The Diem Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-#![forbid(unsafe_code)]
-
 //! The core Move VM logic.
 //!
 //! It is a design goal for the Move VM to be independent of the Diem blockchain, so that

diff --git a/language/move-vm/types/src/lib.rs b/language/move-vm/types/src/lib.rs
@@ -1,8 +1,6 @@
 // Copyright (c) The Diem Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-#![forbid(unsafe_code)]
-
 macro_rules! debug_write {
     ($($toks: tt)*) => {
         write!($($toks)*).map_err(|_|

diff --git a/language/move-vm/types/src/values/value_tests.rs b/language/move-vm/types/src/values/value_tests.rs
@@ -17,7 +17,7 @@ fn locals() -> PartialVMResult<()> {
 
     assert!(locals.copy_loc(1)?.equals(&Value::u64(42))?);
     let r = locals.borrow_loc(1)?.value_as::<Reference>()?;
-    assert!(r.read_ref()?.equals(&Value::u64(42))?);
+    assert!(r.read_ref().equals(&Value::u64(42))?);
     assert!(locals.move_loc(1)?.equals(&Value::u64(42))?);
 
     assert!(locals.copy_loc(1).is_err());
@@ -56,17 +56,17 @@ fn struct_borrow_field() -> PartialVMResult<()> {
 
     {
         let f: Reference = r.borrow_field(1)?.value_as()?;
-        assert!(f.read_ref()?.equals(&Value::bool(false))?);
+        assert!(f.read_ref().equals(&Value::bool(false))?);
     }
 
-    {
+    unsafe {
         let f: Reference = r.borrow_field(1)?.value_as()?;
         f.write_ref(Value::bool(true))?;
     }
 
     {
         let f: Reference = r.borrow_field(1)?.value_as()?;
-        assert!(f.read_ref()?.equals(&Value::bool(true))?);
+        assert!(f.read_ref().equals(&Value::bool(true))?);
     }
 
     Ok(())
@@ -89,21 +89,21 @@ fn struct_borrow_nested() -> PartialVMResult<()> {
 
     {
         let r3: Reference = r2.borrow_field(0)?.value_as()?;
-        assert!(r3.read_ref()?.equals(&Value::u64(20))?);
+        assert!(r3.read_ref().equals(&Value::u64(20))?);
     }
 
-    {
+    unsafe {
         let r3: Reference = r2.borrow_field(0)?.value_as()?;
         r3.write_ref(Value::u64(30))?;
     }
 
     {
         let r3: Reference = r2.borrow_field(0)?.value_as()?;
-        assert!(r3.read_ref()?.equals(&Value::u64(30))?);
+        assert!(r3.read_ref().equals(&Value::u64(30))?);
     }
 
-    assert!(r2.read_ref()?.equals(&inner(30))?);
-    assert!(r1.read_ref()?.equals(&outer(30))?);
+    assert!(r2.into_ref().read_ref().equals(&inner(30))?);
+    assert!(r1.into_ref().read_ref().equals(&outer(30))?);
 
     Ok(())
 }