From 705f1b88abaa8c169dfbf5b42832aedb53756213 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 28 Apr 2026 10:35:28 -0700 Subject: [PATCH 1/3] Fix passive element segment GC roots' endianness Passive element segments were registered as GC roots by converting a `*mut ValRaw` to a `*mut VMGcRef`. Because we always store GC refs as little endian inside `ValRaw`, regardless of the target architecture's endianness, this cast is only valid on little endian systems. This bug was not exposed until the introduction of the copying collector in https://github.com/bytecodealliance/wasmtime/pull/13107 The fix that this commit makes is to add another kind of GC root for `ValRaw`, where we can get and set the GC root using `ValRaw` internally, to ensure that the endianness (and incidentally also the GC ref offsets within the `ValRaw`) are matched up correctly between the GC root's definition and the collector's use of it. This brings us to three kinds of GC roots: Wasm stack roots, `VMGcRef` roots, and `ValRaw` roots. FWIW, I initially tried to make `VMGcRef` also always store its data as little endian, but this was a larger, more-invasive change and with feedback like https://github.com/bytecodealliance/wasmtime/pull/13193#discussion_r3140185859 suggesting the use of `[u8; 4]` instead of `u32` to make the byte ordering explicit, we break `rustc`'s niche type optimizations (since `VMGcRef` is non-zero right now). I also investigated making `PassiveElementSegment` an `enum` or either funcrefs or externrefs, similar to what we do for `wasmtime::runtime::vm::Table`. This also led to an outsized amount of code churn and didn't feel like it was paying for itself. Ultimately, I abandoned these approaches, preferring the one taken in this commit instead. --- .../wasmtime/src/runtime/externals/global.rs | 2 +- .../wasmtime/src/runtime/externals/table.rs | 2 +- .../src/runtime/gc/enabled/rooting.rs | 4 +- crates/wasmtime/src/runtime/store.rs | 2 +- .../wasmtime/src/runtime/vm/gc/gc_runtime.rs | 50 ++++++++++++++++--- crates/wasmtime/src/runtime/vm/instance.rs | 9 ++-- crates/wasmtime/src/runtime/vm/vmcontext.rs | 27 ++++++---- 7 files changed, 71 insertions(+), 25 deletions(-) diff --git a/crates/wasmtime/src/runtime/externals/global.rs b/crates/wasmtime/src/runtime/externals/global.rs index e0a9991d8ad5..cce1ad0ef5ff 100644 --- a/crates/wasmtime/src/runtime/externals/global.rs +++ b/crates/wasmtime/src/runtime/externals/global.rs @@ -304,7 +304,7 @@ impl Global { if let Some(gc_ref) = unsafe { self.definition(store).as_ref().as_gc_ref() } { unsafe { - gc_roots_list.add_root(gc_ref.into(), "Wasm global"); + gc_roots_list.add_vmgcref_root(gc_ref.into(), "Wasm global"); } } } diff --git a/crates/wasmtime/src/runtime/externals/table.rs b/crates/wasmtime/src/runtime/externals/table.rs index 98202ba71b9f..700b5aa48d00 100644 --- a/crates/wasmtime/src/runtime/externals/table.rs +++ b/crates/wasmtime/src/runtime/externals/table.rs @@ -542,7 +542,7 @@ impl Table { for gc_ref in table.gc_refs_mut() { if let Some(gc_ref) = gc_ref { unsafe { - gc_roots_list.add_root(gc_ref.into(), "Wasm table element"); + gc_roots_list.add_vmgcref_root(gc_ref.into(), "Wasm table element"); } } } diff --git a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs index 05bee990132b..7c589b716309 100644 --- a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs +++ b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs @@ -452,7 +452,7 @@ impl RootSet { log::trace!("Begin trace user LIFO roots"); for root in &mut self.lifo_roots { unsafe { - gc_roots_list.add_root((&mut root.gc_ref).into(), "user LIFO root"); + gc_roots_list.add_vmgcref_root((&mut root.gc_ref).into(), "user LIFO root"); } } log::trace!("End trace user LIFO roots"); @@ -460,7 +460,7 @@ impl RootSet { log::trace!("Begin trace user owned roots"); for (_id, root) in self.owned_rooted.iter_mut() { unsafe { - gc_roots_list.add_root(root.into(), "user owned root"); + gc_roots_list.add_vmgcref_root(root.into(), "user owned root"); } } log::trace!("End trace user owned roots"); diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs index f4848de5dd62..8aed22866bcc 100644 --- a/crates/wasmtime/src/runtime/store.rs +++ b/crates/wasmtime/src/runtime/store.rs @@ -2336,7 +2336,7 @@ impl StoreOpaque { if let Some(pending_exception) = self.pending_exception.as_mut() { unsafe { let root = pending_exception.as_gc_ref_mut(); - gc_roots_list.add_root(root.into(), "Pending exception"); + gc_roots_list.add_vmgcref_root(root.into(), "Pending exception"); } } log::trace!("End trace GC roots :: pending exception"); diff --git a/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs b/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs index 836a70417b05..113749f2820e 100644 --- a/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs +++ b/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs @@ -3,7 +3,7 @@ use crate::prelude::*; use crate::runtime::vm::{ ExternRefHostDataId, ExternRefHostDataTable, GcHeapObject, SendSyncPtr, TypedGcRef, VMArrayRef, - VMExternRef, VMGcHeader, VMGcObjectData, VMGcRef, + VMExternRef, VMGcHeader, VMGcObjectData, VMGcRef, ValRaw, }; use crate::store::Asyncness; use crate::vm::VMMemoryDefinition; @@ -575,12 +575,18 @@ pub struct GcRootsList(Vec); )] enum RawGcRoot { Stack(SendSyncPtr), - NonStack(SendSyncPtr), + VMGcRef(SendSyncPtr), + ValRaw(SendSyncPtr), } #[cfg(feature = "gc")] impl GcRootsList { /// Add a GC root that is inside a Wasm stack frame to this list. + /// + /// # Safety + /// + /// The pointer must be to a valid stack-map slot on the Wasm stack and must + /// remain valid for the duration of the `'a` lifetime. #[inline] pub unsafe fn add_wasm_stack_root(&mut self, ptr_to_root: SendSyncPtr) { unsafe { @@ -595,15 +601,37 @@ impl GcRootsList { } /// Add a GC root to this list. + /// + /// # Safety + /// + /// The pointer must be to a valid `VMGcRef` and must remain valid for the + /// duration of the `'a` lifetime. #[inline] - pub unsafe fn add_root(&mut self, ptr_to_root: SendSyncPtr, why: &str) { + pub unsafe fn add_vmgcref_root(&mut self, ptr_to_root: SendSyncPtr, why: &str) { unsafe { log::trace!( - "Adding non-stack root: {why}: {:#p}", + "Adding VMGcRef root: {why}: {:#p}", ptr_to_root.as_ref().unchecked_copy() ); } - self.0.push(RawGcRoot::NonStack(ptr_to_root)) + self.0.push(RawGcRoot::VMGcRef(ptr_to_root)) + } + + /// Add a GC root to this list. + /// + /// # Safety + /// + /// The pointer must be to a valid `ValRaw` that is a GC reference and must + /// remain valid for the duration of the `'a` lifetime. + #[inline] + pub unsafe fn add_val_raw_root(&mut self, ptr_to_root: SendSyncPtr, why: &str) { + unsafe { + log::trace!( + "Adding ValRaw root: {why}: {:#x}", + ptr_to_root.as_ref().get_anyref() + ); + } + self.0.push(RawGcRoot::ValRaw(ptr_to_root)) } /// Get an iterator over all roots in this list. @@ -677,11 +705,15 @@ impl GcRoot<'_> { #[inline] pub fn get(&self) -> VMGcRef { match self.raw { - RawGcRoot::NonStack(ptr) => unsafe { ptr::read(ptr.as_ptr()) }, + RawGcRoot::VMGcRef(ptr) => unsafe { ptr::read(ptr.as_ptr()) }, RawGcRoot::Stack(ptr) => unsafe { let raw: u32 = ptr::read(ptr.as_ptr()); VMGcRef::from_raw_u32(raw).expect("non-null") }, + RawGcRoot::ValRaw(ptr) => unsafe { + let val: ValRaw = ptr::read(ptr.as_ptr()); + val.get_vmgcref().expect("non-null") + }, } } @@ -694,12 +726,16 @@ impl GcRoot<'_> { /// referencing. pub fn set(&mut self, new_ref: VMGcRef) { match self.raw { - RawGcRoot::NonStack(ptr) => unsafe { + RawGcRoot::VMGcRef(ptr) => unsafe { ptr::write(ptr.as_ptr(), new_ref); }, RawGcRoot::Stack(ptr) => unsafe { ptr::write(ptr.as_ptr(), new_ref.as_raw_u32()); }, + RawGcRoot::ValRaw(ptr) => unsafe { + let val = ValRaw::vmgcref(Some(new_ref)); + ptr::write(ptr.as_ptr(), val); + }, } } } diff --git a/crates/wasmtime/src/runtime/vm/instance.rs b/crates/wasmtime/src/runtime/vm/instance.rs index 32617b89ec14..cd0ebcb9468a 100644 --- a/crates/wasmtime/src/runtime/vm/instance.rs +++ b/crates/wasmtime/src/runtime/vm/instance.rs @@ -227,15 +227,16 @@ impl Instance { for segment in self.passive_elements_mut().iter_mut() { if segment.needs_gc_rooting { for e in segment.elements() { - let Some(root) = e.as_vmgc_ref_ptr() else { + if e.get_vmgcref().is_none() { continue; - }; - let root: SendSyncPtr = root.into(); + } + + let root: SendSyncPtr = e.into(); // Safety: We know this is a type that needs GC rooting and // the lifetime is implied by our safety contract. unsafe { - gc_roots.add_root(root, "passive element segment"); + gc_roots.add_val_raw_root(root, "passive element segment"); } } } diff --git a/crates/wasmtime/src/runtime/vm/vmcontext.rs b/crates/wasmtime/src/runtime/vm/vmcontext.rs index 6a3ba227d542..2e9854163543 100644 --- a/crates/wasmtime/src/runtime/vm/vmcontext.rs +++ b/crates/wasmtime/src/runtime/vm/vmcontext.rs @@ -1725,6 +1725,19 @@ impl ValRaw { ValRaw { exnref: r.to_le() } } + #[inline] + #[cfg(feature = "gc")] + pub(crate) fn vmgcref(r: Option) -> ValRaw { + let raw = r.map_or(0, |r| r.as_raw_u32()); + + // NB: All `VMGcRef`-based `ValRaw`s are the same. + debug_assert_eq!(raw, ValRaw::anyref(raw).get_exnref()); + debug_assert_eq!(raw, ValRaw::exnref(raw).get_externref()); + debug_assert_eq!(raw, ValRaw::externref(raw).get_anyref()); + + ValRaw::anyref(raw) + } + /// Gets the WebAssembly `i32` value #[inline] pub fn get_i32(&self) -> i32 { @@ -1798,15 +1811,11 @@ impl ValRaw { exnref } - /// Convert this `&ValRaw` into a pointer to its inner `VMGcRef`. - #[cfg(feature = "gc")] - pub(crate) fn as_vmgc_ref_ptr(&self) -> Option> { - if self.get_anyref() == 0 { - return None; - } - let ptr = &raw const self.anyref; - let ptr = NonNull::new(ptr.cast_mut()).unwrap(); - Some(ptr.cast()) + /// Get the inner `VMGcRef`. + pub(crate) fn get_vmgcref(&self) -> Option { + debug_assert_eq!(self.get_anyref(), self.get_exnref()); + debug_assert_eq!(self.get_anyref(), self.get_externref()); + VMGcRef::from_raw_u32(self.get_anyref()) } } From 81a2b899d1dc67283e055136c2306f738ae2c2b7 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 29 Apr 2026 06:42:30 -0700 Subject: [PATCH 2/3] fix compilation of no-gc builds --- crates/wasmtime/src/runtime/vm/vmcontext.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/wasmtime/src/runtime/vm/vmcontext.rs b/crates/wasmtime/src/runtime/vm/vmcontext.rs index 2e9854163543..484564d5356e 100644 --- a/crates/wasmtime/src/runtime/vm/vmcontext.rs +++ b/crates/wasmtime/src/runtime/vm/vmcontext.rs @@ -1726,7 +1726,6 @@ impl ValRaw { } #[inline] - #[cfg(feature = "gc")] pub(crate) fn vmgcref(r: Option) -> ValRaw { let raw = r.map_or(0, |r| r.as_raw_u32()); From 93bd31c9fda4c20cddce02dd590660aada21fd85 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 29 Apr 2026 06:45:12 -0700 Subject: [PATCH 3/3] review feedback --- crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs b/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs index 113749f2820e..376588b40bff 100644 --- a/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs +++ b/crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs @@ -586,7 +586,7 @@ impl GcRootsList { /// # Safety /// /// The pointer must be to a valid stack-map slot on the Wasm stack and must - /// remain valid for the duration of the `'a` lifetime. + /// remain valid while registered within this `GcRootsList`. #[inline] pub unsafe fn add_wasm_stack_root(&mut self, ptr_to_root: SendSyncPtr) { unsafe { @@ -604,8 +604,8 @@ impl GcRootsList { /// /// # Safety /// - /// The pointer must be to a valid `VMGcRef` and must remain valid for the - /// duration of the `'a` lifetime. + /// The pointer must be to a valid `VMGcRef` and must remain valid while + /// registered within this `GcRootsList`. #[inline] pub unsafe fn add_vmgcref_root(&mut self, ptr_to_root: SendSyncPtr, why: &str) { unsafe { @@ -622,7 +622,7 @@ impl GcRootsList { /// # Safety /// /// The pointer must be to a valid `ValRaw` that is a GC reference and must - /// remain valid for the duration of the `'a` lifetime. + /// remain valid while registered within this `GcRootsList`. #[inline] pub unsafe fn add_val_raw_root(&mut self, ptr_to_root: SendSyncPtr, why: &str) { unsafe {