feat(runtime): real function names in Error stacks — frame-pointer walk + the existing name registry (#9486)

.cargo/config.toml

@@ -27,3 +27,24 @@ global-min-publish-age = "7 days"
 # aborts loudly if the tables are missing (perry-runtime/src/eh.rs).
 [build]
 rustflags = ["-C", "force-unwind-tables=yes"]
+
+# #9486: x86_64 needs frame pointers, and cannot inherit them from `[build]`.
+#
+# `Error.stack` captures its frames by walking the `rbp` / `x29` chain, which
+# is only a chain if every frame between `new Error` and the throwing JS
+# function maintains one. Generated code always does — codegen tags it
+# `"frame-pointer"="non-leaf"` — but the runtime is Rust, and on
+# x86_64-unknown-linux-gnu rustc leaves `rbp` as a general-purpose
+# callee-saved register. Measured: `rbp` inside `alloc_error` held `0x1`, so
+# the walk had no root at all and every stack fell back to `at <anonymous>`.
+# The AArch64 platform ABI reserves x29, which is why this is a x86_64-only
+# knob and why the collector's own `fp_chain` walker was AArch64-only.
+#
+# `force-unwind-tables` is REPEATED here on purpose: cargo does not merge
+# `target.*.rustflags` with `build.rustflags` — the target flags take
+# precedence and the `[build]` list is dropped entirely for this target. Omit
+# the repeat and every throw crossing a runtime frame is stranded (the runtime
+# self-checks on the first `try` and aborts loudly, so the mistake is loud
+# rather than silent — but it is still a mistake).
+[target.'cfg(target_arch = "x86_64")']
+rustflags = ["-C", "force-unwind-tables=yes", "-C", "force-frame-pointers=yes"]

changelog.d/9486-error-stack-frames.md

@@ -0,0 +1,87 @@
+### Fixed
+
+- **`Error.prototype.stack` now carries real, named frames.** Every stack a
+  compiled program printed was one line — `    at <anonymous>` — where node
+  prints the call chain. `#9432`/`#9410` gave `.stack` its *existence*; nothing
+  ever looked at the native stack, so its *content* was a placeholder. That is
+  what made `claude doctor`'s raw-mode report and commander's parse-error
+  render useless (120 bytes of stderr against node's 14,573), and it is why
+  every divergence investigation that touched a compiled app had to reach for
+  gdb and a symbolized build.
+
+  Two halves, and the split between them is the design:
+
+  **Capture** is a frame-pointer chain walk on every `new Error` — two loads
+  per frame, no allocation, no symbolication. Codegen already tags generated
+  functions `"frame-pointer"="non-leaf"`, the property the collector's own
+  `fp_chain` walker relies on, so `[fp] = caller fp` / `[fp+8] = return
+  address` holds for JS frames. The captured addresses ride in a new
+  `ErrorHeader.frames` slot (a `StringHeader`, so it needs no new `GC_TYPE_*`
+  and no new rewrite-descriptor arm — one added `visit(...)` line in the
+  `GcRewriteDescriptorKind::Error` trace arm covers it).
+
+  **Resolution** happens on the first `.stack` read and reuses the registry
+  codegen already fills: `js_register_function_name` records
+  `(compiled address, JS display name)` once per function at module init
+  (72,713 entries for the claude-code bundle) so `fn.name` and `[Function: f]`
+  work. That table is keyed by exact function start; a return address points
+  into the middle of a function, so the resolver snapshots it into an
+  address-sorted vector once and answers containment with a binary search.
+  Codegen now registers the same name against the function BODY symbol
+  (`perry_fn_<prefix>__<name>`) as well as the wrapper — a direct call between
+  two compiled functions targets the body, so the wrapper address a closure
+  value carries is not what a return address points into. Both keys map to the
+  same name and `fn.name` still reads the wrapper key, so nothing that
+  consulted the registry before sees a different answer.
+
+  Building `.stack` eagerly is what the fix REMOVES: `alloc_error` used to
+  decode its own message from UTF-8 and allocate two `String`s per
+  construction to produce a line almost no program ever reads. Constructing a
+  million errors without reading `.stack` is now cheaper than before, not more
+  expensive, and the symbolication — the part that costs — happens only for
+  errors whose `.stack` is actually read, then memoises into the `stack` slot.
+
+  **Frames are named but not positioned.** A `file:line:col` needs a
+  per-return-address line table, an O(instructions) artifact against this
+  one's O(functions); a resolved frame renders as `    at <name> (<anonymous>)`
+  — V8's own spelling for a frame whose script position is unknown, which is
+  also the `name (location)` shape the stack-parsing libraries in real bundles
+  read a name out of. Frames the
+  resolver cannot attribute to a registered JS function — the runtime's own,
+  between `new Error` and the throwing code — are elided rather than printed
+  as bare addresses, the way node elides its internals; a capture in which
+  nothing resolves falls back to the pre-fix single `<anonymous>` line, so no
+  program loses what it had. Windows and any target without a guaranteed
+  frame-pointer chain keep the old behavior rather than guess at a frame
+  shape the ABI does not promise.
+
+  **Two limits worth knowing.** Inlining removes frames: `a() → b() → c()`
+  where all three are small folds into one function, so the trace names the
+  frame that survives rather than all three. V8 keeps inlined frames because it
+  retains inlining metadata for deoptimization; an ahead-of-time compiler has
+  no such record, and the frames that matter in real traces — the ones across
+  `try`/`catch`, callbacks and module boundaries — are exactly the ones the
+  inliner does not fold. And a frame is only as good as the registry's
+  coverage: an address inside a function nothing registered resolves to
+  whichever registered function precedes it, so this change also registers
+  class constructors, static methods and accessors, which previously had no
+  name of their own and were the ones a neighbour's name leaked into (measured:
+  a `new Widget()` frame came out labelled `main`).
+
+  Registering a name is gated on `LlModule::has_function`. `method_names` is
+  a DISPATCH registry, not an emission record — it carries keys this module
+  never defines a body for, and emitting a registration against one makes
+  module init reference an undefined global. The claude-code bundle found
+  exactly one, a getter (`UT7.__get_get_extensionName`) out of ~46k functions,
+  and failed to compile; nothing smaller than that bundle reproduced it.
+
+- **x86_64 builds now keep frame pointers.** The capture above walks the
+  `rbp` / `x29` chain, which is only a chain if every frame between
+  `new Error` and the throwing JS function maintains one. Generated code always
+  did; the runtime is Rust, and on `x86_64-unknown-linux-gnu` rustc leaves
+  `rbp` as a general-purpose callee-saved register — measured, `rbp` inside
+  `alloc_error` held `0x1`, so the walk had no root and every stack on that
+  target fell back to `at <anonymous>`. `.cargo/config.toml` now adds
+  `-C force-frame-pointers=yes` for x86_64 only; the AArch64 platform ABI
+  reserves `x29`, which is why the collector's own `fp_chain` walker was
+  AArch64-only and why this knob is not needed there.

crates/perry-codegen/src/codegen/artifacts.rs

@@ -1745,6 +1745,116 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
                 .map(|sym| (format!("__perry_wrap_{}", sym), display))
         })
         .collect();
+    // #9486: the same name against the function BODY symbol as well.
+    //
+    // The wrapper address above is what a closure VALUE carries, so it is what
+    // `fn.name` needs — but it is not what a return address on the native
+    // stack points into. A direct call from one compiled function to another
+    // targets `perry_fn_<prefix>__<name>` itself, so an `Error.stack` frame
+    // resolves against the body or against nothing at all. Both keys map to
+    // the same name, and `fn.name` still reads the wrapper key it always did,
+    // so nothing that consulted this registry before sees a different answer.
+    let body_symbol_display_names: Vec<(String, String)> = hir
+        .functions
+        .iter()
+        .filter_map(|f| {
+            let display = hir.closure_display_names.get(&f.id).cloned().or_else(|| {
+                if f.name.is_empty() || f.name.starts_with('_') {
+                    None
+                } else {
+                    Some(f.name.clone())
+                }
+            })?;
+            func_names
+                .get(&f.id)
+                .filter(|sym| llmod.has_function(sym))
+                .map(|sym| (sym.clone(), display))
+        })
+        .collect();
+    user_fn_display_names.extend(body_symbol_display_names);
+    // #9486: class methods, under the `Class.method` label node uses for a
+    // prototype-method frame. `method_names` is the map codegen itself keyed
+    // the emitted `perry_method_*` symbols by, and the `__perry_wrap_*`
+    // generator earlier in this function walks exactly this pair of loops
+    // with the same `.get(...)` guard — so every symbol here is one this module
+    // definitely emitted, which is the condition the #318/#343 "use of
+    // undefined value" class turns on. Only the BODY symbol is registered:
+    // the wrapper address is what `fn.name` reads, and giving a method a
+    // `.name` it never had is a separate, observable change.
+    {
+        let mut seen: std::collections::HashSet<String> = std::collections::HashSet::new();
+        // `method_names` is a DISPATCH registry, not an emission record: it
+        // carries entries this module never defines a body for (an accessor
+        // reached only through a cross-module or typed path, a stale key from
+        // a shape that lowered elsewhere). Registering one of those emits a
+        // `js_register_function_name(ptr @perry_method_…)` against a symbol
+        // that does not exist, and the module fails to build with "reference
+        // to unknown global" — measured on the claude-code bundle, where
+        // exactly one getter (`UT7.__get_get_extensionName`) had a registry
+        // key and no definition out of ~46k functions. `has_function` is the
+        // authority on what this module actually emitted, so every name below
+        // goes through it.
+        let mut push_defined = |symbol: String, display: String| {
+            if symbol.is_empty() || display.is_empty() || !llmod.has_function(&symbol) {
+                return;
+            }
+            if seen.insert(symbol.clone()) {
+                user_fn_display_names.push((symbol, display));
+            }
+        };
+        for class in &hir.classes {
+            for method in &class.methods {
+                let Some(symbol) = method_names
+                    .get(&(class.name.clone(), method.name.clone()))
+                    .cloned()
+                else {
+                    continue;
+                };
+                push_defined(symbol, format!("{}.{}", class.name, method.name));
+            }
+            for method in &class.static_methods {
+                let Some(symbol) = method_names
+                    .get(&(class.name.clone(), method.name.clone()))
+                    .cloned()
+                else {
+                    continue;
+                };
+                push_defined(symbol, format!("{}.{}", class.name, method.name));
+            }
+            // Accessors are keyed with the `__get_` / `__set_` prefix
+            // `method_registry` gives them, and node labels their frames
+            // `get x` / `set x`.
+            for (accessors, prefix, label) in [
+                (&class.getters, "__get_", "get"),
+                (&class.setters, "__set_", "set"),
+            ] {
+                for (prop, _) in accessors {
+                    let Some(symbol) = method_names
+                        .get(&(class.name.clone(), format!("{prefix}{prop}")))
+                        .cloned()
+                    else {
+                        continue;
+                    };
+                    push_defined(symbol, format!("{label} {prop}"));
+                }
+            }
+            // The constructor is registered in `method_names` under the
+            // synthesized `<Class>_constructor` method name (method_registry.rs
+            // emits one for EVERY class, explicit or not), and node labels its
+            // frame `new <Class>`.
+            //
+            // Registering these is not only about naming THEIR frames. A
+            // registry entry names a function START and carries no end, so an
+            // address inside an UNREGISTERED function resolves to whatever
+            // registered function precedes it — measured: a `new Widget()`
+            // frame came out labelled `main`. Every emitted function this list
+            // covers is one that can no longer borrow a neighbour's name.
+            let ctor_key = (class.name.clone(), format!("{}_constructor", class.name));
+            if let Some(symbol) = method_names.get(&ctor_key).cloned() {
+                push_defined(symbol, format!("new {}", class.name));
+            }
+        }
+    }
     // (b) Closures bound to a top-level `let`/`const`. #2076: a named
     // function expression's own name takes precedence over the binding
     // name (`const bar = function namedBar(){}` ⇒ `"namedBar"`).

crates/perry-codegen/src/codegen/string_pool.rs

@@ -257,11 +257,27 @@ pub(super) fn emit_string_pool(
     // Each entry becomes one `js_register_function_name(<sym>, <str>,
     // <len>)` call inside the init function. See #1202.
     let mut user_fn_name_constants: Vec<(String, String, usize)> = Vec::new();
+    // Deduplicated by CONTENT (#9486): the same display name is now registered
+    // against several symbols — a top-level function's wrapper and its body,
+    // a class method and its `__perry_wrap_*` twin — and `add_string_constant`
+    // mints a fresh `@.str.N` per call, so without this every extra
+    // registration also cost a duplicate copy of the name in rodata.
+    // Deterministic: the map only reuses a global the loop already minted in
+    // its (already sorted) input order, so emission order is unchanged (#7622).
+    let mut name_constant_cache: std::collections::HashMap<&str, (String, usize)> =
+        std::collections::HashMap::new();
     for (wrapper_sym, display_name) in user_fn_display_names {
         if wrapper_sym.is_empty() || display_name.is_empty() {
             continue;
         }
-        let (const_name, byte_len) = llmod.add_string_constant(display_name);
+        let (const_name, byte_len) = match name_constant_cache.get(display_name.as_str()) {
+            Some(hit) => hit.clone(),
+            None => {
+                let minted = llmod.add_string_constant(display_name);
+                name_constant_cache.insert(display_name.as_str(), minted.clone());
+                minted
+            }
+        };
         user_fn_name_constants.push((wrapper_sym.clone(), const_name, byte_len));
     }
 

crates/perry-ext-mysql2/src/lib.rs

@@ -1828,7 +1828,9 @@ mod tests {
                 runtime_string((*error).message),
                 "Invalid connection handle"
             );
-            let stack = runtime_string((*error).stack);
+            // #9486: through the accessor — the field is null until the
+            // first read materialises the string.
+            let stack = runtime_string(perry_runtime::error::js_error_get_stack(error));
             assert!(stack.contains("Error: Invalid connection handle"));
         }
     }

crates/perry-ext-sharp/src/lib.rs

@@ -1103,7 +1103,9 @@ mod tests {
                 perry_ffi::copy_string_from_raw(message),
                 "Invalid sharp handle"
             );
-            assert!(!(*error).stack.is_null());
+            // #9486: through the accessor — the field is null until the
+            // first read materialises the string.
+            assert!(!perry_runtime::error::js_error_get_stack(error).is_null());
         }
     }
 

crates/perry-runtime/src/builtins/formatting.rs

@@ -429,6 +429,28 @@ pub fn register_function_name_if_absent(func_ptr: usize, name: &str) {
     }
 }
 
+/// #9486: how many `(function address, name)` pairs the registry currently
+/// holds. Cheap enough to consult on every `.stack` read, so the stack-frame
+/// resolver can tell a stale address-sorted snapshot from a current one
+/// without cloning the table to compare it.
+pub fn function_name_registry_len() -> Option<usize> {
+    function_name_registry().lock().ok().map(|map| map.len())
+}
+
+/// #9486: snapshot the registry as `(function address, name bytes)` pairs for
+/// the `Error.stack` frame resolver to sort by address.
+///
+/// The `Arc` clones make this a pointer copy per entry rather than a name
+/// copy, and the lock is held only for the walk — resolution (a binary search
+/// per frame) happens outside it, so a `.stack` read never blocks a
+/// concurrent registration for longer than the snapshot itself.
+pub fn function_name_registry_entries() -> Option<Vec<(usize, std::sync::Arc<[u8]>)>> {
+    function_name_registry()
+        .lock()
+        .ok()
+        .map(|map| map.iter().map(|(k, v)| (*k, v.clone())).collect())
+}
+
 /// Look up the codegen-registered JS name for a function pointer.
 ///
 /// Returns the name registered by `js_register_function_name` (keyed on the
@@ -699,8 +721,16 @@ unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) ->
     }
 }
 
-unsafe fn format_error_stack_frame(error_ptr: *const crate::error::ErrorHeader) -> Option<String> {
-    let stack = string_header_to_string((*error_ptr).stack, "");
+/// The one stack line `util.inspect` shows under an error's headline.
+///
+/// #9486: through the accessor, never off the field — `alloc_error` leaves
+/// `stack` null and the first read materialises it, so a direct field read
+/// here made `console.log(err)` print no frame at all. It is called from
+/// `format_error_value` as the LAST use of `error_ptr` on purpose: the
+/// accessor allocates, and a moving scavenge during that allocation would
+/// leave any later read of `error_ptr` pointing at from-space.
+unsafe fn format_error_stack_frame(error_ptr: *mut crate::error::ErrorHeader) -> Option<String> {
+    let stack = string_header_to_string(crate::error::js_error_get_stack(error_ptr), "");
     stack
         .lines()
         .skip(1)
@@ -758,7 +788,7 @@ unsafe fn format_error_value(error_ptr: *const crate::error::ErrorHeader, depth:
     }
 
     let mut out = headline;
-    if let Some(frame) = format_error_stack_frame(error_ptr) {
+    if let Some(frame) = format_error_stack_frame(error_ptr as *mut _) {
         out.push('\n');
         out.push_str(&frame);
         out.push_str(" {");

crates/perry-runtime/src/builtins/mod.rs

@@ -144,7 +144,8 @@ pub(crate) use console::{
 pub(crate) use console::{test_console_instance_count, test_seed_console_instance};
 
 pub use formatting::{
-    function_name_for_ptr, function_source_for_func_ptr, function_source_for_ptr, js_array_print,
+    function_name_for_ptr, function_name_registry_entries, function_name_registry_len,
+    function_source_for_func_ptr, function_source_for_ptr, js_array_print,
     js_boxed_bigint_new, js_boxed_boolean_new, js_boxed_number_new, js_boxed_string_new,
     js_boxed_symbol_new, js_register_function_name, js_register_function_source, js_util_format,
     js_util_format_with_options, js_util_inspect, js_util_is_deep_strict_equal,