Merge pull request #602 from martian56/fix/concurrent-gc-missed-root

fix: trace enums by active variant (GC use-after-free) (#601)

Author: martian56

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to Raven are documented in this file.
 
+## [2.18.121] - 2026-06-16
+
+### Fixed
+
+- The garbage collector traces an enum value by its active variant rather than the union of every variant's pointer slots. An enum whose variants store a scalar (for example a `Float`) and a GC pointer (for example a `String`) in the same slot, such as a JSON value, previously had the scalar's bits followed as a pointer during a collection, corrupting the heap and crashing the program (a `SIGSEGV`, or a scheduler assertion under concurrency). The back end now registers a per-variant pointer mask and the collector selects it by the discriminant (#601).
+
 ## [2.18.120] - 2026-06-16
 
 Continuing the codex-review fix batch (one patch per issue).

Cargo.lock

@@ -2,7 +2,7 @@
 members = [".", "raven-runtime"]
 
 [workspace.package]
-version = "2.18.120"
+version = "2.18.121"
 edition = "2021"
 authors = ["martian56"]
 license = "MIT"

Cargo.toml

@@ -0,0 +1,56 @@
+// Regression for the enum GC-descriptor bug. `Json.Num` holds a `Float` (a
+// scalar) in the same slot where `Json.Text` holds a `String` (a GC pointer).
+// The collector must trace each value by its active variant, not the union of
+// every variant's pointer slots; otherwise it follows a `Num` value's raw
+// `Float` bits as if they were a pointer and corrupts the heap. The junk list
+// forces collections while many `Num` values are live.
+// Prints:
+//   nums=2666
+//   textlen=11634
+import std/list
+
+enum Json {
+    Num(Float)
+    Text(String)
+}
+
+fun main() {
+    let items: List<Json> = []
+    let i = 0
+    let f = 0.5
+    while i < 4000 {
+        if i % 3 == 0 {
+            items.push(Json.Text("item-${i}"))
+        } else {
+            items.push(Json.Num(f))
+        }
+        f = f + 1.5
+        i = i + 1
+    }
+
+    // Churn several megabytes of short-lived strings so collections run while
+    // the `Num`-bearing enums above stay live and reachable. Each collection
+    // traces `items`, which is where the buggy union mask followed a `Float`.
+    let churn = 0
+    let k = 0
+    while k < 80000 {
+        let s = "garbage-${k}-with-some-padding-bytes"
+        churn = churn + s.len()
+        k = k + 1
+    }
+
+    let nums = 0
+    let textlen = 0
+    for v in items {
+        match v {
+            Num(_) -> {
+                nums = nums + 1
+            }
+            Text(s) -> {
+                textlen = textlen + s.len()
+            }
+        }
+    }
+    print("nums=${nums}")
+    print("textlen=${textlen}")
+}

examples/v2/gc_enum_scalar_payload.rv

@@ -0,0 +1,2 @@
+nums=2666
+textlen=11634

examples/v2/gc_enum_scalar_payload.rv.out

@@ -557,6 +557,40 @@ fn struct_descriptor(type_id: u32) -> u64 {
         .unwrap_or(0)
 }
 
+/// Per-variant GC pointer descriptors for enum types, keyed by the enum's type
+/// id and then by the variant discriminant. Unlike a struct, an enum's pointer
+/// layout depends on the active variant: variant A may hold a `String` (a GC
+/// pointer) in a slot where variant B holds a `Float` (a scalar). A single
+/// per-type mask (the union) would trace the scalar's bits as a pointer, so the
+/// collector selects the mask by the value's discriminant instead. An enum's id
+/// appears only here, never in [`struct_descriptors`], so a present entry marks
+/// the value as an enum.
+fn enum_descriptors() -> &'static std::sync::RwLock<HashMap<u32, HashMap<u32, u64>>> {
+    static ENUM_DESCRIPTORS: std::sync::OnceLock<
+        std::sync::RwLock<HashMap<u32, HashMap<u32, u64>>>,
+    > = std::sync::OnceLock::new();
+    ENUM_DESCRIPTORS.get_or_init(|| std::sync::RwLock::new(HashMap::new()))
+}
+
+/// Register the GC pointer mask of one enum variant. `type_id` is the enum
+/// type's id (shared by every variant); `variant` is the discriminant; `mask`
+/// has bit `slot` set when that slot of this variant holds a GC pointer (slot 0
+/// is the discriminant and is never a pointer). The back end emits one call per
+/// constructed variant in the program entry point, before any value is built.
+///
+/// # Safety
+///
+/// Called only from the generated entry shim with back-end-computed arguments.
+#[no_mangle]
+pub extern "C" fn raven_enum_register(type_id: u32, variant: u32, mask: u64) {
+    enum_descriptors()
+        .write()
+        .unwrap()
+        .entry(type_id)
+        .or_default()
+        .insert(variant, mask);
+}
+
 /// Number of root slots currently registered. Test and diagnostic aid.
 #[cfg(test)]
 pub(crate) fn root_count() -> usize {
@@ -861,9 +895,24 @@ unsafe fn trace_object(object: *mut ObjectHeader, work: &mut Vec<*mut ObjectHead
             // SAFETY: tag confirms the struct layout. `len` is the field
             // count and `cap` is the per-type descriptor id.
             let (field_count, type_id) = unsafe { ((*object).len, (*object).cap) };
-            let mask = struct_descriptor(type_id);
+            let fields = (object as *const u8).wrapping_add(STRUCT_FIELDS_OFFSET);
+            // An enum's pointer layout depends on its active variant, so select
+            // the mask by the discriminant in slot 0; a plain struct uses its
+            // single per-type mask. Without this, a scalar payload (a `Float` in
+            // one variant) sharing a slot with a pointer payload (a `String` in
+            // another) would be traced as a pointer and corrupt the collector.
+            let mask = {
+                let enums = enum_descriptors().read().unwrap();
+                if let Some(variants) = enums.get(&type_id) {
+                    // SAFETY: an enum always has slot 0 (the discriminant).
+                    let disc = unsafe { *(fields as *const usize) } as u32;
+                    variants.get(&disc).copied().unwrap_or(0)
+                } else {
+                    drop(enums);
+                    struct_descriptor(type_id)
+                }
+            };
             if mask != 0 {
-                let fields = (object as *const u8).wrapping_add(STRUCT_FIELDS_OFFSET);
                 // The descriptor mask is 64 bits, so only the first 64 fields
                 // can be tracked. Cap the loop so a struct with more than 64
                 // fields does not shift `1u64 << i` past the word width (a panic
@@ -1610,6 +1659,68 @@ mod stress_tests {
         });
     }
 
+    #[test]
+    fn enum_traces_by_active_variant_not_union() {
+        isolated(|| {
+            // Enum type 41: variant 0 (Text) holds a String pointer at slot 1;
+            // variant 1 (Num) holds a scalar Float at slot 1. The union of the
+            // two masks marks slot 1 as a pointer, so tracing a `Num` value by
+            // the union would follow its raw Float bits. Per-variant tracing,
+            // selected by the discriminant in slot 0, must not.
+            raven_enum_register(41, 0, 0b10); // Text: slot 1 is a GC pointer
+            raven_enum_register(41, 1, 0b00); // Num: slot 1 is a scalar
+
+            // A `Num` value: discriminant 1, slot 1 = 1.0's raw f64 bits (which,
+            // as an address, would fault if dereferenced).
+            let num = raven_struct_new(2, 41);
+            // SAFETY: two slots; write discriminant then payload.
+            unsafe {
+                let f = raven_struct_fields(num) as *mut u64;
+                f.write(1);
+                f.add(1).write(0x3ff0000000000000);
+            }
+
+            // A `Text` value: discriminant 0, slot 1 = a real String pointer.
+            let s = marked_string(6, 0x30);
+            let text = raven_struct_new(2, 41);
+            // SAFETY: two slots; write discriminant then the String pointer.
+            unsafe {
+                let f = raven_struct_fields(text) as *mut u64;
+                f.write(0);
+                f.add(1).write(s as u64);
+            }
+
+            let mut slot_a: *mut u8 = num as *mut u8;
+            let mut slot_b: *mut u8 = text as *mut u8;
+            let mut roots: [*mut *mut u8; 2] =
+                [&mut slot_a as *mut *mut u8, &mut slot_b as *mut *mut u8];
+            raven_gc_enter_frame(roots.as_mut_ptr() as *mut *mut u8, 2);
+
+            // num + text + the String. The Num's float payload is not an object.
+            assert_eq!(raven_gc_live_objects(), 3);
+            // Every churn collection traces both enums. Before the fix the Num's
+            // float bits were followed as a pointer and the collector crashed.
+            churn(20_000, 256);
+            assert_eq!(raven_gc_live_objects(), 3);
+            // SAFETY: the Num's scalar slot is untouched, the Text's String is
+            // intact and still marked.
+            unsafe {
+                let num_payload = (raven_struct_fields(num) as *const u64).add(1).read();
+                assert_eq!(
+                    num_payload, 0x3ff0000000000000,
+                    "Num float payload corrupted"
+                );
+                let s_again = (raven_struct_fields(text) as *const u64).add(1).read()
+                    as *mut crate::object::String;
+                assert_eq!(s_again, s);
+            }
+            assert_marked(s, 6, 0x30);
+            raven_gc_leave_frame();
+            raven_gc_collect();
+            assert_eq!(raven_gc_live_objects(), 0);
+        });
+    }
+
     #[test]
     fn list_of_structs_survives_churn() {
         isolated(|| {

raven-runtime/src/gc.rs

@@ -99,6 +99,12 @@ pub struct ModuleCx {
     /// `main` shim registers every descriptor with the collector before
     /// running the program so a struct is always traceable.
     descriptors: HashMap<String, StructDescriptor>,
+    /// Per-variant GC pointer masks for enum types, keyed by the enum's type id
+    /// and then by variant discriminant. An enum is traced by its active
+    /// variant's mask (selected at run time by the discriminant), not the union
+    /// of every variant, so a scalar payload sharing a slot with a pointer
+    /// payload in another variant is not mistaken for a pointer.
+    enum_variants: HashMap<u32, BTreeMap<u32, u64>>,
     /// Counter handing out the next struct descriptor id.
     next_type_id: u32,
     /// Emitted vtables keyed by `<concrete_type>$<trait>`. Each value is
@@ -154,6 +160,7 @@ impl ModuleCx {
             string_counter: 0,
             main_entry: None,
             descriptors: HashMap::new(),
+            enum_variants: HashMap::new(),
             next_type_id: 0,
             vtables: HashMap::new(),
             vtable_counter: 0,
@@ -373,6 +380,17 @@ impl ModuleCx {
         type_id
     }
 
+    /// Record the GC pointer mask of one enum variant under its enum type id, so
+    /// the entry shim can register it with the collector for discriminant-aware
+    /// tracing. Idempotent: the back end always passes the same mask for a given
+    /// `(type_id, variant)`.
+    pub fn record_enum_variant(&mut self, type_id: u32, variant: u32, mask: u64) {
+        self.enum_variants
+            .entry(type_id)
+            .or_default()
+            .insert(variant, mask);
+    }
+
     /// Borrow the underlying Cranelift module for low level operations.
     pub fn module(&mut self) -> &mut ObjectModule {
         &mut self.module
@@ -485,6 +503,10 @@ impl ModuleCx {
         sig = self.make_sig(&[i32t, i64t], &[]);
         self.declare_runtime(intrinsics::RUNTIME_STRUCT_REGISTER, &sig)?;
 
+        // raven_enum_register(type_id: u32, variant: u32, ptr_mask: u64)
+        sig = self.make_sig(&[i32t, i32t, i64t], &[]);
+        self.declare_runtime(intrinsics::RUNTIME_ENUM_REGISTER, &sig)?;
+
         // raven_gc_enter_frame(roots: ptr, count: usize)
         sig = self.make_sig(&[ptr, ptr], &[]);
         self.declare_runtime(intrinsics::RUNTIME_GC_ENTER_FRAME, &sig)?;
@@ -915,9 +937,19 @@ impl ModuleCx {
         // builder borrows the function, so the closure below only needs
         // the resolved references.
         let descriptors: Vec<StructDescriptor> = self.descriptors.values().copied().collect();
+        // Flatten the enum variant masks into (type_id, variant, mask) triples
+        // for the entry shim to register.
+        let enum_regs: Vec<(u32, u32, u64)> = self
+            .enum_variants
+            .iter()
+            .flat_map(|(&tid, variants)| variants.iter().map(move |(&v, &m)| (tid, v, m)))
+            .collect();
         let register_id = self.runtime_id(intrinsics::RUNTIME_STRUCT_REGISTER);
         let register_ref =
             register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
+        let enum_register_id = self.runtime_id(intrinsics::RUNTIME_ENUM_REGISTER);
+        let enum_register_ref =
+            enum_register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
         let type_register_id = self.runtime_id(intrinsics::RUNTIME_TYPE_REGISTER);
         let type_register_ref =
             type_register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
@@ -987,6 +1019,16 @@ impl ModuleCx {
                     builder.ins().call(reg, &[id, mask]);
                 }
             }
+            // Register each enum variant's own mask for discriminant-aware
+            // tracing, so a value is traced by its active variant, not the union.
+            if let Some(ereg) = enum_register_ref {
+                for &(tid, variant, mask) in &enum_regs {
+                    let id = builder.ins().iconst(types::I32, tid as i64);
+                    let v = builder.ins().iconst(types::I32, variant as i64);
+                    let m = builder.ins().iconst(types::I64, mask as i64);
+                    builder.ins().call(ereg, &[id, v, m]);
+                }
+            }
             // Register runtime reflection metadata so a boxed value can
             // report its name and walk its fields.
             if let Some(treg) = type_register_ref {

src/codegen/context.rs

@@ -1238,6 +1238,11 @@ fn lower_enum_create(
     // their masks so any variant's pointer payload is traced.
     let key = ty.mangle();
     let type_id = cx.merge_descriptor(&key, mask);
+    // Record this variant's own mask so the collector traces an enum value by
+    // its active variant, not the union of all variants. Without this, a scalar
+    // payload (a `Float` in one variant) sharing a slot with a pointer payload
+    // (a `String` in another) would be traced as a GC pointer.
+    cx.record_enum_variant(type_id, variant as u32, mask);
 
     // Allocate the enum value first and root it while the payload is
     // evaluated, for the same reason struct creation does: a payload

src/codegen/function.rs

@@ -134,6 +134,11 @@ pub const RUNTIME_STRUCT_FIELDS: &str = "raven_struct_fields";
 /// descriptor.
 pub const RUNTIME_STRUCT_REGISTER: &str = "raven_struct_register";
 
+/// Runtime C symbol registering one enum variant's GC pointer descriptor, so
+/// the collector can pick the mask by the value's discriminant instead of
+/// tracing the union of every variant's pointers.
+pub const RUNTIME_ENUM_REGISTER: &str = "raven_enum_register";
+
 /// Runtime C symbol entering a GC root frame.
 pub const RUNTIME_GC_ENTER_FRAME: &str = "raven_gc_enter_frame";