Include Wasm-defined globals and memories in core dumps (#6935)

* Include Wasm-defined globals and memories in core dumps

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

* Narrow scope of unsafe block

* Add missing skip-miri attribute for test that calls into Wasm

---------

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

crates/runtime/src/instance.rs

@@ -350,6 +350,15 @@ impl Instance {
         unsafe { *self.vmctx_plus_offset(self.offsets().vmctx_vmmemory_pointer(index)) }
     }
 
+    /// Return the memories defined within this instance (not imported).
+    pub fn defined_memories<'a>(
+        &'a self,
+    ) -> impl ExactSizeIterator<Item = (DefinedMemoryIndex, &'a Memory)> + 'a {
+        self.memories
+            .iter()
+            .map(|(index, (_alloc_index, memory))| (index, memory))
+    }
+
     /// Return the indexed `VMGlobalDefinition`.
     fn global(&mut self, index: DefinedGlobalIndex) -> &VMGlobalDefinition {
         unsafe { &*self.global_ptr(index) }
@@ -375,6 +384,25 @@ impl Instance {
         }
     }
 
+    /// Get the globals defined in this instance (not imported).
+    pub fn defined_globals<'a>(
+        &'a mut self,
+    ) -> impl ExactSizeIterator<Item = (DefinedGlobalIndex, ExportGlobal)> + 'a {
+        let module = self.module().clone();
+        module
+            .globals
+            .keys()
+            .skip(module.num_imported_globals)
+            .map(move |global_idx| {
+                let def_idx = module.defined_global_index(global_idx).unwrap();
+                let global = ExportGlobal {
+                    definition: self.global_ptr(def_idx),
+                    global: self.module().globals[global_idx],
+                };
+                (def_idx, global)
+            })
+    }
+
     /// Return a pointer to the interrupts structure
     pub fn runtime_limits(&mut self) -> *mut *const VMRuntimeLimits {
         unsafe { self.vmctx_plus_offset_mut(self.offsets().vmctx_runtime_limits()) }
@@ -1322,6 +1350,23 @@ impl InstanceHandle {
         self.instance_mut().get_table_with_lazy_init(index, range)
     }
 
+    /// Return the memories defined in this instance (not imported).
+    pub fn defined_memories<'a>(&'a mut self) -> impl ExactSizeIterator<Item = ExportMemory> + 'a {
+        let idxs = (0..self.module().memory_plans.len())
+            .skip(self.module().num_imported_memories)
+            .map(|i| MemoryIndex::new(i))
+            .collect::<Vec<_>>();
+        idxs.into_iter()
+            .map(|memidx| self.get_exported_memory(memidx))
+    }
+
+    /// Get the globals defined in this instance (not imported).
+    pub fn defined_globals<'a>(
+        &'a mut self,
+    ) -> impl ExactSizeIterator<Item = (DefinedGlobalIndex, ExportGlobal)> + 'a {
+        self.instance_mut().defined_globals()
+    }
+
     /// Return a reference to the contained `Instance`.
     #[inline]
     pub(crate) fn instance(&self) -> &Instance {

crates/wasmtime/src/coredump.rs

@@ -28,13 +28,13 @@ pub struct WasmCoreDump {
     name: String,
     modules: Vec<Module>,
     instances: Vec<Instance>,
-    store_memories: Vec<Memory>,
-    store_globals: Vec<Global>,
+    memories: Vec<Memory>,
+    globals: Vec<Global>,
     backtrace: WasmBacktrace,
 }
 
 impl WasmCoreDump {
-    pub(crate) fn new(store: &StoreOpaque, backtrace: WasmBacktrace) -> WasmCoreDump {
+    pub(crate) fn new(store: &mut StoreOpaque, backtrace: WasmBacktrace) -> WasmCoreDump {
         let modules: Vec<_> = store.modules().all_modules().cloned().collect();
         let instances: Vec<Instance> = store.all_instances().collect();
         let store_memories: Vec<Memory> = store.all_memories().collect();
@@ -44,37 +44,41 @@ impl WasmCoreDump {
             name: String::from("store_name"),
             modules,
             instances,
-            store_memories,
-            store_globals,
+            memories: store_memories,
+            globals: store_globals,
             backtrace,
         }
     }
 
-    /// The stack frames for the CoreDump
+    /// The stack frames for this core dump.
+    ///
+    /// Frames appear in callee to caller order, that is youngest to oldest
+    /// frames.
     pub fn frames(&self) -> &[FrameInfo] {
         self.backtrace.frames()
     }
 
-    /// The names of the modules involved in the CoreDump
+    /// All modules instantiated inside the store when the core dump was
+    /// created.
     pub fn modules(&self) -> &[Module] {
         self.modules.as_ref()
     }
 
-    /// The instances involved in the CoreDump
+    /// All instances within the store when the core dump was created.
     pub fn instances(&self) -> &[Instance] {
         self.instances.as_ref()
     }
 
-    /// The imported globals that belong to the store, rather than a specific
-    /// instance
-    pub fn store_globals(&self) -> &[Global] {
-        self.store_globals.as_ref()
+    /// All globals, instance- or host-defined, within the store when the core
+    /// dump was created.
+    pub fn globals(&self) -> &[Global] {
+        self.globals.as_ref()
     }
 
-    /// The imported memories that belong to the store, rather than a specific
-    /// instance.
-    pub fn store_memories(&self) -> &[Memory] {
-        self.store_memories.as_ref()
+    /// All memories, instance- or host-defined, within the store when the core
+    /// dump was created.
+    pub fn memories(&self) -> &[Memory] {
+        self.memories.as_ref()
     }
 }
 
@@ -92,12 +96,12 @@ impl fmt::Display for WasmCoreDump {
         }
 
         writeln!(f, "memories:")?;
-        for memory in self.store_memories.iter() {
+        for memory in self.memories.iter() {
             writeln!(f, "  {:?}", memory)?;
         }
 
         writeln!(f, "globals:")?;
-        for global in self.store_globals.iter() {
+        for global in self.globals.iter() {
             writeln!(f, "  {:?}", global)?;
         }
 

crates/wasmtime/src/externals.rs

@@ -5,7 +5,6 @@ use crate::{
     SharedMemory, TableType, Val, ValType,
 };
 use anyhow::{anyhow, bail, Result};
-use runtime::ExportGlobal;
 use std::mem;
 use std::ptr;
 use wasmtime_runtime::{self as runtime};
@@ -249,10 +248,6 @@ impl Global {
         }
     }
 
-    pub(crate) fn from_stored(stored: Stored<ExportGlobal>) -> Global {
-        Global(stored)
-    }
-
     /// Returns the underlying type of this `global`.
     ///
     /// # Panics

crates/wasmtime/src/instance.rs

@@ -42,6 +42,15 @@ pub(crate) struct InstanceData {
     exports: Vec<Option<Extern>>,
 }
 
+impl InstanceData {
+    pub fn from_id(id: InstanceId) -> InstanceData {
+        InstanceData {
+            id,
+            exports: vec![],
+        }
+    }
+}
+
 impl Instance {
     /// Creates a new [`Instance`] from the previously compiled [`Module`] and
     /// list of `imports` specified.
@@ -289,7 +298,7 @@ impl Instance {
         // conflicts with the borrow on `store.engine`) but this doesn't
         // matter in practice since initialization isn't even running any
         // code here anyway.
-        let id = store.add_instance(instance_handle.clone(), false);
+        let id = store.add_instance(instance_handle.clone());
 
         // Additionally, before we start doing fallible instantiation, we
         // do one more step which is to insert an `InstanceData`
@@ -330,9 +339,6 @@ impl Instance {
 
         Ok((instance, compiled_module.module().start_func))
     }
-    pub(crate) fn from_stored(stored: Stored<InstanceData>) -> Instance {
-        Instance(stored)
-    }
 
     pub(crate) fn from_wasmtime(handle: InstanceData, store: &mut StoreOpaque) -> Instance {
         Instance(store.store_data_mut().insert(handle))

crates/wasmtime/src/memory.rs

@@ -9,7 +9,7 @@ use std::ops::Range;
 use std::slice;
 use std::time::Instant;
 use wasmtime_environ::MemoryPlan;
-use wasmtime_runtime::{ExportMemory, RuntimeLinearMemory, VMMemoryImport};
+use wasmtime_runtime::{RuntimeLinearMemory, VMMemoryImport};
 
 pub use wasmtime_runtime::WaitResult;
 
@@ -271,10 +271,6 @@ impl Memory {
         }
     }
 
-    pub(crate) fn from_stored(stored: Stored<ExportMemory>) -> Memory {
-        Memory(stored)
-    }
-
     /// Returns the underlying type of this memory.
     ///
     /// # Panics

crates/wasmtime/src/store.rs

@@ -96,9 +96,9 @@ use std::sync::atomic::AtomicU64;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 use wasmtime_runtime::{
-    ExportGlobal, ExportMemory, InstanceAllocationRequest, InstanceAllocator, InstanceHandle,
-    ModuleInfo, OnDemandInstanceAllocator, SignalHandler, StoreBox, StorePtr, VMContext,
-    VMExternRef, VMExternRefActivationsTable, VMFuncRef, VMRuntimeLimits, WasmFault,
+    ExportGlobal, InstanceAllocationRequest, InstanceAllocator, InstanceHandle, ModuleInfo,
+    OnDemandInstanceAllocator, SignalHandler, StoreBox, StorePtr, VMContext, VMExternRef,
+    VMExternRefActivationsTable, VMFuncRef, VMRuntimeLimits, WasmFault,
 };
 
 mod context;
@@ -433,8 +433,14 @@ where
 /// instance allocator.
 struct StoreInstance {
     handle: InstanceHandle,
-    // Stores whether or not to use the on-demand allocator to deallocate the instance
-    ondemand: bool,
+
+    /// Whether or not this is a dummy instance that is just an implementation
+    /// detail for something else. For example, host-created memories internally
+    /// create a dummy instance.
+    ///
+    /// Regardless of the configured instance allocator for this engine, dummy
+    /// instances always use the on-demand allocator to deallocate the instance.
+    dummy: bool,
 }
 
 #[derive(Copy, Clone)]
@@ -1247,10 +1253,23 @@ impl StoreOpaque {
         &mut self.host_globals
     }
 
-    pub unsafe fn add_instance(&mut self, handle: InstanceHandle, ondemand: bool) -> InstanceId {
+    pub unsafe fn add_instance(&mut self, handle: InstanceHandle) -> InstanceId {
+        self.instances.push(StoreInstance {
+            handle: handle.clone(),
+            dummy: false,
+        });
+        InstanceId(self.instances.len() - 1)
+    }
+
+    /// Add a dummy instance that to the store.
+    ///
+    /// These are instances that are just implementation details of something
+    /// else (e.g. host-created memories that are not actually defined in any
+    /// Wasm module) and therefore shouldn't show up in things like core dumps.
+    pub unsafe fn add_dummy_instance(&mut self, handle: InstanceHandle) -> InstanceId {
         self.instances.push(StoreInstance {
             handle: handle.clone(),
-            ondemand,
+            dummy: true,
         });
         InstanceId(self.instances.len() - 1)
     }
@@ -1263,22 +1282,62 @@ impl StoreOpaque {
         &mut self.instances[id.0].handle
     }
 
-    pub fn all_instances(&self) -> impl ExactSizeIterator<Item = Instance> {
-        self.store_data()
-            .iter::<InstanceData>()
-            .map(Instance::from_stored)
+    /// Get all instances (ignoring dummy instances) within this store.
+    pub fn all_instances<'a>(&'a mut self) -> impl ExactSizeIterator<Item = Instance> + 'a {
+        let instances = self
+            .instances
+            .iter()
+            .enumerate()
+            .filter_map(|(idx, inst)| {
+                let id = InstanceId::from_index(idx);
+                if inst.dummy {
+                    None
+                } else {
+                    Some(InstanceData::from_id(id))
+                }
+            })
+            .collect::<Vec<_>>();
+        instances
+            .into_iter()
+            .map(|i| Instance::from_wasmtime(i, self))
     }
 
-    pub fn all_memories(&self) -> impl ExactSizeIterator<Item = Memory> {
-        self.store_data()
-            .iter::<ExportMemory>()
-            .map(Memory::from_stored)
+    /// Get all memories (host- or Wasm-defined) within this store.
+    pub fn all_memories<'a>(&'a mut self) -> impl Iterator<Item = Memory> + 'a {
+        let mems = self
+            .instances
+            .iter_mut()
+            .flat_map(|instance| instance.handle.defined_memories())
+            .collect::<Vec<_>>();
+        mems.into_iter()
+            .map(|memory| unsafe { Memory::from_wasmtime_memory(memory, self) })
     }
 
-    pub fn all_globals(&self) -> impl ExactSizeIterator<Item = Global> {
-        self.store_data()
-            .iter::<ExportGlobal>()
-            .map(Global::from_stored)
+    /// Iterate over all globals (host- or Wasm-defined) within this store.
+    pub fn all_globals<'a>(&'a mut self) -> impl Iterator<Item = Global> + 'a {
+        unsafe {
+            // First gather all the host-created globals.
+            let mut globals = self
+                .host_globals()
+                .iter()
+                .map(|global| ExportGlobal {
+                    definition: &mut (*global.get()).global as *mut _,
+                    global: (*global.get()).ty.to_wasm_type(),
+                })
+                .collect::<Vec<_>>();
+
+            // Then iterate over all instances and yield each of their defined
+            // globals.
+            globals.extend(
+                self.instances.iter_mut().flat_map(|instance| {
+                    instance.handle.defined_globals().map(|(_i, global)| global)
+                }),
+            );
+
+            globals
+                .into_iter()
+                .map(|g| Global::from_wasmtime_global(g, self))
+        }
     }
 
     #[cfg_attr(not(target_os = "linux"), allow(dead_code))] // not used on all platforms
@@ -2207,7 +2266,7 @@ impl Drop for StoreOpaque {
             let allocator = self.engine.allocator();
             let ondemand = OnDemandInstanceAllocator::default();
             for instance in self.instances.iter_mut() {
-                if instance.ondemand {
+                if instance.dummy {
                     ondemand.deallocate_module(&mut instance.handle);
                 } else {
                     allocator.deallocate_module(&mut instance.handle);

crates/wasmtime/src/store/data.rs

@@ -13,6 +13,12 @@ use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
 #[derive(Copy, Clone)]
 pub struct InstanceId(pub(super) usize);
 
+impl InstanceId {
+    pub fn from_index(idx: usize) -> InstanceId {
+        InstanceId(idx)
+    }
+}
+
 pub struct StoreData {
     id: StoreId,
     funcs: Vec<crate::func::FuncData>,

crates/wasmtime/src/trampoline.rs

@@ -50,7 +50,7 @@ fn create_handle(
                 wmemcheck: false,
             })?;
 
-        Ok(store.add_instance(handle, true))
+        Ok(store.add_dummy_instance(handle))
     }
 }
 

crates/wasmtime/src/trampoline/global.rs

@@ -5,8 +5,8 @@ use wasmtime_runtime::{StoreBox, VMGlobalDefinition};
 
 #[repr(C)]
 pub struct VMHostGlobalContext {
-    ty: GlobalType,
-    global: VMGlobalDefinition,
+    pub(crate) ty: GlobalType,
+    pub(crate) global: VMGlobalDefinition,
 }
 
 impl Drop for VMHostGlobalContext {