Merge pull request wasmerio#135 from wasmerio/feature/improve-thread-…

…safety-of-core-types

Improve thread safety of core types

lib/api/src/externals/global.rs

@@ -6,7 +6,8 @@ use crate::GlobalType;
 use crate::Mutability;
 use crate::RuntimeError;
 use std::fmt;
-use wasmer_vm::{Export, ExportGlobal, VMGlobalDefinition};
+use std::sync::Arc;
+use wasmer_vm::{Export, ExportGlobal, Global as RuntimeGlobal};
 
 /// A WebAssembly `global` instance.
 ///
@@ -17,52 +18,50 @@ use wasmer_vm::{Export, ExportGlobal, VMGlobalDefinition};
 #[derive(Clone)]
 pub struct Global {
     store: Store,
-    exported: ExportGlobal,
+    global: Arc<RuntimeGlobal>,
 }
 
 impl Global {
     /// Create a new `Global` with the initial value [`Val`].
     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_value(store, val, Mutability::Const).unwrap()
     }
 
     /// Create a mutable `Global` with the initial value [`Val`].
     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_value(store, val, Mutability::Var).unwrap()
     }
 
-    fn from_type(store: &Store, ty: GlobalType, val: Val) -> Result<Global, RuntimeError> {
+    /// Create a `Global` with the initial value [`Val`] and the provided [`Mutability`].
+    fn from_value(store: &Store, val: Val, mutability: Mutability) -> 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();
+        let global = RuntimeGlobal::new(GlobalType {
+            mutability,
+            ty: val.ty(),
+        });
         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,
+                Val::I32(x) => *global.get_mut().as_i32_mut() = x,
+                Val::I64(x) => *global.get_mut().as_i64_mut() = x,
+                Val::F32(x) => *global.get_mut().as_f32_mut() = x,
+                Val::F64(x) => *global.get_mut().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 definition = global.vmglobal();
         Ok(Global {
             store: store.clone(),
-            exported,
+            global: Arc::new(global),
         })
     }
 
     /// Returns the [`GlobalType`] of the `Global`.
     pub fn ty(&self) -> &GlobalType {
-        &self.exported.global
+        self.global.ty()
     }
 
     /// Returns the [`Store`] where the `Global` belongs.
@@ -73,7 +72,7 @@ impl Global {
     /// Retrieves the current value [`Val`] that the Global has.
     pub fn get(&self) -> Val {
         unsafe {
-            let definition = &mut *self.exported.definition;
+            let definition = self.global.get();
             match self.ty().ty {
                 ValType::I32 => Val::from(*definition.as_i32()),
                 ValType::I64 => Val::from(*definition.as_i64()),
@@ -108,7 +107,7 @@ impl Global {
             return Err(RuntimeError::new("cross-`Store` values are not supported"));
         }
         unsafe {
-            let definition = &mut *self.exported.definition;
+            let definition = self.global.get_mut();
             match val {
                 Val::I32(i) => *definition.as_i32_mut() = i,
                 Val::I64(i) => *definition.as_i64_mut() = i,
@@ -123,13 +122,13 @@ impl Global {
     pub(crate) fn from_export(store: &Store, wasmer_export: ExportGlobal) -> Global {
         Global {
             store: store.clone(),
-            exported: wasmer_export,
+            global: wasmer_export.from.clone(),
         }
     }
 
     /// Returns whether or not these two globals refer to the same data.
     pub fn same(&self, other: &Global) -> bool {
-        self.exported.same(&other.exported)
+        Arc::ptr_eq(&self.global, &other.global)
     }
 }
 
@@ -145,7 +144,10 @@ impl fmt::Debug for Global {
 
 impl<'a> Exportable<'a> for Global {
     fn to_export(&self) -> Export {
-        self.exported.clone().into()
+        ExportGlobal {
+            from: self.global.clone(),
+        }
+        .into()
     }
 
     fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {

lib/api/src/import_object.rs

@@ -320,7 +320,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
         });
@@ -346,7 +346,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
         });
@@ -366,7 +366,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/api/src/instance.rs

@@ -23,7 +23,6 @@ pub struct Instance {
     pub exports: Exports,
 }
 
-/*
 #[cfg(test)]
 mod send_test {
     use super::*;
@@ -35,7 +34,7 @@ mod send_test {
     fn instance_is_send() {
         assert!(is_send::<Instance>());
     }
-}*/
+}
 
 impl Instance {
     /// Creates a new `Instance` from a WebAssembly [`Module`] and a

lib/compiler-cranelift/src/func_environ.rs

@@ -694,20 +694,19 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro
 
         let (ptr, offset) = {
             let vmctx = self.vmctx(func);
-            if let Some(def_index) = self.module.local_global_index(index) {
-                let offset =
-                    i32::try_from(self.offsets.vmctx_vmglobal_definition(def_index)).unwrap();
-                (vmctx, offset)
+            let from_offset = if let Some(def_index) = self.module.local_global_index(index) {
+                self.offsets.vmctx_vmglobal_definition(def_index)
             } else {
-                let from_offset = self.offsets.vmctx_vmglobal_import_definition(index);
-                let global = func.create_global_value(ir::GlobalValueData::Load {
-                    base: vmctx,
-                    offset: Offset32::new(i32::try_from(from_offset).unwrap()),
-                    global_type: pointer_type,
-                    readonly: true,
-                });
-                (global, 0)
-            }
+                self.offsets.vmctx_vmglobal_import_definition(index)
+            };
+            let global = func.create_global_value(ir::GlobalValueData::Load {
+                base: vmctx,
+                offset: Offset32::new(i32::try_from(from_offset).unwrap()),
+                global_type: pointer_type,
+                readonly: true,
+            });
+
+            (global, 0)
         };
 
         Ok(GlobalVariable::Memory {

lib/compiler-llvm/src/translator/intrinsics.rs

@@ -834,34 +834,34 @@ impl<'ctx, 'a> CtxType<'ctx, 'a> {
                 let global_value_type = global_type.ty;
 
                 let global_mutability = global_type.mutability;
-                let global_ptr =
-                    if let Some(local_global_index) = wasm_module.local_global_index(index) {
-                        let offset = offsets.vmctx_vmglobal_definition(local_global_index);
-                        let offset = intrinsics.i32_ty.const_int(offset.into(), false);
-                        unsafe { cache_builder.build_gep(ctx_ptr_value, &[offset], "") }
-                    } else {
-                        let offset = offsets.vmctx_vmglobal_import(index);
-                        let offset = intrinsics.i32_ty.const_int(offset.into(), false);
-                        let global_ptr_ptr =
-                            unsafe { cache_builder.build_gep(ctx_ptr_value, &[offset], "") };
-                        let global_ptr_ptr = cache_builder
-                            .build_bitcast(
-                                global_ptr_ptr,
-                                intrinsics.i32_ptr_ty.ptr_type(AddressSpace::Generic),
-                                "",
-                            )
-                            .into_pointer_value();
-                        let global_ptr = cache_builder
-                            .build_load(global_ptr_ptr, "")
-                            .into_pointer_value();
-                        tbaa_label(
-                            module,
-                            intrinsics,
-                            format!("global_ptr {}", index.as_u32()),
-                            global_ptr.as_instruction_value().unwrap(),
-                        );
-                        global_ptr
-                    };
+                let offset = if let Some(local_global_index) = wasm_module.local_global_index(index)
+                {
+                    offsets.vmctx_vmglobal_definition(local_global_index)
+                } else {
+                    offsets.vmctx_vmglobal_import(index)
+                };
+                let offset = intrinsics.i32_ty.const_int(offset.into(), false);
+                let global_ptr = {
+                    let global_ptr_ptr =
+                        unsafe { cache_builder.build_gep(ctx_ptr_value, &[offset], "") };
+                    let global_ptr_ptr = cache_builder
+                        .build_bitcast(
+                            global_ptr_ptr,
+                            intrinsics.i32_ptr_ty.ptr_type(AddressSpace::Generic),
+                            "",
+                        )
+                        .into_pointer_value();
+                    let global_ptr = cache_builder
+                        .build_load(global_ptr_ptr, "")
+                        .into_pointer_value();
+                    tbaa_label(
+                        module,
+                        intrinsics,
+                        format!("global_ptr {}", index.as_u32()),
+                        global_ptr.as_instruction_value().unwrap(),
+                    );
+                    global_ptr
+                };
                 let global_ptr = cache_builder
                     .build_bitcast(
                         global_ptr,

lib/compiler-singlepass/src/codegen_x64.rs

@@ -1896,7 +1896,13 @@ impl<'a> FuncGen<'a> {
                     self.module.local_global_index(global_index)
                 {
                     let offset = self.vmoffsets.vmctx_vmglobal_definition(local_global_index);
-                    Location::Memory(Machine::get_vmctx_reg(), offset as i32)
+                    self.emit_relaxed_binop(
+                        Assembler::emit_mov,
+                        Size::S64,
+                        Location::Memory(Machine::get_vmctx_reg(), offset as i32),
+                        Location::GPR(tmp),
+                    );
+                    Location::Memory(tmp, 0)
                 } else {
                     // Imported globals require one level of indirection.
                     let offset = self
@@ -1922,7 +1928,13 @@ impl<'a> FuncGen<'a> {
                     self.module.local_global_index(global_index)
                 {
                     let offset = self.vmoffsets.vmctx_vmglobal_definition(local_global_index);
-                    Location::Memory(Machine::get_vmctx_reg(), offset as i32)
+                    self.emit_relaxed_binop(
+                        Assembler::emit_mov,
+                        Size::S64,
+                        Location::Memory(Machine::get_vmctx_reg(), offset as i32),
+                        Location::GPR(tmp),
+                    );
+                    Location::Memory(tmp, 0)
                 } else {
                     // Imported globals require one level of indirection.
                     let offset = self

lib/engine-jit/src/artifact.rs

@@ -21,14 +21,14 @@ use wasmer_engine::{
 };
 #[cfg(feature = "compiler")]
 use wasmer_engine::{Engine, SerializableFunctionFrameInfo, Tunables};
-use wasmer_vm::{MemoryStyle, ModuleInfo, TableStyle, VMFunctionBody, VMSharedSignatureIndex};
+use wasmer_vm::{FunctionBodyPtr, MemoryStyle, ModuleInfo, TableStyle, VMSharedSignatureIndex};
 
 /// A compiled wasm module, ready to be instantiated.
 pub struct JITArtifact {
     _unwind_registry: Arc<UnwindRegistry>,
     serializable: SerializableModule,
-    finished_functions: BoxedSlice<LocalFunctionIndex, *mut [VMFunctionBody]>,
-    finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, *mut [VMFunctionBody]>,
+    finished_functions: BoxedSlice<LocalFunctionIndex, FunctionBodyPtr>,
+    finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, FunctionBodyPtr>,
     signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
     frame_info_registration: Mutex<Option<GlobalFrameInfoRegistration>>,
 }
@@ -278,14 +278,12 @@ impl Artifact for JITArtifact {
         &self.serializable.compile_info.table_styles
     }
 
-    fn finished_functions(&self) -> &BoxedSlice<LocalFunctionIndex, *mut [VMFunctionBody]> {
+    fn finished_functions(&self) -> &BoxedSlice<LocalFunctionIndex, FunctionBodyPtr> {
         &self.finished_functions
     }
 
     // TODO: return *const instead of *mut
-    fn finished_dynamic_function_trampolines(
-        &self,
-    ) -> &BoxedSlice<FunctionIndex, *mut [VMFunctionBody]> {
+    fn finished_dynamic_function_trampolines(&self) -> &BoxedSlice<FunctionIndex, FunctionBodyPtr> {
         &self.finished_dynamic_function_trampolines
     }
 

lib/engine-jit/src/code_memory.rs

@@ -8,7 +8,7 @@ use std::sync::Arc;
 use std::{cmp, mem};
 use wasm_common::entity::{EntityRef, PrimaryMap};
 use wasmer_compiler::{CompiledFunctionUnwindInfo, FunctionBody, SectionBody};
-use wasmer_vm::{Mmap, VMFunctionBody};
+use wasmer_vm::{FunctionBodyPtr, Mmap, VMFunctionBody};
 
 /// The optimal alignment for functions.
 ///
@@ -75,7 +75,7 @@ impl CodeMemory {
         &mut self,
         registry: &mut UnwindRegistry,
         compilation: &PrimaryMap<K, FunctionBody>,
-    ) -> Result<PrimaryMap<K, *mut [VMFunctionBody]>, String>
+    ) -> Result<PrimaryMap<K, FunctionBodyPtr>, String>
     where
         K: EntityRef,
     {
@@ -99,7 +99,7 @@ impl CodeMemory {
             );
             assert!(vmfunc as *mut _ as *mut u8 as usize % ARCH_FUNCTION_ALIGNMENT == 0);
 
-            result.push(vmfunc as *mut [VMFunctionBody]);
+            result.push(FunctionBodyPtr(vmfunc as *mut [VMFunctionBody]));
 
             padding = get_align_padding_size(next_start as usize, ARCH_FUNCTION_ALIGNMENT);
             start = next_start;