Make Global threadsafe, fix memory leak, etc in Global

lib/api/src/externals/global.rs

@@ -1,51 +1,39 @@
 use crate::exports::{ExportError, Exportable};
 use crate::externals::Extern;
 use crate::store::{Store, StoreObject};
-use crate::types::{Val, ValType};
+use crate::types::Val;
 use crate::GlobalType;
 use crate::Mutability;
 use crate::RuntimeError;
 use std::fmt;
-use wasmer_runtime::{Export, ExportGlobal, VMGlobalDefinition};
+use std::sync::Arc;
+use wasmer_runtime::{Export, ExportGlobal, Global as RuntimeGlobal};
 
 #[derive(Clone)]
 pub struct Global {
     store: Store,
     exported: ExportGlobal,
+    // This should not be here;
+    // it should be accessed through `exported`
+    // vm_global_definition: Arc<UnsafeCell<VMGlobalDefinition>>,
 }
 
 impl Global {
     pub fn new(store: &Store, val: Val) -> Global {
-        // Note: we unwrap because the provided type should always match
-        // the value type, so it's safe to unwrap.
-        Self::from_type(store, GlobalType::new(val.ty(), Mutability::Const), val).unwrap()
+        Self::from_type(store, Mutability::Const, val).unwrap()
     }
 
     pub fn new_mut(store: &Store, val: Val) -> Global {
-        // Note: we unwrap because the provided type should always match
-        // the value type, so it's safe to unwrap.
-        Self::from_type(store, GlobalType::new(val.ty(), Mutability::Var), val).unwrap()
+        Self::from_type(store, Mutability::Var, val).unwrap()
     }
 
-    fn from_type(store: &Store, ty: GlobalType, val: Val) -> Result<Global, RuntimeError> {
+    fn from_type(store: &Store, mutability: Mutability, val: Val) -> Result<Global, RuntimeError> {
         if !val.comes_from_same_store(store) {
             return Err(RuntimeError::new("cross-`Store` globals are not supported"));
         }
-        let mut definition = VMGlobalDefinition::new();
-        unsafe {
-            match val {
-                Val::I32(x) => *definition.as_i32_mut() = x,
-                Val::I64(x) => *definition.as_i64_mut() = x,
-                Val::F32(x) => *definition.as_f32_mut() = x,
-                Val::F64(x) => *definition.as_f64_mut() = x,
-                _ => return Err(RuntimeError::new(format!("create_global for {:?}", val))),
-                // Val::V128(x) => *definition.as_u128_bits_mut() = x,
-            }
-        };
-        let exported = ExportGlobal {
-            definition: Box::leak(Box::new(definition)),
-            global: ty,
-        };
+        let from = Arc::new(RuntimeGlobal::new_with_value(mutability, val));
+        let definition = from.vmglobal();
+        let exported = ExportGlobal { definition, from };
         Ok(Global {
             store: store.clone(),
             exported,
@@ -53,7 +41,7 @@ impl Global {
     }
 
     pub fn ty(&self) -> &GlobalType {
-        &self.exported.global
+        self.exported.from.ty()
     }
 
     pub fn store(&self) -> &Store {
@@ -61,44 +49,19 @@ impl Global {
     }
 
     pub fn get(&self) -> Val {
-        unsafe {
-            let definition = &mut *self.exported.definition;
-            match self.ty().ty {
-                ValType::I32 => Val::from(*definition.as_i32()),
-                ValType::I64 => Val::from(*definition.as_i64()),
-                ValType::F32 => Val::F32(*definition.as_f32()),
-                ValType::F64 => Val::F64(*definition.as_f64()),
-                _ => unimplemented!("Global::get for {:?}", self.ty().ty),
-            }
-        }
+        self.exported.from.get()
     }
 
     pub fn set(&self, val: Val) -> Result<(), RuntimeError> {
-        if self.ty().mutability != Mutability::Var {
-            return Err(RuntimeError::new(
-                "immutable global cannot be set".to_string(),
-            ));
-        }
-        if val.ty() != self.ty().ty {
-            return Err(RuntimeError::new(format!(
-                "global of type {:?} cannot be set to {:?}",
-                self.ty().ty,
-                val.ty()
-            )));
-        }
         if !val.comes_from_same_store(&self.store) {
             return Err(RuntimeError::new("cross-`Store` values are not supported"));
         }
         unsafe {
-            let definition = &mut *self.exported.definition;
-            match val {
-                Val::I32(i) => *definition.as_i32_mut() = i,
-                Val::I64(i) => *definition.as_i64_mut() = i,
-                Val::F32(f) => *definition.as_f32_mut() = f,
-                Val::F64(f) => *definition.as_f64_mut() = f,
-                _ => unimplemented!("Global::set for {:?}", val.ty()),
-            }
-        }
+            self.exported
+                .from
+                .set(val)
+                .map_err(|e| RuntimeError::new(e.to_string()))?
+        };
         Ok(())
     }
 

lib/api/src/import_object.rs

@@ -305,7 +305,7 @@ mod test {
         let happy_dog_entry = resolver.resolve_by_name("dog", "happy").unwrap();
 
         assert!(if let Export::Global(happy_dog_global) = happy_dog_entry {
-            happy_dog_global.global.ty == Type::I64
+            happy_dog_global.from.ty().ty == Type::I64
         } else {
             false
         });
@@ -331,7 +331,7 @@ mod test {
         let happy_dog_entry = resolver.resolve_by_name("dog", "happy").unwrap();
 
         assert!(if let Export::Global(happy_dog_global) = happy_dog_entry {
-            happy_dog_global.global.ty == Type::I32
+            happy_dog_global.from.ty().ty == Type::I32
         } else {
             false
         });
@@ -351,7 +351,7 @@ mod test {
         let happy_dog_entry = imports1.resolve_by_name("dog", "happy").unwrap();
 
         assert!(if let Export::Global(happy_dog_global) = happy_dog_entry {
-            happy_dog_global.global.ty == Type::I32
+            happy_dog_global.from.ty().ty == Type::I32
         } else {
             false
         });

lib/engine/src/resolver.rs

@@ -115,8 +115,8 @@ fn get_extern_from_export(_module: &ModuleInfo, export: &Export) -> ExternType {
             ExternType::Memory(memory)
         }
         Export::Global(ref g) => {
-            let global = g.global;
-            ExternType::Global(global)
+            let global = g.from.ty();
+            ExternType::Global(*global)
         }
     }
 }
@@ -221,6 +221,7 @@ pub fn resolve_imports(
             Export::Global(ref g) => {
                 global_imports.push(VMGlobalImport {
                     definition: g.definition,
+                    from: g.from.clone(),
                 });
             }
         }

lib/engine/src/tunables.rs

@@ -2,11 +2,11 @@ use crate::error::LinkError;
 use std::sync::Arc;
 use wasm_common::entity::{EntityRef, PrimaryMap};
 use wasm_common::{
-    LocalGlobalIndex, LocalMemoryIndex, LocalTableIndex, MemoryIndex, MemoryType, TableIndex,
-    TableType,
+    GlobalInit, GlobalType, LocalGlobalIndex, LocalMemoryIndex, LocalTableIndex, MemoryIndex,
+    MemoryType, Mutability, TableIndex, TableType,
 };
 use wasmer_runtime::MemoryError;
-use wasmer_runtime::{Memory, ModuleInfo, Table, VMGlobalDefinition};
+use wasmer_runtime::{Global, Memory, ModuleInfo, Table};
 use wasmer_runtime::{MemoryPlan, TablePlan};
 
 /// Tunables for an engine
@@ -23,6 +23,20 @@ pub trait Tunables {
     /// Create a memory given a memory type
     fn create_table(&self, table_type: TablePlan) -> Result<Arc<dyn Table>, String>;
 
+    /// Create a global with the given value.
+    fn create_initialized_global(
+        &self,
+        mutability: Mutability,
+        init: GlobalInit,
+    ) -> Result<Arc<Global>, String> {
+        Ok(Arc::new(Global::new_with_init(mutability, init)))
+    }
+
+    /// Create a global with a default value.
+    fn create_global(&self, ty: GlobalType) -> Result<Arc<Global>, String> {
+        Ok(Arc::new(Global::new(ty)))
+    }
+
     /// Allocate memory for just the memories of the current module.
     fn create_memories(
         &self,
@@ -63,12 +77,22 @@ pub trait Tunables {
     fn create_globals(
         &self,
         module: &ModuleInfo,
-    ) -> Result<PrimaryMap<LocalGlobalIndex, VMGlobalDefinition>, LinkError> {
+    ) -> Result<PrimaryMap<LocalGlobalIndex, Arc<Global>>, LinkError> {
         let num_imports = module.num_imported_globals;
         let mut vmctx_globals = PrimaryMap::with_capacity(module.globals.len() - num_imports);
 
-        for _ in &module.globals.values().as_slice()[num_imports..] {
-            vmctx_globals.push(VMGlobalDefinition::new());
+        for (idx, &global_type) in module.globals.iter().skip(num_imports) {
+            let idx = LocalGlobalIndex::new(idx.index());
+
+            vmctx_globals.push(
+                if let Some(&initializer) = module.global_initializers.get(idx) {
+                    self.create_initialized_global(global_type.mutability, initializer)
+                        .map_err(LinkError::Resource)?
+                } else {
+                    self.create_global(global_type)
+                        .map_err(LinkError::Resource)?
+                },
+            );
         }
 
         Ok(vmctx_globals)

lib/runtime/src/export.rs

@@ -1,6 +1,7 @@
 // This file contains code from external sources.
 // Attributions: https://github.com/wasmerio/wasmer-reborn/blob/master/ATTRIBUTIONS.md
 
+use crate::global::Global;
 use crate::memory::{Memory, MemoryPlan};
 use crate::table::{Table, TablePlan};
 use crate::vmcontext::{
@@ -9,7 +10,7 @@ use crate::vmcontext::{
 };
 use std::ptr::NonNull;
 use std::sync::Arc;
-use wasm_common::{FunctionType, GlobalType};
+use wasm_common::FunctionType;
 
 /// The value of an export passed from one instance to another.
 #[derive(Debug, Clone)]
@@ -61,13 +62,15 @@ pub struct ExportTable {
     pub from: Arc<dyn Table>,
 }
 
-// This is correct because there is no non-threadsafe logic directly in this type;
-// correct use of the raw table from multiple threads via `definition` requires `unsafe`
-// and is the responsibilty of the user of this type.
+/// # Safety
+/// This is correct because there is no non-threadsafe logic directly in this type;
+/// correct use of the raw table from multiple threads via `definition` requires `unsafe`
+/// and is the responsibilty of the user of this type.
 unsafe impl Send for ExportTable {}
-// This is correct because the values directly in `definition` should be considered immutable
-// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
-// only makes this type easier to use)
+/// # Safety
+/// This is correct because the values directly in `definition` should be considered immutable
+/// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
+/// only makes this type easier to use)
 unsafe impl Sync for ExportTable {}
 
 impl ExportTable {
@@ -104,13 +107,15 @@ pub struct ExportMemory {
     pub from: Arc<dyn Memory>,
 }
 
-// This is correct because there is no non-threadsafe logic directly in this type;
-// correct use of the raw memory from multiple threads via `definition` requires `unsafe`
-// and is the responsibilty of the user of this type.
+/// # Safety
+/// This is correct because there is no non-threadsafe logic directly in this type;
+/// correct use of the raw memory from multiple threads via `definition` requires `unsafe`
+/// and is the responsibilty of the user of this type.
 unsafe impl Send for ExportMemory {}
-// This is correct because the values directly in `definition` should be considered immutable
-// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
-// only makes this type easier to use)
+/// # Safety
+/// This is correct because the values directly in `definition` should be considered immutable
+/// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
+/// only makes this type easier to use)
 unsafe impl Sync for ExportMemory {}
 
 impl ExportMemory {
@@ -136,15 +141,26 @@ impl From<ExportMemory> for Export {
 #[derive(Debug, Clone)]
 pub struct ExportGlobal {
     /// The address of the global storage.
-    pub definition: *mut VMGlobalDefinition,
+    pub definition: NonNull<VMGlobalDefinition>,
     /// The global declaration, used for compatibility checking.
-    pub global: GlobalType,
+    pub from: Arc<Global>,
 }
 
+/// # Safety
+/// This is correct because there is no non-threadsafe logic directly in this type;
+/// correct use of the raw global from multiple threads via `definition` requires `unsafe`
+/// and is the responsibilty of the user of this type.
+unsafe impl Send for ExportGlobal {}
+/// # Safety
+/// This is correct because the values directly in `definition` should be considered immutable
+/// from the perspective of users of this type and the type is both `Send` and `Clone` (thus
+/// marking it `Sync` adds no new behavior, it only makes this type easier to use)
+unsafe impl Sync for ExportGlobal {}
+
 impl ExportGlobal {
     /// Returns whether or not the two `ExportGlobal`s refer to the same Global.
     pub fn same(&self, other: &Self) -> bool {
-        self.definition == other.definition && self.global == other.global
+        self.definition == other.definition && Arc::ptr_eq(&self.from, &other.from)
     }
 }
 

lib/runtime/src/global.rs

@@ -0,0 +1,157 @@
+use crate::vmcontext::VMGlobalDefinition;
+use std::cell::UnsafeCell;
+use std::ptr::NonNull;
+use std::sync::Mutex;
+use thiserror::Error;
+use wasm_common::{GlobalInit, GlobalType, Mutability, Type, Value};
+
+#[derive(Debug)]
+/// TODO: figure out a decent name for this thing
+pub struct Global {
+    ty: GlobalType,
+    // this box may be unnecessary
+    vm_global_definition: Box<UnsafeCell<VMGlobalDefinition>>,
+    // used to synchronize gets/sets
+    lock: Mutex<()>,
+}
+
+/// # Safety
+/// This is safe to send between threads because there is no-thread specific logic.
+/// TODO: look into other reasons that make something not `Send`
+unsafe impl Send for Global {}
+/// # Safety
+/// This is safe to share between threads because it uses a `Mutex` internally.
+unsafe impl Sync for Global {}
+
+/// Error type describing things that can go wrong when operating on Wasm Globals.
+#[derive(Error, Debug, Clone, PartialEq, Hash)]
+pub enum GlobalError {
+    /// The error returned when attempting to set an immutable global.
+    #[error("Attempted to set an immutable global")]
+    ImmutableGlobalCannotBeSet,
+
+    /// The error returned when attempting to operate on a global as a specific type
+    /// that differs from the global's own type.
+    #[error("Attempted to operate on a global of type {expected} as a global of type {found}")]
+    IncorrectType {
+        /// The type that the global is.
+        expected: Type,
+        /// The type that we were asked to use it as.
+        found: Type,
+    },
+}
+
+impl Global {
+    /// Create a new, zero bit-pattern initialized global from a [`GlobalType`].
+    pub fn new(global_type: GlobalType) -> Self {
+        Self {
+            ty: global_type,
+            vm_global_definition: Box::new(UnsafeCell::new(VMGlobalDefinition::new())),
+            lock: Mutex::new(()),
+        }
+    }
+
+    /// Create a new, zero bit-pattern initialized global from a [`GlobalType`].
+    pub fn new_with_init(mutability: Mutability, value: GlobalInit) -> Self {
+        let mut vm_global_definition = VMGlobalDefinition::new();
+        let global_type = unsafe {
+            match value {
+                GlobalInit::I32Const(v) => {
+                    *vm_global_definition.as_i32_mut() = v;
+                    GlobalType {
+                        ty: Type::I32,
+                        mutability,
+                    }
+                }
+                GlobalInit::I64Const(v) => {
+                    *vm_global_definition.as_i64_mut() = v;
+                    GlobalType {
+                        ty: Type::I64,
+                        mutability,
+                    }
+                }
+                GlobalInit::F32Const(v) => {
+                    *vm_global_definition.as_f32_mut() = v;
+                    GlobalType {
+                        ty: Type::F32,
+                        mutability,
+                    }
+                }
+                GlobalInit::F64Const(v) => {
+                    *vm_global_definition.as_f64_mut() = v;
+                    GlobalType {
+                        ty: Type::F64,
+                        mutability,
+                    }
+                }
+                _ => unimplemented!("Global is not implemented for initializer {:?}", value),
+            }
+        };
+        Self {
+            ty: global_type,
+            vm_global_definition: Box::new(UnsafeCell::new(vm_global_definition)),
+            lock: Mutex::new(()),
+        }
+    }
+
+    /// Create a new, zero bit-pattern initialized global from a [`GlobalType`].
+    pub fn new_with_value<T>(mutability: Mutability, value: Value<T>) -> Self {
+        Self::new_with_init(mutability, GlobalInit::from_value(value))
+    }
+
+    /// Get the type of the global.
+    pub fn ty(&self) -> &GlobalType {
+        &self.ty
+    }
+
+    /// Get a pointer to the underlying definition used by the generated code.
+    pub fn vmglobal(&self) -> NonNull<VMGlobalDefinition> {
+        let ptr = self.vm_global_definition.as_ref() as *const UnsafeCell<VMGlobalDefinition>
+            as *const VMGlobalDefinition as *mut VMGlobalDefinition;
+        unsafe { NonNull::new_unchecked(ptr) }
+    }
+
+    /// Get a value from the global.
+    pub fn get<T>(&self) -> Value<T> {
+        let _global_guard = self.lock.lock().unwrap();
+        unsafe {
+            let definition = &*self.vm_global_definition.get();
+            match self.ty().ty {
+                Type::I32 => Value::from(*definition.as_i32()),
+                Type::I64 => Value::from(*definition.as_i64()),
+                Type::F32 => Value::F32(*definition.as_f32()),
+                Type::F64 => Value::F64(*definition.as_f64()),
+                _ => unimplemented!("Global::get for {:?}", self.ty),
+            }
+        }
+    }
+
+    /// Get a value from the global.
+    ///
+    /// # Safety
+    /// The caller should check that the `val` comes from the same store as this global.
+    pub unsafe fn set<T>(&self, val: Value<T>) -> Result<(), GlobalError> {
+        let _global_guard = self.lock.lock().unwrap();
+        if self.ty().mutability != Mutability::Var {
+            return Err(GlobalError::ImmutableGlobalCannotBeSet);
+        }
+        if val.ty() != self.ty().ty {
+            return Err(GlobalError::IncorrectType {
+                expected: self.ty.ty,
+                found: val.ty(),
+            });
+        }
+        // TODO: checking which store values are in is not done in this function
+
+        // ideally we'd use atomics here
+        let definition = &mut *self.vm_global_definition.get();
+        match val {
+            Value::I32(i) => *definition.as_i32_mut() = i,
+            Value::I64(i) => *definition.as_i64_mut() = i,
+            Value::F32(f) => *definition.as_f32_mut() = f,
+            Value::F64(f) => *definition.as_f64_mut() = f,
+            _ => unimplemented!("Global::set for {:?}", val.ty()),
+        }
+        Ok(())
+    }
+}

lib/runtime/src/instance.rs

@@ -5,6 +5,7 @@
 //! wasm module (except its callstack and register state). An
 //! `InstanceHandle` is a reference-counting handle for an `Instance`.
 use crate::export::Export;
+use crate::global::Global;
 use crate::imports::Imports;
 use crate::memory::{Memory, MemoryError};
 use crate::table::Table;
@@ -73,12 +74,14 @@ pub(crate) struct Instance {
     offsets: VMOffsets,
 
     /// WebAssembly linear memory data.
-    // TODO: maybe `*mut Arc`
     memories: BoxedSlice<LocalMemoryIndex, Arc<dyn Memory>>,
 
     /// WebAssembly table data.
     tables: BoxedSlice<LocalTableIndex, Arc<dyn Table>>,
 
+    /// WebAssembly global data.
+    globals: BoxedSlice<LocalGlobalIndex, Arc<Global>>,
+
     /// Passive elements in this instantiation. As `elem.drop`s happen, these
     /// entries get removed. A missing entry is considered equivalent to an
     /// empty slice.
@@ -235,21 +238,21 @@ impl Instance {
 
     /// Return the indexed `VMGlobalDefinition`.
     fn global(&self, index: LocalGlobalIndex) -> VMGlobalDefinition {
-        unsafe { *self.global_ptr(index) }
+        unsafe { self.global_ptr(index).as_ref().clone() }
     }
 
     /// Set the indexed global to `VMGlobalDefinition`.
     #[allow(dead_code)]
-    fn set_global(&self, index: LocalGlobalIndex, global: VMGlobalDefinition) {
+    fn set_global(&self, index: LocalGlobalIndex, global: &VMGlobalDefinition) {
         unsafe {
-            *self.global_ptr(index) = global;
+            *self.global_ptr(index).as_ptr() = global.clone();
         }
     }
 
     /// Return the indexed `VMGlobalDefinition`.
-    fn global_ptr(&self, index: LocalGlobalIndex) -> *mut VMGlobalDefinition {
+    fn global_ptr(&self, index: LocalGlobalIndex) -> NonNull<VMGlobalDefinition> {
         let index = usize::try_from(index.as_u32()).unwrap();
-        unsafe { self.globals_ptr().add(index) }
+        NonNull::new(unsafe { self.globals_ptr().add(index) }).unwrap()
     }
 
     /// Return a pointer to the `VMGlobalDefinition`s.
@@ -330,15 +333,17 @@ impl Instance {
                     };
                 ExportMemory { definition, from }.into()
             }
-            ExportIndex::Global(index) => ExportGlobal {
-                definition: if let Some(def_index) = self.module.local_global_index(*index) {
-                    self.global_ptr(def_index)
+            ExportIndex::Global(index) => {
+                let (definition, from) = {
+                    if let Some(def_index) = self.module.local_global_index(*index) {
+                        (self.global_ptr(def_index), self.globals[def_index].clone())
                     } else {
-                    self.imported_global(*index).definition
-                },
-                global: self.module.globals[*index],
+                        let import = self.imported_global(*index);
+                        (import.definition, import.from.clone())
+                    }
+                };
+                ExportGlobal { definition, from }.into()
             }
-            .into(),
         }
     }
 
@@ -789,7 +794,7 @@ impl InstanceHandle {
         finished_functions: BoxedSlice<LocalFunctionIndex, *mut [VMFunctionBody]>,
         finished_memories: BoxedSlice<LocalMemoryIndex, Arc<dyn Memory>>,
         finished_tables: BoxedSlice<LocalTableIndex, Arc<dyn Table>>,
-        finished_globals: BoxedSlice<LocalGlobalIndex, VMGlobalDefinition>,
+        finished_globals: BoxedSlice<LocalGlobalIndex, Arc<Global>>,
         imports: Imports,
         vmshared_signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
         host_state: Box<dyn Any>,
@@ -815,7 +820,14 @@ impl InstanceHandle {
             .collect::<PrimaryMap<LocalMemoryIndex, _>>()
             .into_boxed_slice();
 
-        let vmctx_globals = finished_globals;
+        let vmctx_globals = finished_globals
+            .values()
+            .map(|m| {
+                let vmglobal_ptr = m.as_ref().vmglobal();
+                vmglobal_ptr.as_ref().clone()
+            })
+            .collect::<PrimaryMap<LocalGlobalIndex, _>>()
+            .into_boxed_slice();
 
         let offsets = VMOffsets::new(mem::size_of::<*const u8>() as u8, &module);
 
@@ -827,6 +839,7 @@ impl InstanceHandle {
                 offsets,
                 memories: finished_memories,
                 tables: finished_tables,
+                globals: finished_globals,
                 passive_elements: Default::default(),
                 passive_data,
                 finished_functions,
@@ -1112,7 +1125,7 @@ fn get_memory_init_start(init: &DataInitializer<'_>, instance: &Instance) -> usi
             if let Some(def_index) = instance.module.local_global_index(base) {
                 *instance.global(def_index).as_u32()
             } else {
-                *(*instance.imported_global(base).definition).as_u32()
+                *instance.imported_global(base).definition.as_ref().as_u32()
             }
         };
         start += usize::try_from(val).unwrap();
@@ -1165,7 +1178,7 @@ fn get_table_init_start(init: &TableElements, instance: &Instance) -> usize {
             if let Some(def_index) = instance.module.local_global_index(base) {
                 *instance.global(def_index).as_u32()
             } else {
-                *(*instance.imported_global(base).definition).as_u32()
+                *instance.imported_global(base).definition.as_ref().as_u32()
             }
         };
         start += usize::try_from(val).unwrap();
@@ -1258,7 +1271,7 @@ fn initialize_globals(instance: &Instance) {
     let module = Arc::clone(&instance.module);
     for (index, initializer) in module.global_initializers.iter() {
         unsafe {
-            let to = instance.global_ptr(index);
+            let to = instance.global_ptr(index).as_ptr();
             match initializer {
                 GlobalInit::I32Const(x) => *(*to).as_i32_mut() = *x,
                 GlobalInit::I64Const(x) => *(*to).as_i64_mut() = *x,
@@ -1269,7 +1282,7 @@ fn initialize_globals(instance: &Instance) {
                     let from = if let Some(def_x) = module.local_global_index(*x) {
                         instance.global(def_x)
                     } else {
-                        *instance.imported_global(*x).definition
+                        instance.imported_global(*x).definition.as_ref().clone()
                     };
                     *to = from;
                 }

lib/runtime/src/lib.rs

@@ -22,6 +22,7 @@
 )]
 
 mod export;
+mod global;
 mod imports;
 mod instance;
 mod memory;
@@ -37,6 +38,7 @@ mod vmoffsets;
 pub mod libcalls;
 
 pub use crate::export::*;
+pub use crate::global::*;
 pub use crate::imports::Imports;
 pub use crate::instance::InstanceHandle;
 pub use crate::memory::{Memory, MemoryError, MemoryPlan, MemoryStyle};

lib/runtime/src/vmcontext.rs

@@ -4,6 +4,7 @@
 //! This file declares `VMContext` and several related structs which contain
 //! fields that compiled wasm code accesses directly.
 
+use crate::global::Global;
 use crate::instance::Instance;
 use crate::memory::Memory;
 use crate::table::Table;
@@ -213,13 +214,28 @@ mod test_vmmemory_import {
 
 /// The fields compiled code needs to access to utilize a WebAssembly global
 /// variable imported from another instance.
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone)]
 #[repr(C)]
 pub struct VMGlobalImport {
     /// A pointer to the imported global variable description.
-    pub definition: *mut VMGlobalDefinition,
+    pub definition: NonNull<VMGlobalDefinition>,
+
+    /// A pointer to the `Global` that owns the global description.
+    pub from: Arc<Global>,
 }
 
+/// # Safety
+/// This data is safe to share between threads because it's plain data that
+/// is the user's responsibility to synchronize. Additionally, all operations
+/// on `from` are thread-safe through the use of a mutex in [`Global`].
+unsafe impl Send for VMGlobalImport {}
+/// # Safety
+/// This data is safe to share between threads because it's plain data that
+/// is the user's responsibility to synchronize. And because it's `Clone`, there's
+/// really no difference between passing it by reference or by value as far as
+/// correctness in a multi-threaded context is concerned.
+unsafe impl Sync for VMGlobalImport {}
+
 #[cfg(test)]
 mod test_vmglobal_import {
     use super::VMGlobalImport;
@@ -239,6 +255,10 @@ mod test_vmglobal_import {
             offset_of!(VMGlobalImport, definition),
             usize::from(offsets.vmglobal_import_definition())
         );
+        assert_eq!(
+            offset_of!(VMGlobalImport, from),
+            usize::from(offsets.vmglobal_import_from())
+        );
     }
 }
 
@@ -405,10 +425,9 @@ mod test_vmtable_definition {
 ///
 /// TODO: Pack the globals more densely, rather than using the same size
 /// for every type.
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone)]
 #[repr(C, align(16))]
 pub struct VMGlobalDefinition {
-    // TODO: use `UnsafeCell` here, make this not Copy; there's probably a ton of UB in this code right now
     storage: [u8; 16],
     // If more elements are added here, remember to add offset_of tests below!
 }

lib/runtime/src/vmoffsets.rs

@@ -209,7 +209,7 @@ impl VMOffsets {
         0 * self.pointer_size
     }
 
-    /// The offset of the `vmctx` field.
+    /// The offset of the `from` field.
     #[allow(clippy::identity_op)]
     pub const fn vmmemory_import_from(&self) -> u8 {
         1 * self.pointer_size
@@ -256,18 +256,24 @@ impl VMOffsets {
 ///
 /// [`VMGlobalImport`]: crate::vmcontext::VMGlobalImport
 impl VMOffsets {
-    /// The offset of the `from` field.
+    /// The offset of the `definition` field.
     #[allow(clippy::erasing_op)]
     pub const fn vmglobal_import_definition(&self) -> u8 {
         0 * self.pointer_size
     }
 
+    /// The offset of the `from` field.
+    #[allow(clippy::identity_op)]
+    pub const fn vmglobal_import_from(&self) -> u8 {
+        1 * self.pointer_size
+    }
+
     /// Return the size of [`VMGlobalImport`].
     ///
     /// [`VMGlobalImport`]: crate::vmcontext::VMGlobalImport
     #[allow(clippy::identity_op)]
     pub const fn size_of_vmglobal_import(&self) -> u8 {
-        1 * self.pointer_size
+        2 * self.pointer_size
     }
 }