Skip to content

Commit d28ebc6

Browse files
committed
fix: trace enums by active variant so a scalar payload is not a GC root
An enum's GC pointer descriptor was the union of every variant's pointer slots. When two variants store different kinds of value in the same slot, one a scalar (a `Float`) and another a GC pointer (a `String`), the union marked that slot as a pointer. A collection then followed the scalar variant's raw bits as if they were an object pointer, writing GC bits to an arbitrary address: a segfault, or the `suspend_current` scheduler assertion when the corrupted memory was a coroutine. JSON values (a `Number(Float)` sharing a slot with `Str(String)`) and any `Result<Int, String>`-shaped enum hit this under collection pressure, which is why an allocation-heavy concurrent server crashed within seconds. The back end now registers a per-variant pointer mask via a new `raven_enum_register(type_id, variant, mask)` intrinsic, and the collector selects the mask by the value's discriminant (slot 0) instead of tracing the union. A plain struct still uses its single per-type mask. Adds a runtime unit test for discriminant-aware tracing and an end-to-end golden example (`gc_enum_scalar_payload`) that forces collections while `Float`-payload enums are live; it crashes on the old union mask and passes on the fix. Version 2.18.121. Fixes #601
1 parent 12a9519 commit d28ebc6

9 files changed

Lines changed: 232 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
All notable changes to Raven are documented in this file.
44

5+
## [2.18.121] - 2026-06-16
6+
7+
### Fixed
8+
9+
- 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).
10+
511
## [2.18.120] - 2026-06-16
612

713
Continuing the codex-review fix batch (one patch per issue).

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
members = [".", "raven-runtime"]
33

44
[workspace.package]
5-
version = "2.18.120"
5+
version = "2.18.121"
66
edition = "2021"
77
authors = ["martian56"]
88
license = "MIT"
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Regression for the enum GC-descriptor bug. `Json.Num` holds a `Float` (a
2+
// scalar) in the same slot where `Json.Text` holds a `String` (a GC pointer).
3+
// The collector must trace each value by its active variant, not the union of
4+
// every variant's pointer slots; otherwise it follows a `Num` value's raw
5+
// `Float` bits as if they were a pointer and corrupts the heap. The junk list
6+
// forces collections while many `Num` values are live.
7+
// Prints:
8+
// nums=2666
9+
// textlen=11634
10+
import std/list
11+
12+
enum Json {
13+
Num(Float)
14+
Text(String)
15+
}
16+
17+
fun main() {
18+
let items: List<Json> = []
19+
let i = 0
20+
let f = 0.5
21+
while i < 4000 {
22+
if i % 3 == 0 {
23+
items.push(Json.Text("item-${i}"))
24+
} else {
25+
items.push(Json.Num(f))
26+
}
27+
f = f + 1.5
28+
i = i + 1
29+
}
30+
31+
// Churn several megabytes of short-lived strings so collections run while
32+
// the `Num`-bearing enums above stay live and reachable. Each collection
33+
// traces `items`, which is where the buggy union mask followed a `Float`.
34+
let churn = 0
35+
let k = 0
36+
while k < 80000 {
37+
let s = "garbage-${k}-with-some-padding-bytes"
38+
churn = churn + s.len()
39+
k = k + 1
40+
}
41+
42+
let nums = 0
43+
let textlen = 0
44+
for v in items {
45+
match v {
46+
Num(_) -> {
47+
nums = nums + 1
48+
}
49+
Text(s) -> {
50+
textlen = textlen + s.len()
51+
}
52+
}
53+
}
54+
print("nums=${nums}")
55+
print("textlen=${textlen}")
56+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
nums=2666
2+
textlen=11634

raven-runtime/src/gc.rs

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,40 @@ fn struct_descriptor(type_id: u32) -> u64 {
557557
.unwrap_or(0)
558558
}
559559

560+
/// Per-variant GC pointer descriptors for enum types, keyed by the enum's type
561+
/// id and then by the variant discriminant. Unlike a struct, an enum's pointer
562+
/// layout depends on the active variant: variant A may hold a `String` (a GC
563+
/// pointer) in a slot where variant B holds a `Float` (a scalar). A single
564+
/// per-type mask (the union) would trace the scalar's bits as a pointer, so the
565+
/// collector selects the mask by the value's discriminant instead. An enum's id
566+
/// appears only here, never in [`struct_descriptors`], so a present entry marks
567+
/// the value as an enum.
568+
fn enum_descriptors() -> &'static std::sync::RwLock<HashMap<u32, HashMap<u32, u64>>> {
569+
static ENUM_DESCRIPTORS: std::sync::OnceLock<
570+
std::sync::RwLock<HashMap<u32, HashMap<u32, u64>>>,
571+
> = std::sync::OnceLock::new();
572+
ENUM_DESCRIPTORS.get_or_init(|| std::sync::RwLock::new(HashMap::new()))
573+
}
574+
575+
/// Register the GC pointer mask of one enum variant. `type_id` is the enum
576+
/// type's id (shared by every variant); `variant` is the discriminant; `mask`
577+
/// has bit `slot` set when that slot of this variant holds a GC pointer (slot 0
578+
/// is the discriminant and is never a pointer). The back end emits one call per
579+
/// constructed variant in the program entry point, before any value is built.
580+
///
581+
/// # Safety
582+
///
583+
/// Called only from the generated entry shim with back-end-computed arguments.
584+
#[no_mangle]
585+
pub extern "C" fn raven_enum_register(type_id: u32, variant: u32, mask: u64) {
586+
enum_descriptors()
587+
.write()
588+
.unwrap()
589+
.entry(type_id)
590+
.or_default()
591+
.insert(variant, mask);
592+
}
593+
560594
/// Number of root slots currently registered. Test and diagnostic aid.
561595
#[cfg(test)]
562596
pub(crate) fn root_count() -> usize {
@@ -861,9 +895,24 @@ unsafe fn trace_object(object: *mut ObjectHeader, work: &mut Vec<*mut ObjectHead
861895
// SAFETY: tag confirms the struct layout. `len` is the field
862896
// count and `cap` is the per-type descriptor id.
863897
let (field_count, type_id) = unsafe { ((*object).len, (*object).cap) };
864-
let mask = struct_descriptor(type_id);
898+
let fields = (object as *const u8).wrapping_add(STRUCT_FIELDS_OFFSET);
899+
// An enum's pointer layout depends on its active variant, so select
900+
// the mask by the discriminant in slot 0; a plain struct uses its
901+
// single per-type mask. Without this, a scalar payload (a `Float` in
902+
// one variant) sharing a slot with a pointer payload (a `String` in
903+
// another) would be traced as a pointer and corrupt the collector.
904+
let mask = {
905+
let enums = enum_descriptors().read().unwrap();
906+
if let Some(variants) = enums.get(&type_id) {
907+
// SAFETY: an enum always has slot 0 (the discriminant).
908+
let disc = unsafe { *(fields as *const usize) } as u32;
909+
variants.get(&disc).copied().unwrap_or(0)
910+
} else {
911+
drop(enums);
912+
struct_descriptor(type_id)
913+
}
914+
};
865915
if mask != 0 {
866-
let fields = (object as *const u8).wrapping_add(STRUCT_FIELDS_OFFSET);
867916
// The descriptor mask is 64 bits, so only the first 64 fields
868917
// can be tracked. Cap the loop so a struct with more than 64
869918
// fields does not shift `1u64 << i` past the word width (a panic
@@ -1610,6 +1659,68 @@ mod stress_tests {
16101659
});
16111660
}
16121661

1662+
#[test]
1663+
fn enum_traces_by_active_variant_not_union() {
1664+
isolated(|| {
1665+
// Enum type 41: variant 0 (Text) holds a String pointer at slot 1;
1666+
// variant 1 (Num) holds a scalar Float at slot 1. The union of the
1667+
// two masks marks slot 1 as a pointer, so tracing a `Num` value by
1668+
// the union would follow its raw Float bits. Per-variant tracing,
1669+
// selected by the discriminant in slot 0, must not.
1670+
raven_enum_register(41, 0, 0b10); // Text: slot 1 is a GC pointer
1671+
raven_enum_register(41, 1, 0b00); // Num: slot 1 is a scalar
1672+
1673+
// A `Num` value: discriminant 1, slot 1 = 1.0's raw f64 bits (which,
1674+
// as an address, would fault if dereferenced).
1675+
let num = raven_struct_new(2, 41);
1676+
// SAFETY: two slots; write discriminant then payload.
1677+
unsafe {
1678+
let f = raven_struct_fields(num) as *mut u64;
1679+
f.write(1);
1680+
f.add(1).write(0x3ff0000000000000);
1681+
}
1682+
1683+
// A `Text` value: discriminant 0, slot 1 = a real String pointer.
1684+
let s = marked_string(6, 0x30);
1685+
let text = raven_struct_new(2, 41);
1686+
// SAFETY: two slots; write discriminant then the String pointer.
1687+
unsafe {
1688+
let f = raven_struct_fields(text) as *mut u64;
1689+
f.write(0);
1690+
f.add(1).write(s as u64);
1691+
}
1692+
1693+
let mut slot_a: *mut u8 = num as *mut u8;
1694+
let mut slot_b: *mut u8 = text as *mut u8;
1695+
let mut roots: [*mut *mut u8; 2] =
1696+
[&mut slot_a as *mut *mut u8, &mut slot_b as *mut *mut u8];
1697+
raven_gc_enter_frame(roots.as_mut_ptr() as *mut *mut u8, 2);
1698+
1699+
// num + text + the String. The Num's float payload is not an object.
1700+
assert_eq!(raven_gc_live_objects(), 3);
1701+
// Every churn collection traces both enums. Before the fix the Num's
1702+
// float bits were followed as a pointer and the collector crashed.
1703+
churn(20_000, 256);
1704+
assert_eq!(raven_gc_live_objects(), 3);
1705+
// SAFETY: the Num's scalar slot is untouched, the Text's String is
1706+
// intact and still marked.
1707+
unsafe {
1708+
let num_payload = (raven_struct_fields(num) as *const u64).add(1).read();
1709+
assert_eq!(
1710+
num_payload, 0x3ff0000000000000,
1711+
"Num float payload corrupted"
1712+
);
1713+
let s_again = (raven_struct_fields(text) as *const u64).add(1).read()
1714+
as *mut crate::object::String;
1715+
assert_eq!(s_again, s);
1716+
}
1717+
assert_marked(s, 6, 0x30);
1718+
raven_gc_leave_frame();
1719+
raven_gc_collect();
1720+
assert_eq!(raven_gc_live_objects(), 0);
1721+
});
1722+
}
1723+
16131724
#[test]
16141725
fn list_of_structs_survives_churn() {
16151726
isolated(|| {

src/codegen/context.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ pub struct ModuleCx {
9999
/// `main` shim registers every descriptor with the collector before
100100
/// running the program so a struct is always traceable.
101101
descriptors: HashMap<String, StructDescriptor>,
102+
/// Per-variant GC pointer masks for enum types, keyed by the enum's type id
103+
/// and then by variant discriminant. An enum is traced by its active
104+
/// variant's mask (selected at run time by the discriminant), not the union
105+
/// of every variant, so a scalar payload sharing a slot with a pointer
106+
/// payload in another variant is not mistaken for a pointer.
107+
enum_variants: HashMap<u32, BTreeMap<u32, u64>>,
102108
/// Counter handing out the next struct descriptor id.
103109
next_type_id: u32,
104110
/// Emitted vtables keyed by `<concrete_type>$<trait>`. Each value is
@@ -154,6 +160,7 @@ impl ModuleCx {
154160
string_counter: 0,
155161
main_entry: None,
156162
descriptors: HashMap::new(),
163+
enum_variants: HashMap::new(),
157164
next_type_id: 0,
158165
vtables: HashMap::new(),
159166
vtable_counter: 0,
@@ -373,6 +380,17 @@ impl ModuleCx {
373380
type_id
374381
}
375382

383+
/// Record the GC pointer mask of one enum variant under its enum type id, so
384+
/// the entry shim can register it with the collector for discriminant-aware
385+
/// tracing. Idempotent: the back end always passes the same mask for a given
386+
/// `(type_id, variant)`.
387+
pub fn record_enum_variant(&mut self, type_id: u32, variant: u32, mask: u64) {
388+
self.enum_variants
389+
.entry(type_id)
390+
.or_default()
391+
.insert(variant, mask);
392+
}
393+
376394
/// Borrow the underlying Cranelift module for low level operations.
377395
pub fn module(&mut self) -> &mut ObjectModule {
378396
&mut self.module
@@ -485,6 +503,10 @@ impl ModuleCx {
485503
sig = self.make_sig(&[i32t, i64t], &[]);
486504
self.declare_runtime(intrinsics::RUNTIME_STRUCT_REGISTER, &sig)?;
487505

506+
// raven_enum_register(type_id: u32, variant: u32, ptr_mask: u64)
507+
sig = self.make_sig(&[i32t, i32t, i64t], &[]);
508+
self.declare_runtime(intrinsics::RUNTIME_ENUM_REGISTER, &sig)?;
509+
488510
// raven_gc_enter_frame(roots: ptr, count: usize)
489511
sig = self.make_sig(&[ptr, ptr], &[]);
490512
self.declare_runtime(intrinsics::RUNTIME_GC_ENTER_FRAME, &sig)?;
@@ -915,9 +937,19 @@ impl ModuleCx {
915937
// builder borrows the function, so the closure below only needs
916938
// the resolved references.
917939
let descriptors: Vec<StructDescriptor> = self.descriptors.values().copied().collect();
940+
// Flatten the enum variant masks into (type_id, variant, mask) triples
941+
// for the entry shim to register.
942+
let enum_regs: Vec<(u32, u32, u64)> = self
943+
.enum_variants
944+
.iter()
945+
.flat_map(|(&tid, variants)| variants.iter().map(move |(&v, &m)| (tid, v, m)))
946+
.collect();
918947
let register_id = self.runtime_id(intrinsics::RUNTIME_STRUCT_REGISTER);
919948
let register_ref =
920949
register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
950+
let enum_register_id = self.runtime_id(intrinsics::RUNTIME_ENUM_REGISTER);
951+
let enum_register_ref =
952+
enum_register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
921953
let type_register_id = self.runtime_id(intrinsics::RUNTIME_TYPE_REGISTER);
922954
let type_register_ref =
923955
type_register_id.map(|id| self.module.declare_func_in_func(id, &mut ctx.func));
@@ -987,6 +1019,16 @@ impl ModuleCx {
9871019
builder.ins().call(reg, &[id, mask]);
9881020
}
9891021
}
1022+
// Register each enum variant's own mask for discriminant-aware
1023+
// tracing, so a value is traced by its active variant, not the union.
1024+
if let Some(ereg) = enum_register_ref {
1025+
for &(tid, variant, mask) in &enum_regs {
1026+
let id = builder.ins().iconst(types::I32, tid as i64);
1027+
let v = builder.ins().iconst(types::I32, variant as i64);
1028+
let m = builder.ins().iconst(types::I64, mask as i64);
1029+
builder.ins().call(ereg, &[id, v, m]);
1030+
}
1031+
}
9901032
// Register runtime reflection metadata so a boxed value can
9911033
// report its name and walk its fields.
9921034
if let Some(treg) = type_register_ref {

src/codegen/function.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,6 +1238,11 @@ fn lower_enum_create(
12381238
// their masks so any variant's pointer payload is traced.
12391239
let key = ty.mangle();
12401240
let type_id = cx.merge_descriptor(&key, mask);
1241+
// Record this variant's own mask so the collector traces an enum value by
1242+
// its active variant, not the union of all variants. Without this, a scalar
1243+
// payload (a `Float` in one variant) sharing a slot with a pointer payload
1244+
// (a `String` in another) would be traced as a GC pointer.
1245+
cx.record_enum_variant(type_id, variant as u32, mask);
12411246

12421247
// Allocate the enum value first and root it while the payload is
12431248
// evaluated, for the same reason struct creation does: a payload

src/codegen/intrinsics.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ pub const RUNTIME_STRUCT_FIELDS: &str = "raven_struct_fields";
134134
/// descriptor.
135135
pub const RUNTIME_STRUCT_REGISTER: &str = "raven_struct_register";
136136

137+
/// Runtime C symbol registering one enum variant's GC pointer descriptor, so
138+
/// the collector can pick the mask by the value's discriminant instead of
139+
/// tracing the union of every variant's pointers.
140+
pub const RUNTIME_ENUM_REGISTER: &str = "raven_enum_register";
141+
137142
/// Runtime C symbol entering a GC root frame.
138143
pub const RUNTIME_GC_ENTER_FRAME: &str = "raven_gc_enter_frame";
139144

0 commit comments

Comments
 (0)