Swap *const Function to ArenaPointer everywhere, minor arena.rs reorg

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

@@ -4,7 +4,7 @@
 
 use crate::{
     loader::{
-        arena,
+        arena::{self, ArenaPointer},
         ast::{Bytecode, Function},
         Loader, Resolver,
     },
@@ -108,7 +108,7 @@ impl Interpreter {
     /// Entrypoint into the interpreter. All external calls need to be routed through this
     /// function.
     pub(crate) fn entrypoint(
-        fun_ref: *const Function,
+        funtction: ArenaPointer<Function>,
         ty_args: Vec<Type>,
         args: Vec<Value>,
         data_store: &mut impl DataStore,
@@ -121,7 +121,7 @@ impl Interpreter {
             call_stack: CallStack::new(),
             runtime_limits_config: loader.vm_config().runtime_limits_config.clone(),
         };
-        let fun_ref = arena::to_ref(fun_ref);
+        let fun_ref = funtction.to_ref();
         profile_open_frame!(gas_meter, fun_ref.pretty_string());
 
         if fun_ref.is_native() {
@@ -203,7 +203,7 @@ impl Interpreter {
                     }
                 }
                 ExitCode::Call(function) => {
-                    let func = arena::to_ref(function);
+                    let func = function.to_ref();
                     #[cfg(feature = "gas-profiler")]
                     let func_name = func.pretty_string();
                     profile_open_frame!(gas_meter, func_name.clone());
@@ -765,7 +765,7 @@ struct Frame {
 #[derive(Debug)]
 enum ExitCode {
     Return,
-    Call(*const Function),
+    Call(ArenaPointer<Function>),
     CallGeneric(FunctionInstantiationIndex),
 }
 

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

@@ -1,16 +1,30 @@
 // Copyright (c) The Move Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-// Arena Definitions
-
 #![allow(unsafe_code)]
 
 use std::mem::MaybeUninit;
 
 use bumpalo::Bump;
 
+// -------------------------------------------------------------------------------------------------
+// ARENA DEFINITIONS
+// -------------------------------------------------------------------------------------------------
+
+// -----------------------------------------------
+// Types
+// -----------------------------------------------
+
 pub struct Arena(Bump);
 
+/// An Arena Pointer, which allows conversion to references and const. Equality is defined as
+/// pointer equality, and clone/copy are shallow.
+pub struct ArenaPointer<T>(*const T);
+
+// -----------------------------------------------
+// Impls
+// -----------------------------------------------
+
 impl Arena {
     pub fn new() -> Self {
         Arena(Bump::new())
@@ -26,12 +40,26 @@ impl Arena {
     }
 }
 
-// SAFETY: these are okay, if callers follow the documented safety requirements for `Arena`'s
-// unsafe methods.
-unsafe impl Send for Arena {}
-unsafe impl Sync for Arena {}
+impl<T> ArenaPointer<T> {
+    pub fn new(value: *const T) -> Self {
+        ArenaPointer(value)
+    }
+
+    #[allow(dead_code)]
+    pub fn to_const(self) -> *const T {
+        self.0
+    }
+
+    pub fn to_ref<'a>(self) -> &'a T {
+        to_ref(self.0)
+    }
+}
 
-/// Returns a pointer to a slice, but nulled. This must be set before use.
+// -----------------------------------------------
+// Pointer Operations
+// -----------------------------------------------
+
+///// Returns a pointer to a slice, but nulled. This must be set before use.
 pub fn null_ptr<T>() -> *const [T] {
     unsafe { MaybeUninit::<*const [T]>::zeroed().assume_init() }
 }
@@ -52,28 +80,38 @@ pub fn to_ref<'a, T>(value: *const T) -> &'a T {
     unsafe { &*value as &T }
 }
 
-#[derive(Clone, Copy)]
-pub struct ArenaPointer<T>(*const T);
+// -----------------------------------------------
+// Trait Implementations
+// -----------------------------------------------
 
-impl<T> ArenaPointer<T> {
-    pub fn new(value: *const T) -> Self {
-        ArenaPointer(value)
-    }
+// SAFETY: these are okay, if callers follow the documented safety requirements for `Arena`'s
+// unsafe methods.
 
-    pub fn to_const(&self) -> *const T {
-        self.0
-    }
+unsafe impl Send for Arena {}
+unsafe impl Sync for Arena {}
 
-    pub fn to_ref<'a>(&self) -> &'a T {
-        to_ref(self.0)
-    }
-}
+unsafe impl<T> Send for ArenaPointer<T> {}
+unsafe impl<T> Sync for ArenaPointer<T> {}
 
 impl<T: ::std::fmt::Debug> ::std::fmt::Debug for ArenaPointer<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "ptr->{:?}", to_ref(self.0))
     }
 }
 
-unsafe impl<T> Send for ArenaPointer<T> {}
-unsafe impl<T> Sync for ArenaPointer<T> {}
+// Pointer equality
+impl<T> PartialEq for ArenaPointer<T> {
+    fn eq(&self, other: &Self) -> bool {
+        std::ptr::eq(self.0, other.0)
+    }
+}
+
+impl<T> Eq for ArenaPointer<T> {}
+
+impl<T> Clone for ArenaPointer<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T> Copy for ArenaPointer<T> {}

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

@@ -214,7 +214,7 @@ pub(crate) enum DatatypeTagType {
 ///
 /// Bytecodes operate on a stack machine and each bytecode has side effect on the stack and the
 /// instruction stream.
-#[derive(Clone, Hash, Eq, PartialEq)]
+#[derive(Clone, Eq, PartialEq)]
 pub enum Bytecode {
     /// Pop and discard the value at the top of the stack.
     /// The value on the stack must be an copyable type.
@@ -336,7 +336,7 @@ pub enum Bytecode {
     ///
     /// ```..., arg(1), arg(2), ...,  arg(n) -> ..., return_value(1), return_value(2), ...,
     /// return_value(k)```
-    Call(*const Function),
+    Call(ArenaPointer<Function>),
     CallGeneric(FunctionInstantiationIndex),
     /// Create an instance of the type specified via `DatatypeHandleIndex` and push it on the stack.
     /// The values of the fields of the struct, in the order they appear in the struct declaration,
@@ -936,7 +936,7 @@ impl ::std::fmt::Debug for Bytecode {
             Bytecode::CopyLoc(a) => write!(f, "CopyLoc({})", a),
             Bytecode::MoveLoc(a) => write!(f, "MoveLoc({})", a),
             Bytecode::StLoc(a) => write!(f, "StLoc({})", a),
-            Bytecode::Call(a) => write!(f, "Call({})", arena::to_ref(*a).name),
+            Bytecode::Call(a) => write!(f, "Call({})", a.to_ref().name),
             Bytecode::CallGeneric(ndx) => write!(f, "CallGeneric({})", ndx),
             Bytecode::Pack(a) => write!(f, "Pack({})", a),
             Bytecode::PackGeneric(a) => write!(f, "PackGeneric({})", a),

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

@@ -2,6 +2,7 @@
 // Copyright (c) The Move Contributors
 // SPDX-License-Identifier: Apache-2.0
 
+#[allow(unsafe_code)]
 pub mod arena;
 pub mod ast;
 pub mod translate;
@@ -584,13 +585,13 @@ impl ModuleCache {
         func_name: &IdentStr,
         runtime_id: &ModuleId,
         link_context: AccountAddress,
-    ) -> PartialVMResult<*const Function> {
+    ) -> PartialVMResult<ArenaPointer<Function>> {
         match self
             .loaded_modules
             .get(&(link_context, runtime_id.clone()))
             .and_then(|module| module.function_map.get(func_name))
         {
-            Some(func_idx) => Ok(func_idx.to_ref()),
+            Some(func_idx) => Ok(*func_idx),
             None => Err(
                 PartialVMError::new(StatusCode::FUNCTION_RESOLUTION_FAILURE).with_message(format!(
                     "Cannot find {:?}::{:?} in cache for context {:?}",
@@ -849,7 +850,7 @@ impl Loader {
     ) -> VMResult<(
         Arc<CompiledModule>,
         Arc<LoadedModule>,
-        *const Function,
+        ArenaPointer<Function>,
         LoadedFunctionInstantiation,
     )> {
         let link_context = data_store.link_context();
@@ -860,7 +861,7 @@ impl Loader {
             .resolve_function_by_name(function_name, runtime_id, link_context)
             .map_err(|err| err.finish(Location::Undefined))?;
 
-        let fun_ref = arena::to_ref(function);
+        let fun_ref = function.to_ref();
 
         let parameters = compiled
             .signature_at(fun_ref.parameters)

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

@@ -28,7 +28,7 @@ struct Context<'a> {
     link_context: AccountAddress,
     cache: &'a ModuleCache,
     module: &'a CompiledModule,
-    function_map: HashMap<Identifier, *const Function>,
+    function_map: HashMap<Identifier, ArenaPointer<Function>>,
     single_signature_token_map: BTreeMap<SignatureIndex, Type>,
 }
 
@@ -77,7 +77,7 @@ pub fn module(
     let mut struct_instantiations = vec![];
     let mut enums = vec![];
     let mut enum_instantiations = vec![];
-    let mut function_refs: Vec<*const Function> = vec![];
+    let mut function_refs: Vec<ArenaPointer<Function>> = vec![];
     let mut function_instantiations = vec![];
     let mut field_handles = vec![];
     let mut field_instantiations: Vec<FieldInstantiation> = vec![];
@@ -172,7 +172,12 @@ pub fn module(
 
     let function_map = arena::mut_to_ref_slice(loaded_functions)
         .iter()
-        .map(|function| (function.name.clone(), function as *const Function))
+        .map(|function| {
+            (
+                function.name.clone(),
+                ArenaPointer::new(function as *const Function),
+            )
+        })
         .collect::<HashMap<_, _>>();
 
     if DEBUG_FLAGS.function_list_sizes {
@@ -200,7 +205,7 @@ pub fn module(
                     if DEBUG_FLAGS.function_resolution {
                         println!("pushing {}", function.to_ref().name.as_ident_str());
                     }
-                    function_refs.push(function.to_const());
+                    function_refs.push(*function);
                     break;
                 }
             }
@@ -257,7 +262,7 @@ pub fn module(
             cache,
         )?;
         function_instantiations.push(FunctionInstantiation {
-            handle: arena::ArenaPointer::new(handle),
+            handle,
             instantiation_idx,
         });
     }
@@ -287,10 +292,7 @@ pub fn module(
         function_instantiations,
         field_handles,
         field_instantiations,
-        function_map: function_map
-            .into_iter()
-            .map(|(key, value)| (key, arena::ArenaPointer::new(value)))
-            .collect::<HashMap<_, _>>(),
+        function_map,
         single_signature_token_map,
         instantiation_signatures,
         variant_handles: module.variant_handles().to_vec(),
@@ -498,7 +500,7 @@ fn bytecode(context: &mut Context, bytecode: &FF::Bytecode) -> PartialVMResult<B
 fn call(
     context: &mut Context,
     function_handle_index: FunctionHandleIndex,
-) -> PartialVMResult<*const Function> {
+) -> PartialVMResult<ArenaPointer<Function>> {
     let func_handle = context.module.function_handle_at(function_handle_index);
     let func_name = context.module.identifier_at(func_handle.name);
     let module_handle = context.module.module_handle_at(func_handle.module);

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

@@ -5,7 +5,7 @@
 use crate::{
     data_cache::TransactionDataCache,
     interpreter::Interpreter,
-    loader::{ast::Function, Loader},
+    loader::{arena::ArenaPointer, ast::Function, Loader},
     native_extensions::NativeContextExtensions,
     native_functions::{NativeFunction, NativeFunctions},
     session::{LoadedFunctionInstantiation, SerializedReturnValues, Session},
@@ -312,7 +312,7 @@ impl VMRuntime {
 
     fn execute_function_impl(
         &self,
-        func: *const Function,
+        func: ArenaPointer<Function>,
         ty_args: Vec<Type>,
         param_types: Vec<Type>,
         return_types: Vec<Type>,