diff --git a/changelog.d/7376-symbol-description-stale.md b/changelog.d/7376-symbol-description-stale.md new file mode 100644 index 0000000000..eb27c78a6f --- /dev/null +++ b/changelog.d/7376-symbol-description-stale.md @@ -0,0 +1,26 @@ +### Fixed + +- **A `Symbol`'s description was stored into the header already stale.** + `alloc_symbol` calls `gc_malloc` — a collection point — and then writes the + description pointer its caller computed *before* that call. An evacuating + minor moves the description string, so a live `SymbolHeader` holds a retired + from-space address and `js_symbol_to_string` faults reading it through + `str_from_header`. + + The description is now rooted across the allocation and re-read, the same fix + as `RegExpHeader::flags_ptr` (#7374). + + Closes **6 of the 31** catches in #7341, not 3: the same stale field is read + by two different helpers. `js_symbol_to_string` reaches it directly, and + `infer_symbol_function_name` reaches it through + `js_object_literal_infer_computed_function_name` — which had been triaged as a + separate cluster until the fix closed both. + + **A related gap is left open deliberately, and is worth knowing about.** The + header is allocated `GC_TYPE_STRING`, whose payload the collector treats as + opaque — so a fresh (non-registered) symbol's description is never marked or + rewritten after construction. The existing comment says so: *"kept alive + through the SYMBOL_REGISTRY (for registered symbols) or not at all (for fresh + symbols — in practice they live for the duration of the program, which is fine + for test workloads)"*. This change makes the stored value correct; keeping it + alive for the symbol's lifetime is a separate fix, tracked in #7341. diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 5a6503d02b..51a1566ac8 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -377,10 +377,27 @@ pub(crate) unsafe fn alloc_symbol( // SYMBOL_REGISTRY (for registered symbols) or not at all (for fresh // symbols — in practice they live for the duration of the program, // which is fine for test workloads). + // #7341: `gc_malloc` below is a collection point, and `description` was + // computed by the caller before it. An evacuating minor there relocates the + // description string, and the pre-collection address is then written into + // the header — permanently stale in a live symbol, exactly the shape fixed + // for `RegExpHeader::flags_ptr`. `js_symbol_to_string` reads it through + // `str_from_header` and faults on retired from-space; that is 3 of the 31 + // catches in #7341. + // + // Root across the allocation and re-read. NOTE the remaining gap the + // comment above describes and this does not close: the payload is opaque to + // the collector (`GC_TYPE_STRING`), so a fresh symbol's description is + // neither marked nor rewritten afterwards. Rooting here makes the STORED + // value correct; keeping it alive for the symbol's lifetime is a separate + // fix, tracked in #7341. + let scope = crate::gc::RuntimeHandleScope::new(); + let desc_root = scope.root_string_ptr(description); let raw = crate::gc::gc_malloc( std::mem::size_of::(), crate::gc::GC_TYPE_STRING, ); + let description = desc_root.get_raw_mut_ptr::(); let ptr = raw as *mut SymbolHeader; (*ptr).magic = SYMBOL_MAGIC; (*ptr).registered = if registered { 1 } else { 0 };