Fix passive element segment GC roots' endianness (#13230)

* 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
#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
#13193 (comment)
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.

* fix compilation of no-gc builds

* review feedback

Author: fitzgen

crates/wasmtime/src/runtime/externals/global.rs

@@ -312,7 +312,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");
                 }
             }
         }

crates/wasmtime/src/runtime/externals/table.rs

@@ -574,7 +574,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");
                 }
             }
         }

crates/wasmtime/src/runtime/gc/enabled/rooting.rs

@@ -452,15 +452,15 @@ 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");
 
         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");

crates/wasmtime/src/runtime/store.rs

@@ -2350,7 +2350,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");

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<RawGcRoot>);
 )]
 enum RawGcRoot {
     Stack(SendSyncPtr<u32>),
-    NonStack(SendSyncPtr<VMGcRef>),
+    VMGcRef(SendSyncPtr<VMGcRef>),
+    ValRaw(SendSyncPtr<ValRaw>),
 }
 
 #[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 while registered within this `GcRootsList`.
     #[inline]
     pub unsafe fn add_wasm_stack_root(&mut self, ptr_to_root: SendSyncPtr<u32>) {
         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 while
+    /// registered within this `GcRootsList`.
     #[inline]
-    pub unsafe fn add_root(&mut self, ptr_to_root: SendSyncPtr<VMGcRef>, why: &str) {
+    pub unsafe fn add_vmgcref_root(&mut self, ptr_to_root: SendSyncPtr<VMGcRef>, 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 while registered within this `GcRootsList`.
+    #[inline]
+    pub unsafe fn add_val_raw_root(&mut self, ptr_to_root: SendSyncPtr<ValRaw>, 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);
+            },
         }
     }
 }

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<super::VMGcRef> = root.into();
+                    }
+
+                    let root: SendSyncPtr<ValRaw> = 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");
                     }
                 }
             }

crates/wasmtime/src/runtime/vm/vmcontext.rs

@@ -1750,6 +1750,18 @@ impl ValRaw {
         ValRaw { exnref: r.to_le() }
     }
 
+    #[inline]
+    pub(crate) fn vmgcref(r: Option<VMGcRef>) -> 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 {
@@ -1823,15 +1835,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<NonNull<crate::vm::VMGcRef>> {
-        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<crate::vm::VMGcRef> {
+        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())
     }
 }