diff --git a/changelog.d/9410-error-subclass-stack.md b/changelog.d/9410-error-subclass-stack.md new file mode 100644 index 0000000000..0d935f4307 --- /dev/null +++ b/changelog.d/9410-error-subclass-stack.md @@ -0,0 +1,77 @@ +### Fixed + +- **An `Error` subclass now has a `.stack` and reports `[object Error]`.** + `class A extends Error {}` produced instances whose `.stack` was `undefined` + and whose `Object.prototype.toString` tag was `"[object Object]"`. The base + class was fine — `new Error("x").stack` has always been a string — so only + subclasses were affected, and the claude-code bundle has **93** of them and + **106** `.stack` reads. `claude doctor` printed ~10 real frames and 14,573 + bytes of stderr under node; under perry it printed ` - at ` + and 120 bytes. Silent: no error, just a missing trace. + + One root cause behind both symptoms. `class A extends Error {}` deliberately + produces an ordinary `GC_TYPE_OBJECT` class instance rather than a + `GC_TYPE_ERROR` `ErrorHeader`, so that the subclass's own fields have + somewhere to live. `alloc_error` — the only place that fills + `ErrorHeader.stack` — is therefore never reached, and neither is any + `stack` on `Error.prototype`, which carries only `name` and `message`. The + `[object Error]` branch of `js_object_to_string` is keyed on that same GC + header byte, so a subclass fell through to the `class_id` block and out the + `"[object Object]"` default. + + The class-id registry that answers this question already existed and was + wired at four other sites — `instanceof Error`, `util.types.isNativeError`, + `Error.prototype.toString`'s subclass arm, and prototype-chain resolution + all consult `extends_builtin_error(class_id)`. Neither the tag nor the stack + did. + + - `crates/perry-runtime/src/object/to_string_tag.rs` — tag a + `extends_builtin_error` class instance `"Error"`, set *before* the + `Symbol.toStringTag` hook so a subclass's own tag still wins (§20.1.3.6 + consults the tag property last). + - `crates/perry-runtime/src/error_subclass_stack.rs` (new; `error.rs` was + within 90 lines of the 2,000-line CI cap) — `js_error_subclass_capture_stack` + installs the own, non-enumerable, configurable `stack` accessor node + installs, capturing the FRAME at the construction site. The head + (`"name: message"`) is formatted on read, not at capture, because that is + what V8 does and what the ubiquitous + `constructor(m) { super(m); this.name = "X" }` shape needs: node reports + `"X: m"`, and the assignment happens after `super()` returns. A user + `Error.prepareStackTrace` still wins, as it does for + `Error.captureStackTrace`. The setter redefines `stack` as a plain data + property, so `err.stack = ""` keeps working. + - `crates/perry-runtime/src/object/class_constructors.rs` — install it from + `js_error_subclass_default_init` (the synthesized standalone ctor, which + also serves the dynamic-parent `super` path) and from + `default_error_init_for_implicit_chain` (the dynamic `new` replay), the + two runtime sites that already stamped `message`/`name` and stopped there. + In the replay the install is moved above the message guard, which returns + early for a no-argument `new X()` — exactly the instances that would + otherwise still have no trace. + - `crates/perry-codegen/src/expr/this_super_call.rs`, + `crates/perry-codegen/src/lower_call/new_error_init.rs` (new; the + static-`new` Error arm moved out of `new.rs`, which was 5 lines from the + 2,000-line CI gate) — the same call from the two codegen sites that stamp + `message`/`name` inline: an explicit `super(message)` into a built-in + Error, and the static-`new` arm for a subclass with no own constructor. + `this` is reloaded from its slot first; the stamps above it can collect. + + A unit test in the new module installs the accessor under forced evacuation, + which is the only condition that can expose an unrooted pointer — and which + caught the first cut of that rooting reading a NaN-box handle back with + `get_raw_const_ptr`, aborting every Error-subclass construction with + "runtime handle kind mismatch". Nothing in the unit suite constructed an + Error subclass before, so only a compiled probe saw it. + + Validation: `test-files/test_gap_9410_error_subclass_stack.ts` + byte-compared against `node --experimental-strip-types` across a bare + subclass, a `this.name`-assigning subclass, one with an extra field, a + two-level subclass, a subclass that sets `message` after an argument-less + `super()`, `TypeError`/`RangeError` subclasses, a factory-constructed + instance, a caught throw, `Error.captureStackTrace` on a subclass, and + controls for the base `Error`, a non-Error class and a plain object. The + fixture asserts the portable parts of the contract — `typeof stack`, the + head line, the `toString` tag, `name`/`message`/`instanceof`, and that + `stack` is an own but non-enumerable property that stays out of + `Object.keys` — because stack CONTENTS are host-specific. Demonstrated + failing on a compiler built from unfixed `origin/main` (46 diverging lines). diff --git a/changelog.d/9412-cjs-entry-next-tick-order.md b/changelog.d/9412-cjs-entry-next-tick-order.md new file mode 100644 index 0000000000..a803fbf821 --- /dev/null +++ b/changelog.d/9412-cjs-entry-next-tick-order.md @@ -0,0 +1,66 @@ +### Fixed + +- **A `require()` of a builtin no longer demotes `process.nextTick` below + promise microtasks.** + + ```js + require("path"); // delete this line and perry matched node + const o = []; + process.nextTick(() => o.push("nextTick")); + Promise.resolve().then(() => o.push("p1")); + (async () => { await null; o.push("await"); })(); + setTimeout(() => console.log(JSON.stringify(o)), 20); + // node: ["nextTick","p1","await"] + // perry: ["p1","await","nextTick"] (5/5 deterministic) + ``` + + The deferral itself is correct, and measurement says so: the same file run + by node 26 as `.cjs` prints `["nextTick","p1","await"]`, as `.mjs` + `["p1","await","nextTick"]`. An ES module evaluates inside its module job's + promise chain, so its first tick drain lands after the promise queue — which + is exactly what `js_mark_entry_module_esm` (#788) models. It was being + applied to the wrong module kind. + + Entry codegen decided "is this an ES module?" with + `!hir.imports.is_empty() || !hir.exports.is_empty() || has_top_level_await`. + A bare `require(` with no top-level `import` classifies the entry as + CommonJS, and `cjs_wrap` then rewrites it to ESM — injecting + `import { createRequire as __perry_cjs_create_require } from 'node:module'` + and `export default _cjs`. Both halves of that predicate became true for + every CommonJS program. The `require("path")` call itself contributes no + import at all; it folds to a native-module reference. Every real bundle + requires a builtin and every minimal fixture does not, so the ordering was + right in exactly the programs a test suite contains and wrong in exactly the + programs users run. + + - `crates/perry-codegen/src/collectors/cjs_scaffolding.rs` — + `is_cjs_wrapped_module`, keyed on the local name the wrap's synthetic + `createRequire` import binds. Recognised from the HIR, not from an + expectation about the template: if the wrap stops emitting it the + predicate degrades to "not wrapped" (today's behaviour) rather than to a + wrong answer for hand-written ESM, and a user's own + `import { createRequire } from 'node:module'` is not mistaken for it + because the match is on the alias, not the specifier. + - `crates/perry-codegen/src/codegen/entry.rs` — gate only the + `js_mark_entry_module_esm` call on that. The `is_esm_entry` below it keeps + its meaning for GlobalDeclarationInstantiation: a CommonJS module's + top-level `function` declarations live inside the module wrapper and are + not global-object properties either, so "not a Script" stays the right + answer there — and that predicate is mirrored in `perry-hir`'s + `lower_module_fn`, which runs before the wrap flag is knowable in codegen. + - `crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs` — + a template canary in the same family as #7139/#7152: rename the local in + `wrap.rs` and every CommonJS entry silently goes back to ES-module tick + ordering with nothing going red. Plus a negative control, so the fix + cannot drift the other way and give real ESM entries CommonJS ordering. + + Validation: `test-files/test_gap_9412_require_builtin_tick_order.cts` + byte-compared against node — ticks first, a tick scheduled from inside a tick + joining the same drain, a tick scheduled from inside a microtask landing + after it, and a second event-loop turn where no evaluation checkpoint could + apply. It has to be a `.cts`: this repo is `"type": "module"`, so a plain + `.ts` is an ES module for node and perry alike and cannot carry the shape + (#9418 taught the runner to discover `.cts`). + `test-files/test_gap_9412_entry_tick_order.ts` pins the ESM side so the fix + cannot be "stop deferring, always". Demonstrated failing on a compiler built + from unfixed `origin/main`. diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 6da576dbce..54ce1be20d 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -1046,7 +1046,31 @@ pub(super) fn compile_module_entry( // first microtask drain finishes promise/queueMicrotask jobs before // the nextTick queue, matching Node's job-within-checkpoint ordering // for ESM evaluation (#788). CJS-style entries keep ticks-first. - if !hir.imports.is_empty() || !hir.exports.is_empty() || hir.has_top_level_await { + // + // #9412: "has imports or exports" is not the same question for a + // CommonJS entry, because `cjs_wrap` gives every CommonJS file BOTH — + // a synthetic `import { createRequire as __perry_cjs_create_require } + // from 'node:module'` and an `export default _cjs`. So any entry + // containing a bare `require(` answered "ESM" here and ran its + // `process.nextTick` callbacks AFTER the promise queue, where Node + // runs a CommonJS program's ticks first. Measured against Node 26: + // an entry as `.cjs` prints ["tick","promise","await"], the same file + // as `.mjs` prints ["promise","await","tick"] — the deferral is right, + // it was just being applied to the wrong module kind. Every real + // bundle requires a builtin and every minimal fixture doesn't, so the + // ordering was correct in exactly the programs a test suite contains. + // + // Only this checkpoint is re-gated. `is_esm_entry` below keeps its + // original meaning for GlobalDeclarationInstantiation: a CommonJS + // module's top-level `function` declarations live inside the module + // wrapper and are NOT global-object properties either, so "not a + // Script" is the right answer there for a wrapped entry too — and + // that predicate is mirrored in `perry-hir`'s `lower_module_fn`, + // which runs before the wrap flag is knowable here. + let cjs_wrapped_entry = crate::collectors::is_cjs_wrapped_module(hir); + if (!hir.imports.is_empty() || !hir.exports.is_empty() || hir.has_top_level_await) + && !cjs_wrapped_entry + { ctx.block().call_void("js_mark_entry_module_esm", &[]); } // Initialize static class fields with their declared init diff --git a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs index a89fcaa948..8801eec9c0 100644 --- a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs +++ b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs @@ -617,6 +617,44 @@ fn for_each_stmt(stmts: &[Stmt], f: &mut dyn FnMut(&Stmt)) { } } +/// The local name `cjs_wrap` binds its synthetic `createRequire` import to. +/// +/// Mirrors the `imports` prefix in +/// `perry/src/commands/compile/cjs_wrap/wrap.rs` — a wrapped module always +/// opens with +/// `import { createRequire as __perry_cjs_create_require } from 'node:module';` +/// and nothing else in the pipeline ever mints that local. The `perry` crate's +/// template canary +/// (`commands/compile/cjs_wrap/preamble_canary_tests.rs`) asserts the wrap +/// still emits it, so a template edit fails a test instead of silently +/// un-recognising every CommonJS entry. +pub const CJS_WRAP_CREATE_REQUIRE_LOCAL: &str = "__perry_cjs_create_require"; + +/// True when `module` is the output of `cjs_wrap`'s CommonJS-to-ESM rewrite +/// rather than a module the user wrote with `import`/`export`. +/// +/// #9412: `is_esm_entry` asks "does this module have imports or exports?", and +/// the wrap gives EVERY CommonJS file both — a synthetic `node:module` import +/// and an `export default _cjs`. A CommonJS entry therefore answered "yes" and +/// took Node's *ES-module* `process.nextTick` ordering (ticks after the promise +/// queue drains) when Node runs it with *CommonJS* ordering (ticks first). +/// +/// Recognised from the HIR, not from an expectation about the wrap template: +/// if the template stops emitting this binding the predicate degrades to +/// "not wrapped" — today's behaviour — rather than to a wrong answer for +/// hand-written ESM. +pub fn is_cjs_wrapped_module(module: &Module) -> bool { + module.imports.iter().any(|import| { + import.specifiers.iter().any(|specifier| { + matches!( + specifier, + perry_hir::ImportSpecifier::Named { local, .. } + if local == CJS_WRAP_CREATE_REQUIRE_LOCAL + ) + }) + }) +} + #[cfg(test)] mod tests { use super::super::ptr_shape::collect_shape_proven_ptr_locals; diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 41247478a4..83df07fe53 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -57,7 +57,10 @@ mod this_as_value; mod uppercase_strings; // Public re-exports for the visible API. -pub use cjs_scaffolding::{census as cjs_preamble_census, CjsPreambleCensus}; +pub use cjs_scaffolding::{ + census as cjs_preamble_census, is_cjs_wrapped_module, CjsPreambleCensus, + CJS_WRAP_CREATE_REQUIRE_LOCAL, +}; pub use clamp_detect::{detect_clamp3, detect_clamp_u8, returns_i32_identity_arg, returns_integer}; // Internal-to-crate re-exports — explicit names because globs don't diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index d26184c700..724cd2a0ef 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -1327,6 +1327,26 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &this_handle), (DOUBLE, opts_val)], ); } + // #9410: `stack`. `super(message)` into a built-in + // Error stamps `message`/`name`/`cause` onto the + // already-allocated plain instance and stops there, + // so `new (class extends Error {})("x").stack` was + // `undefined` while `new Error("x").stack` is a + // string. The frame is captured HERE, at the + // construction site; the `name: message` head is + // formatted on read, because a subclass + // constructor assigns `this.name` after `super()` + // returns and Node reports the assigned name. + let blk = ctx.block(); + // Reload `this` from its slot: the stamps above + // can collect, and a DOUBLE held across a + // collecting call is the bare-pointer hazard + // #8770 is about. + let this_for_stack = blk.load(DOUBLE, &this_slot); + blk.call_void( + "js_error_subclass_capture_stack", + &[(DOUBLE, &this_for_stack)], + ); } } bind_derived_this_after_super(ctx); diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 9bcb7326ee..46b144be39 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -218,3 +218,25 @@ pub fn module_exported_return_shapes( pub fn cjs_preamble_census(hir: &perry_hir::Module) -> CjsPreambleCensus { collectors::cjs_preamble_census(hir) } + +/// #9412 template-change canary: the local name `cjs_wrap` binds its synthetic +/// `createRequire` import to. +/// +/// [`crate::collectors::is_cjs_wrapped_module`] keys the CommonJS-entry +/// recognition on this name, and the entry codegen keys the +/// `process.nextTick`-vs-microtask ordering on that. The `perry` crate's +/// template canary asserts the wrap still emits it, so a template edit fails a +/// test rather than silently putting every CommonJS entry back on ES-module +/// tick ordering. +pub fn cjs_wrap_create_require_local() -> &'static str { + collectors::CJS_WRAP_CREATE_REQUIRE_LOCAL +} + +/// #9412: is `hir` the output of `cjs_wrap`'s CommonJS-to-ESM rewrite? +/// +/// Public for the `perry` crate's template canary, alongside +/// [`cjs_wrap_create_require_local`]. The compile pipeline reaches the same +/// predicate through `collectors::is_cjs_wrapped_module`. +pub fn module_is_cjs_wrapped(hir: &perry_hir::Module) -> bool { + collectors::is_cjs_wrapped_module(hir) +} diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index 3720d62662..d9832bc16a 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -80,6 +80,7 @@ mod native_table; mod new; pub(crate) mod new_alloc; mod new_ctor_args; +mod new_error_init; mod new_helpers; pub(crate) use new_helpers::emit_ctor_return_override; mod omitted_native_params; diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index 66831eb586..fdc93a03c5 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -1489,75 +1489,11 @@ fn lower_new_impl_inner<'a>( .get(class_name) .map(|ctor| ctor.stops_constructor_walk()) .unwrap_or(false); - if !found_inherited_ctor && !imported_ctor_has_body_or_fields { - // Trace the chain to find the first Error-like ancestor name. - let mut error_kind: Option = None; - let mut cur = class.extends_name.clone(); - let mut depth = 0usize; - while let Some(pname) = cur { - if matches!( - pname.as_str(), - "Error" - | "TypeError" - | "RangeError" - | "ReferenceError" - | "SyntaxError" - | "URIError" - | "EvalError" - | "AggregateError" - ) { - error_kind = Some(pname); - break; - } - cur = ctx - .classes - .get(pname.as_str()) - .and_then(|c| c.extends_name.clone()); - depth += 1; - if depth > 32 { - break; - } - } - if let Some(kind) = error_kind { - let this_slot_for_err = ctx.this_stack.last().cloned().unwrap_or_default(); - let blk = ctx.block(); - let this_box = blk.load(DOUBLE, &this_slot_for_err); - let this_bits = blk.bitcast_double_to_i64(&this_box); - let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); - if let Some(msg_val) = lowered_args.first() { - let key_idx = ctx.strings.intern("message"); - let key_handle_global = - format!("@{}", ctx.strings.entry(key_idx).handle_global); - let blk = ctx.block(); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); - // Spec: built-in Error sets `message` non-enumerable via - // DefinePropertyOrThrow (Test262 NativeError/*-message). - blk.call_void( - "js_object_set_field_by_name_nonenum", - &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], - ); - } - let name_idx = ctx.strings.intern("name"); - let name_handle_global = format!("@{}", ctx.strings.entry(name_idx).handle_global); - let name_val_idx = ctx.strings.intern(&kind); - let name_val_global = format!("@{}", ctx.strings.entry(name_val_idx).handle_global); - let blk = ctx.block(); - let name_key_box = blk.load(DOUBLE, &name_handle_global); - let name_key_bits = blk.bitcast_double_to_i64(&name_key_box); - let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64); - let name_val_box = blk.load(DOUBLE, &name_val_global); - blk.call_void( - "js_object_set_field_by_name", - &[ - (I64, &this_handle), - (I64, &name_key_raw), - (DOUBLE, &name_val_box), - ], - ); - found_inherited_ctor = true; // skip the imported-ctor fallback below - } + if !found_inherited_ctor + && !imported_ctor_has_body_or_fields + && super::new_error_init::emit_default_error_init(ctx, class, &lowered_args) + { + found_inherited_ctor = true; // skip the imported-ctor fallback below } if let Some(runtime_fn) = builtin_parent_runtime { let undef_lit = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); diff --git a/crates/perry-codegen/src/lower_call/new_error_init.rs b/crates/perry-codegen/src/lower_call/new_error_init.rs new file mode 100644 index 0000000000..86fd0d208b --- /dev/null +++ b/crates/perry-codegen/src/lower_call/new_error_init.rs @@ -0,0 +1,109 @@ +//! `new (msg)` — the spec default Error-init for a class with +//! no own constructor whose ancestor walk terminates at a native Error family +//! base (#573). +//! +//! Split out of `new.rs` to keep that file under the 2,000-line CI gate +//! (`scripts/check_file_size.sh`); the body is a pure move, plus the #9410 +//! `stack` install. + +use perry_hir::Class; + +use crate::expr::FnCtx; +use crate::nanbox::POINTER_MASK_I64; +use crate::types::{DOUBLE, I64}; + +/// Stamp `message`, `name` and `stack` onto the freshly allocated instance of +/// an Error-family subclass, mirroring the `SuperCall` Error-like arm in +/// `expr/this_super_call.rs`. +/// +/// Returns `true` when the class's `extends` chain does terminate at an Error +/// family base and the init was emitted — the caller then skips its +/// imported-ctor fallback. Returns `false` (emitting nothing) otherwise. +pub(super) fn emit_default_error_init( + ctx: &mut FnCtx, + class: &Class, + lowered_args: &[String], +) -> bool { + // Trace the chain to find the first Error-like ancestor name. + let mut error_kind: Option = None; + let mut cur = class.extends_name.clone(); + let mut depth = 0usize; + while let Some(pname) = cur { + if matches!( + pname.as_str(), + "Error" + | "TypeError" + | "RangeError" + | "ReferenceError" + | "SyntaxError" + | "URIError" + | "EvalError" + | "AggregateError" + ) { + error_kind = Some(pname); + break; + } + cur = ctx + .classes + .get(pname.as_str()) + .and_then(|c| c.extends_name.clone()); + depth += 1; + if depth > 32 { + break; + } + } + if let Some(kind) = error_kind { + let this_slot_for_err = ctx.this_stack.last().cloned().unwrap_or_default(); + let blk = ctx.block(); + let this_box = blk.load(DOUBLE, &this_slot_for_err); + let this_bits = blk.bitcast_double_to_i64(&this_box); + let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); + if let Some(msg_val) = lowered_args.first() { + let key_idx = ctx.strings.intern("message"); + let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, &key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); + // Spec: built-in Error sets `message` non-enumerable via + // DefinePropertyOrThrow (Test262 NativeError/*-message). + blk.call_void( + "js_object_set_field_by_name_nonenum", + &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], + ); + } + let name_idx = ctx.strings.intern("name"); + let name_handle_global = format!("@{}", ctx.strings.entry(name_idx).handle_global); + let name_val_idx = ctx.strings.intern(&kind); + let name_val_global = format!("@{}", ctx.strings.entry(name_val_idx).handle_global); + let blk = ctx.block(); + let name_key_box = blk.load(DOUBLE, &name_handle_global); + let name_key_bits = blk.bitcast_double_to_i64(&name_key_box); + let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64); + let name_val_box = blk.load(DOUBLE, &name_val_global); + blk.call_void( + "js_object_set_field_by_name", + &[ + (I64, &this_handle), + (I64, &name_key_raw), + (DOUBLE, &name_val_box), + ], + ); + // #9410: `stack`. This arm stamps `message` and `name` onto an + // ordinary class instance; nothing ever filled `stack`, so + // `new MyError("x").stack` was `undefined` where the base + // `new Error("x").stack` is a string. The runtime installs a + // lazily-formatted own accessor and captures the FRAME here, + // at the construction site. + let blk = ctx.block(); + // Reload `this`: the `message`/`name` stamps above can + // collect, so the earlier `this_box` may be stale (#8770). + let this_for_stack = blk.load(DOUBLE, &this_slot_for_err); + blk.call_void( + "js_error_subclass_capture_stack", + &[(DOUBLE, &this_for_stack)], + ); + return true; + } + false +} diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index 645665f81d..536d04d56c 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -133,6 +133,10 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { // #5127: apply ES2022 `cause` from a `super(message, options)` forward to // a user Error-subclass instance (a generic object). (this_handle, options) module.declare_function("js_error_apply_cause_to_object", VOID, &[I64, DOUBLE]); + // #9410: install the own, lazily-formatted `stack` accessor Node gives an + // `Error` subclass instance, capturing the frame at the construction site. + // (this) + module.declare_function("js_error_subclass_capture_stack", VOID, &[DOUBLE]); module.declare_function("js_with_has_binding", I32, &[DOUBLE, I64]); module.declare_function("js_with_get_binding", DOUBLE, &[DOUBLE, I64]); module.declare_function("js_with_set_binding", DOUBLE, &[DOUBLE, I64, DOUBLE, I32]); diff --git a/crates/perry-runtime/src/error.rs b/crates/perry-runtime/src/error.rs index f0b1e05d2d..0efcecc28a 100644 --- a/crates/perry-runtime/src/error.rs +++ b/crates/perry-runtime/src/error.rs @@ -1862,6 +1862,10 @@ static KEEP_AGGREGATEERROR_NEW_FULL: extern "C" fn( #[used] static KEEP_ERROR_IS_ERROR: extern "C" fn(f64) -> f64 = js_error_is_error; +#[path = "error_subclass_stack.rs"] +mod subclass_stack; +pub use subclass_stack::js_error_subclass_capture_stack; + #[cfg(test)] #[path = "error_tostring_tests.rs"] mod tostring_tests; diff --git a/crates/perry-runtime/src/error_subclass_stack.rs b/crates/perry-runtime/src/error_subclass_stack.rs new file mode 100644 index 0000000000..556df23556 --- /dev/null +++ b/crates/perry-runtime/src/error_subclass_stack.rs @@ -0,0 +1,389 @@ +//! #9410 — the own `stack` an `Error` SUBCLASS instance gets at construction. +//! +//! Split out of `error.rs` to keep that file under the 2,000-line CI cap +//! (`scripts/check_file_size.sh`). Included from there with +//! `#[path = "error_subclass_stack.rs"] mod subclass_stack;` plus a +//! `pub use`, so `use super::*` resolves against `error.rs` and the +//! `#[no_mangle]` entry keeps its symbol. +//! +//! `class A extends Error {}` produces an ordinary `GC_TYPE_OBJECT` class +//! instance, not a `GC_TYPE_ERROR` `ErrorHeader` — deliberately, so the +//! subclass's own fields have somewhere to live. `alloc_error`, the only site +//! that fills `ErrorHeader.stack`, is therefore never reached, and +//! `Error.prototype` carries `name` and `message` but no `stack`. So +//! `new A("x").stack` was `undefined` while `new Error("x").stack` was a +//! string. + +use super::*; + +/// #9410: read `key` off `obj` as an owned `String`, or `None` when absent / +/// undefined / null. Local twin of `value::to_string`'s private helper — the +/// lazy subclass `stack` getter needs `name` and `message` the same way +/// `Error.prototype.toString` does. +unsafe fn error_object_field_string( + obj: *const crate::object::ObjectHeader, + key: &[u8], +) -> Option { + let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); + let v = crate::object::js_object_get_field_by_name(obj, key_ptr); + if v.is_undefined() || v.is_null() { + return None; + } + let s_ptr = crate::value::js_jsvalue_to_string(f64::from_bits(v.bits())); + if s_ptr.is_null() { + return None; + } + Some(read_string_header_owned(s_ptr)) +} + +/// `": "` for an Error SUBCLASS instance, following +/// `Error.prototype.toString` (§20.5.3.4) exactly as `value::to_string`'s +/// subclass arm does: `name` alone when `message` is empty, `message` alone +/// when `name` is empty. Absent `name` defaults to `"Error"`, matching the +/// value a subclass inherits from `Error.prototype`. +unsafe fn error_subclass_stack_head(receiver: f64) -> String { + let receiver_ptr = crate::value::js_nanbox_get_pointer(receiver); + if receiver_ptr == 0 + || !crate::value::addr_class::is_above_handle_band(receiver_ptr as usize) + || !crate::object::is_valid_obj_ptr(receiver_ptr as *const u8) + { + return "Error".to_string(); + } + // Each field read allocates (the key, and the ToString of a non-string + // value), so the receiver is re-read through a handle between them rather + // than held as a raw pointer across a collection. + let scope = crate::gc::RuntimeHandleScope::new(); + let handle = scope.root_nanbox_f64(receiver); + let obj = || { + crate::value::js_nanbox_get_pointer(handle.get_nanbox_f64()) + as *const crate::object::ObjectHeader + }; + let name = error_object_field_string(obj(), b"name").unwrap_or_else(|| "Error".to_string()); + let message = error_object_field_string(obj(), b"message").unwrap_or_default(); + if name.is_empty() { + message + } else if message.is_empty() { + name + } else { + format!("{name}: {message}") + } +} + +/// Lazy `stack` getter for an Error SUBCLASS instance (#9410). +/// +/// Capture slot 0 holds the frame string captured at CONSTRUCTION; the head +/// (`": "`) is formatted HERE, on read, because that is what V8 +/// does — `class E extends Error { constructor(m) { super(m); this.name = "E" }}` +/// reports `"E: m"`, and the assignment happens after `super()` returns. A +/// user `Error.prepareStackTrace` still wins, same as `captureStackTrace`'s +/// getter. +extern "C" fn error_subclass_stack_getter(closure: *const crate::closure::ClosureHeader) -> f64 { + let receiver = crate::object::js_implicit_this_get(); + unsafe { + if let Some(prep) = error_prepare_stack_trace_override() { + let structured = build_structured_stack(10); + let prep_ptr = + crate::value::js_nanbox_get_pointer(prep) as *const crate::closure::ClosureHeader; + return crate::closure::js_closure_call2(prep_ptr, receiver, structured); + } + let frame = { + let bits = crate::closure::js_closure_get_capture_bits(closure, 0); + let ptr = (bits & crate::value::POINTER_MASK) as *const StringHeader; + if ptr.is_null() + || !crate::value::addr_class::is_above_handle_band(ptr as usize) + || !crate::object::is_valid_obj_ptr(ptr as *const u8) + { + current_stack_frame() + } else { + read_string_header_owned(ptr) + } + }; + let head = error_subclass_stack_head(receiver); + let s = format!("{head}\n{frame}"); + let ptr = js_string_from_bytes(s.as_ptr(), s.len() as u32); + crate::value::js_nanbox_string(ptr as i64) + } +} + +/// Setter half of the subclass `stack` accessor (#9410). +/// +/// Node's `stack` is writable — `err.stack = ""` is a common way to shorten a +/// diagnostic, and a getter-only property would turn that into a silent no-op +/// (sloppy mode) or a TypeError (strict). Redefine it as a plain +/// non-enumerable own data property: after a write the lazy formatting is +/// gone, and reads return exactly what was assigned, which is the observable +/// contract. (V8 keeps the accessor shape and stores into an internal slot, +/// so `getOwnPropertyDescriptor(err, "stack")` after a write still reports +/// `get`/`set` there and reports `value`/`writable` here. Same reads, same +/// enumerability, different reflection — a deliberate simplification.) +extern "C" fn error_subclass_stack_setter( + _closure: *const crate::closure::ClosureHeader, + value: f64, +) -> f64 { + let receiver = crate::object::js_implicit_this_get(); + let ptr = crate::value::js_nanbox_get_pointer(receiver); + if ptr != 0 + && crate::value::addr_class::is_above_handle_band(ptr as usize) + && crate::object::is_valid_obj_ptr(ptr as *const u8) + { + // The key allocation can collect, so both the receiver and the value + // are re-read through handles after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let this_handle = scope.root_nanbox_f64(receiver); + let value_handle = scope.root_nanbox_f64(value); + let key_handle = scope.root_string_ptr(js_string_from_bytes(b"stack".as_ptr(), 5)); + let target = crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader; + crate::object::clear_accessor_descriptor(target as usize, "stack"); + // The generic field setter can grow property storage or invoke user + // code. The accessor's receiver is an ordinary Error-subclass object + // and `stack` remains an own key after its descriptor is cleared, so + // this route reaches the setter's self-rooting ordinary-object tail. + // Still, put the whole call inside nested `across_*` windows: only + // scoped entry pointers feed it, and both the key and receiver used by + // the post-call attribute write are re-read after it returns. + let (((), stack_key), receiver) = this_handle.across_nanbox(|| { + key_handle.across_const::(|| { + key_handle.with_const_ptr::(|key| { + let target = crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader; + crate::object::js_object_set_field_by_name( + target, + key, + value_handle.get_nanbox_f64(), + ); + }) + }) + }); + let target = + crate::value::js_nanbox_get_pointer(receiver) as *mut crate::object::ObjectHeader; + if !stack_key.is_null() + && crate::value::addr_class::is_above_handle_band(target as usize) + && crate::object::is_valid_obj_ptr(target as *const u8) + { + crate::object::set_property_attrs( + target as usize, + unsafe { read_string_header_owned(stack_key) }, + crate::object::PropertyAttrs::new(true, false, true), + ); + } + } + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +/// #9410: give an `Error` SUBCLASS instance the own `stack` property Node +/// gives it. +/// +/// `class X extends Error {}` produces an ordinary `GC_TYPE_OBJECT` class +/// instance, never a `GC_TYPE_ERROR` `ErrorHeader` — deliberately, so the +/// subclass's own fields have somewhere to live — and `alloc_error`, the only +/// place that fills `ErrorHeader.stack`, is therefore never reached. Neither +/// is `Error.prototype`, which carries `name` and `message` but no `stack`. +/// So `new X("m").stack` was `undefined` while `new Error("m").stack` was a +/// string: the cc bundle's 93 `extends Error` classes lost every trace they +/// printed. +/// +/// Installed as an accessor rather than a precomputed string for the reason +/// the getter documents: the head is `name`/`message` at READ time, and a +/// subclass constructor almost always assigns `this.name` after `super()`. +/// The FRAME is captured here, at construction, which is the part that would +/// be wrong if it were deferred. +/// +/// Idempotent and defensive: a non-pointer receiver, a receiver that already +/// has its own `stack` (a subclass with a `stack` class field, or a second +/// call on the same instance), and a failed closure allocation all return +/// without touching anything. +#[no_mangle] +pub extern "C" fn js_error_subclass_capture_stack(this_val: f64) { + unsafe { + let ptr = crate::value::js_nanbox_get_pointer(this_val); + if ptr == 0 + || !crate::value::addr_class::is_above_handle_band(ptr as usize) + || !crate::object::is_valid_obj_ptr(ptr as *const u8) + { + return; + } + // Every heap value that outlives an allocation below gets a handle: + // the receiver, the `"stack"` key and the captured frame string all + // survive two closure births, and the moving scavenge relocates + // anything it is not shown. (The `alloc_error` twin above roots for + // exactly this reason.) + let scope = crate::gc::RuntimeHandleScope::new(); + let this_handle = scope.root_nanbox_f64(this_val); + let key_handle = scope.root_string_ptr(js_string_from_bytes(b"stack".as_ptr(), 5)); + + let target = crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader; + if key_handle.with_const_ptr::(|stack_key| { + crate::object::object_ops::own_key_present(target, stack_key) + }) { + return; + } + + // Capture the frame NOW — this is the whole point of installing at + // construction rather than formatting the string on first read. + let frame = current_stack_frame(); + let frame_ptr = js_string_from_bytes(frame.as_ptr(), frame.len() as u32); + if frame_ptr.is_null() { + return; + } + let frame_handle = scope.root_string_ptr(frame_ptr); + + let getter_fn = error_subclass_stack_getter as *const u8; + let setter_fn = error_subclass_stack_setter as *const u8; + crate::closure::js_register_closure_arity(getter_fn, 0); + crate::closure::js_register_closure_arity(setter_fn, 1); + let getter = crate::closure::js_closure_alloc(getter_fn, 1); + if getter.is_null() { + return; + } + let getter_handle = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(getter as i64)); + let setter = crate::closure::js_closure_alloc(setter_fn, 0); + if setter.is_null() { + return; + } + let setter_handle = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(setter as i64)); + + // Re-read every pointer through its handle: the closure births above + // may have moved all of them. `getter_handle` is a NaN-box handle, so + // it is unboxed rather than read as a raw-pointer handle + // (`get_raw_const_ptr` on a nanbox slot panics with "runtime handle + // kind mismatch"). + let getter_ptr = crate::value::js_nanbox_get_pointer(getter_handle.get_nanbox_f64()) + as *mut crate::closure::ClosureHeader; + frame_handle.with_const_ptr::(|frame| { + crate::closure::js_closure_set_capture_bits( + getter_ptr, + 0, + crate::value::js_nanbox_string(frame as i64).to_bits(), + ); + }); + + // The key-array append can grow (and therefore allocate), so it runs + // BEFORE the closure bits and the descriptor-table key — an address + // recorded ahead of it would be the pre-move one. + // `ensure_key_in_keys_array` can allocate, but roots both incoming + // pointers at entry. Scope the entry key and re-read it only after the + // whole operation; no pre-call receiver pointer survives the closure. + let (((), stack_key), receiver) = this_handle.across_nanbox(|| { + key_handle.across_const::(|| { + key_handle.with_const_ptr::(|key| { + crate::object::ensure_key_in_keys_array( + crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader, + key, + ); + }) + }) + }); + let descriptor_key = if stack_key.is_null() { + "stack".to_string() + } else { + read_string_header_owned(stack_key) + }; + let target = + crate::value::js_nanbox_get_pointer(receiver) as *mut crate::object::ObjectHeader; + crate::object::set_builtin_accessor_descriptor( + target as usize, + descriptor_key, + crate::object::AccessorDescriptor { + get: getter_handle.get_nanbox_f64().to_bits(), + set: setter_handle.get_nanbox_f64().to_bits(), + }, + // writable is N/A for an accessor; Node's `stack` is + // non-enumerable and configurable. + crate::object::PropertyAttrs::new(true, false, true), + ); + } +} + +/// Generated-code-only callee (#9410): anchor against the auto-optimize LTO +/// dead-strip. +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_ERROR_SUBCLASS_CAPTURE_STACK: extern "C" fn(f64) = js_error_subclass_capture_stack; + +#[cfg(test)] +mod tests { + use super::*; + + fn str_ptr(bytes: &[u8]) -> *mut StringHeader { + js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + } + + /// #9410. Installing the accessor allocates twice AFTER the frame string is + /// born and after the receiver is in hand, so every one of those values has + /// to be reachable through a handle rather than held as a raw pointer. This + /// test forces evacuation so a collection actually relocates them, which is + /// the only condition under which an unrooted pointer misbehaves. + /// + /// It also catches the plainer failure the first cut of that rooting shipped + /// with: reading a NaN-box handle back with `get_raw_const_ptr` aborts the + /// process with "runtime handle kind mismatch", so EVERY Error-subclass + /// construction panicked. Nothing in the unit suite constructed a subclass, + /// so only a compiled fixture saw it. + #[test] + fn capture_stack_installs_a_non_enumerable_own_accessor() { + unsafe { + let _copying_nursery = crate::gc::CopyingNurseryTestGuard::new(0); + let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _force_evacuation = crate::gc::knob_overrides::ForcedEvacuationTestGuard::on(); + crate::gc::register_runtime_handle_root_scanner_for_tests(); + + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = crate::object::js_object_alloc(0, 4); + let this_handle = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as i64)); + + let obj_now = || { + crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader + }; + crate::object::js_object_set_field_by_name( + obj_now(), + str_ptr(b"name"), + crate::value::js_nanbox_string(str_ptr(b"Named") as i64), + ); + crate::object::js_object_set_field_by_name_nonenum( + obj_now(), + str_ptr(b"message"), + crate::value::js_nanbox_string(str_ptr(b"boom") as i64), + ); + + js_error_subclass_capture_stack(this_handle.get_nanbox_f64()); + + let target = obj_now(); + assert!( + crate::object::object_ops::own_key_present(target, str_ptr(b"stack")), + "`stack` must be an OWN key — node reports \ + hasOwnProperty(err, 'stack') === true for an Error subclass" + ); + let accessor = crate::object::get_accessor_descriptor(target as usize, "stack") + .expect("an accessor descriptor must be installed for `stack`"); + assert_ne!(accessor.get, 0, "the lazy getter half must be installed"); + assert_ne!( + accessor.set, 0, + "the setter half must be installed — `err.stack = \"\"` must not \ + become a silent no-op" + ); + let attrs = crate::object::get_property_attrs(target as usize, "stack") + .expect("`stack` must carry explicit attributes, not the enumerable default"); + assert!( + !attrs.enumerable(), + "`stack` must stay out of Object.keys / JSON.stringify" + ); + assert!(attrs.configurable(), "node's `stack` is configurable"); + + // Idempotent: a second capture (the dynamic-`new` replay can reach + // an instance the super-call path already stamped) must not replace + // the construction-time frame with a later one. + let first_getter = accessor.get; + js_error_subclass_capture_stack(this_handle.get_nanbox_f64()); + let again = crate::object::get_accessor_descriptor(obj_now() as usize, "stack") + .expect("the accessor must survive a second capture"); + assert_eq!( + again.get, first_getter, + "a second capture must leave the first frame in place" + ); + } + } +} diff --git a/crates/perry-runtime/src/object/class_constructors.rs b/crates/perry-runtime/src/object/class_constructors.rs index 30f787101e..37b2b33ea2 100644 --- a/crates/perry-runtime/src/object/class_constructors.rs +++ b/crates/perry-runtime/src/object/class_constructors.rs @@ -1051,7 +1051,15 @@ unsafe fn default_error_init_for_implicit_chain( args_ptr: *const f64, args_len: usize, ) { - if !crate::object::extends_builtin_error(class_cid) || args_ptr.is_null() || args_len == 0 { + if !crate::object::extends_builtin_error(class_cid) { + return; + } + // #9410: the dynamic replay path is a construction site like any other, + // so the instance gets its own lazily-formatted `stack` here — before the + // message guard below, which returns early for `new X()` with no argument + // and would otherwise leave exactly those instances trace-less. + crate::error::js_error_subclass_capture_stack(crate::value::js_nanbox_pointer(inst as i64)); + if args_ptr.is_null() || args_len == 0 { return; } let msg = *args_ptr; @@ -1113,6 +1121,11 @@ pub unsafe extern "C" fn js_error_subclass_default_init( let key = crate::string::js_string_from_bytes(b"name".as_ptr(), b"name".len() as u32); crate::object::js_object_set_field_by_name(inst, key, name_boxed); } + // #9410: `stack`. The synthesized standalone ctor stamps `message` and + // `name` but installed nothing for `stack`, so `new X("m").stack` was + // `undefined` for every `class X extends Error {}` with no own + // constructor. Last, so the getter's head sees the `name` just written. + crate::error::js_error_subclass_capture_stack(this_val); } /// Keepalive: generated code is the only caller (#6469). diff --git a/crates/perry-runtime/src/object/to_string_tag.rs b/crates/perry-runtime/src/object/to_string_tag.rs index 2b1bdd5770..da985ed099 100644 --- a/crates/perry-runtime/src/object/to_string_tag.rs +++ b/crates/perry-runtime/src/object/to_string_tag.rs @@ -385,6 +385,24 @@ pub unsafe extern "C" fn js_object_to_string(value: f64) -> f64 { tag_str = Some("RegExp String Iterator".to_string()); } else if class_id == crate::object::namespace_create::MODULE_NAMESPACE_CLASS_ID { tag_str = Some("Module".to_string()); + } else if class_id != 0 && crate::object::extends_builtin_error(class_id) { + // #9410: §20.1.3.6 picks `builtinTag` from the internal slot, + // and an Error SUBCLASS instance has [[ErrorData]] — it is + // just not a `GC_TYPE_ERROR` cell here (it is an ordinary + // class instance, so the subclass's own fields have somewhere + // to live), which is why the header-typed branch above misses + // it and `Object.prototype.toString.call(new (class extends + // Error {})())` reported "[object Object]" while the base + // `Error` reported "[object Error]". + // + // Set BEFORE the `Symbol.toStringTag` hook below, not after: + // the spec consults the tag property last and lets it win, so + // a subclass that declares `[Symbol.toStringTag]` still + // overrides this. `extends_builtin_error` is the same + // class-id chain walk that already backs `instanceof Error`, + // `util.types.isNativeError` and `Error.prototype.toString` + // for these instances. + tag_str = Some("Error".to_string()); } if let Some(func_ptr) = lookup_to_string_tag_hook(class_id) { let getter: extern "C" fn(f64) -> f64 = std::mem::transmute(func_ptr as *const u8); diff --git a/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs b/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs index 249ac98013..88f08698cb 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs @@ -261,3 +261,62 @@ fn computed_relative_requires_are_joined_against_the_module_dir() { './chunks/N.js' will miss it (#8040)" ); } + +// ── #9412: the wrap's synthetic `createRequire` import ───────────────────── + +/// The entry codegen decides `process.nextTick`-vs-microtask ordering from +/// "is this entry an ES module?", and the wrap answers yes for every CommonJS +/// file because it injects an import and an export. #9412 re-gates that on +/// `collectors::is_cjs_wrapped_module`, which recognises a wrapped module by +/// the local name the synthetic `node:module` import binds. +/// +/// Nothing else links the two. Rename the local in `wrap.rs` and every +/// CommonJS entry silently goes back to ES-module tick ordering — ticks after +/// the promise queue, where Node runs them first — with no test failing and no +/// error, which is exactly how #9412 survived in the first place. +#[test] +fn the_wrap_still_binds_the_local_the_cjs_entry_recogniser_keys_on() { + let local = perry_codegen::cjs_wrap_create_require_local(); + let path = Path::new("/tmp/perry-canary/node_modules/dep/index.js"); + let wrapped = wrap_commonjs_for_target(CJS_FIXTURE, path, None); + + // Anti-vacuity: the template must still emit the binding at all. + assert!( + wrapped.contains(local), + "the CJS wrap no longer binds `{local}`.\n\ + `is_cjs_wrapped_module` in perry-codegen/src/collectors/cjs_scaffolding.rs \ + keys on that local to tell a CommonJS entry from a hand-written ES module; \ + without it every CommonJS entry runs `process.nextTick` after the promise \ + queue again (#9412). Rename it in both places, or replace the recogniser." + ); + + let hir = wrap_and_lower(CJS_FIXTURE); + assert!( + perry_codegen::module_is_cjs_wrapped(&hir), + "a wrapped CommonJS module is no longer recognised as one. The wrap's \ + synthetic `node:module` import survived the source, but not lowering — \ + compare `is_cjs_wrapped_module`'s specifier match against what \ + `perry_hir::lower_module` produces for the wrap's import prefix." + ); +} + +/// Negative control: a genuine ES module must NOT be mistaken for a wrapped +/// CommonJS one, or #9412's fix would give real ESM entries CommonJS tick +/// ordering — the same bug pointed the other way. +#[test] +fn a_hand_written_es_module_is_not_recognised_as_cjs_wrapped() { + const ESM_FIXTURE: &str = r#"import { createRequire } from 'node:module'; +const require2 = createRequire(import.meta.url); +export const value = 1; +"#; + let ast = perry_parser::parse_typescript(ESM_FIXTURE, "esm.ts").expect("fixture must parse"); + let hir = perry_hir::lower_module(&ast, "esm", "/tmp/perry-canary/esm.ts") + .expect("fixture must lower"); + assert!( + !perry_codegen::module_is_cjs_wrapped(&hir), + "a user's own `import {{ createRequire }} from 'node:module'` was taken for \ + Perry's wrap. The recogniser must key on the aliased local \ + (`{}`), not on the module specifier.", + perry_codegen::cjs_wrap_create_require_local() + ); +} diff --git a/test-files/test_gap_9410_error_subclass_stack.ts b/test-files/test_gap_9410_error_subclass_stack.ts new file mode 100644 index 0000000000..57b8ea9477 --- /dev/null +++ b/test-files/test_gap_9410_error_subclass_stack.ts @@ -0,0 +1,118 @@ +// #9410 — `class X extends Error {}` produced instances with no `.stack` and a +// `[object Object]` tag. Only subclasses were affected; the base `Error` was +// fine, which is why no fixture caught it. The cc bundle has 93 `extends Error` +// classes, so every diagnostic it printed lost its trace. +// +// Stack CONTENTS are host-specific (absolute paths, frame counts), so this +// fixture asserts only what is portable: that `.stack` is a string that starts +// with the constructor name and the message, that `Object.prototype.toString` +// reports `[object Error]`, and that the ordinary Error surface +// (`name`/`message`/`instanceof`) is intact. + +class Plain extends Error {} + +class Named extends Error { + constructor(message: string) { + super(message); + this.name = "Named"; + } +} + +class WithField extends Error { + code = "E_FIELD"; +} + +class Deep extends Named {} + +class Late extends Error { + constructor(message: string) { + super(); + this.message = message; + } +} + +class TypeErrorSub extends TypeError {} +class RangeErrorSub extends RangeError {} + +function describe(label: string, error: any, expectedName: string, expectedMessage: string): void { + const stack = error.stack; + console.log(label + " stack typeof: " + typeof stack); + console.log(label + " stack nonempty: " + (typeof stack === "string" && stack.length > 0)); + console.log( + label + " stack head: " + + (typeof stack === "string" + ? stack.split("\n")[0] + : "") + ); + console.log(label + " tag: " + Object.prototype.toString.call(error)); + console.log(label + " name: " + error.name); + console.log(label + " message: " + error.message); + console.log(label + " instanceof Error: " + (error instanceof Error)); + console.log(label + " toString: " + String(error)); + console.log( + label + " own stack or inherited: " + + (typeof stack === "string" || stack === undefined ? "reported" : "other") + ); + console.log(label + " name matches: " + (error.name === expectedName)); + console.log(label + " message matches: " + (error.message === expectedMessage)); +} + +describe("base", new Error("base-msg"), "Error", "base-msg"); +describe("plain-subclass", new Plain("plain-msg"), "Error", "plain-msg"); +describe("named-subclass", new Named("named-msg"), "Named", "named-msg"); +describe("field-subclass", new WithField("field-msg"), "Error", "field-msg"); +describe("deep-subclass", new Deep("deep-msg"), "Named", "deep-msg"); +describe("late-message", new Late("late-msg"), "Error", "late-msg"); +describe("typeerror-subclass", new TypeErrorSub("te-msg"), "TypeError", "te-msg"); +describe("rangeerror-subclass", new RangeErrorSub("re-msg"), "RangeError", "re-msg"); + +// A subclass instance created through a factory (the shape cc's bundle uses). +function make(message: string): Plain { + return new Plain(message); +} +describe("factory-subclass", make("factory-msg"), "Error", "factory-msg"); + +// The extra field survives, and the subclass's own property is enumerable while +// `stack` is not (node installs `stack` as a non-enumerable own property). +const withField = new WithField("field-msg"); +console.log("field value: " + withField.code); +// The subclass's own field enumerates; `stack` must not. NOT asserted here: +// the full `Object.keys` list, because perry additionally stamps an own +// ENUMERABLE `name` onto an Error-subclass instance where node leaves `name` +// on `Error.prototype` — a separate, pre-existing divergence (perry +// `["code","name"]` vs node `["code"]`) with its own fix, and asserting the +// whole list here would tie this fixture to that one. +console.log("field key enumerates: " + Object.keys(withField).includes("code")); +console.log("stack key enumerates: " + Object.keys(withField).includes("stack")); +console.log( + "stack own: " + Object.prototype.hasOwnProperty.call(withField, "stack") +); +const stackDescriptor = Object.getOwnPropertyDescriptor(withField, "stack"); +console.log( + "stack enumerable: " + + (stackDescriptor === undefined ? "" : String(stackDescriptor.enumerable)) +); + +// A caught subclass error keeps its stack through the throw. +try { + throw new Plain("thrown-msg"); +} catch (error: any) { + console.log("caught stack typeof: " + typeof error.stack); + console.log("caught tag: " + Object.prototype.toString.call(error)); +} + +// `Error.captureStackTrace`, when present, must also work on a subclass. +console.log( + "captureStackTrace present: " + + (typeof (Error as any).captureStackTrace === "function") +); +if (typeof (Error as any).captureStackTrace === "function") { + const target = new Plain("capture-msg"); + (Error as any).captureStackTrace(target); + console.log("captured stack typeof: " + typeof target.stack); +} + +// Non-Error classes must NOT gain the Error tag. +class NotAnError {} +console.log("non-error tag: " + Object.prototype.toString.call(new NotAnError())); +console.log("plain-object tag: " + Object.prototype.toString.call({})); diff --git a/test-files/test_gap_9411_private_brand_in.ts b/test-files/test_gap_9411_private_brand_in.ts new file mode 100644 index 0000000000..e4834374cb --- /dev/null +++ b/test-files/test_gap_9411_private_brand_in.ts @@ -0,0 +1,149 @@ +// #9411 — `#x in o` (ES2022 ergonomic brand check, ECMA-262 §13.10.2) answered +// `false` for objects that really do carry the private field. Downleveled +// TypeScript/Babel output uses exactly this form to ask "is this one of mine?", +// so the wrong answer is a silently-taken wrong branch, not an error. +// +// The pre-existing fixtures only covered the brand check from an *instance* +// method; the reported failure is the check evaluated from a *static* method +// (and from a static block / static field initializer), which is the shape +// `class A { #x = 1; static has(o) { return #x in o } }` — the one downlevelers +// emit for `WeakMap`-free brand checks. + +function log(label: string, value: unknown): void { + console.log(label + ": " + String(value)); +} + +class FieldBrand { + #x = 1; + + static has(o: any): boolean { + return #x in o; + } + + static hasArrow: (o: any) => boolean = (o: any) => #x in o; + + static fromBlock = false; + + static { + FieldBrand.fromBlock = #x in new FieldBrand(); + } + + hasFromInstance(o: any): boolean { + return #x in o; + } + + read(): number { + return this.#x; + } +} + +class MethodBrand { + #m(): string { + return "m"; + } + + static has(o: any): boolean { + return #m in o; + } + + call(): string { + return this.#m(); + } +} + +class AccessorBrand { + get #g(): string { + return "g"; + } + set #s(_value: string) {} + + static hasGetter(o: any): boolean { + return #g in o; + } + + static hasSetter(o: any): boolean { + return #s in o; + } +} + +class StaticFieldBrand { + static #s = 1; + + static has(o: any): boolean { + return #s in o; + } +} + +class FieldSubclass extends FieldBrand {} +class MethodSubclass extends MethodBrand {} + +class Foreign { + #x = 1; +} + +const instance = new FieldBrand(); +const subclassInstance = new FieldSubclass(); + +log("field brand from static method", FieldBrand.has(instance)); +log("field brand from static arrow", FieldBrand.hasArrow(instance)); +log("field brand from static block", FieldBrand.fromBlock); +log("field brand from instance method", instance.hasFromInstance(instance)); +log("field brand on subclass instance (static)", FieldBrand.has(subclassInstance)); +log( + "field brand on subclass instance (instance)", + instance.hasFromInstance(subclassInstance) +); +log("field read still works", instance.read()); + +log("method brand from static method", MethodBrand.has(new MethodBrand())); +log("method brand on subclass instance", MethodBrand.has(new MethodSubclass())); +log("method call still works", new MethodBrand().call()); + +log("getter brand from static method", AccessorBrand.hasGetter(new AccessorBrand())); +log("setter brand from static method", AccessorBrand.hasSetter(new AccessorBrand())); + +log("static field brand on constructor", StaticFieldBrand.has(StaticFieldBrand)); + +// Negatives — every one of these must stay false. +log("field brand on plain object", FieldBrand.has({})); +log("field brand on public hash key", FieldBrand.has({ "#x": 1 })); +log("field brand on foreign class instance", FieldBrand.has(new Foreign())); +log("field brand on constructor itself", FieldBrand.has(FieldBrand)); +log("field brand on prototype", FieldBrand.has(FieldBrand.prototype)); +log("method brand on plain object", MethodBrand.has({})); +log("method brand on foreign instance", MethodBrand.has(new Foreign())); +log("static field brand on instance", StaticFieldBrand.has(new FieldBrand())); +log("field brand on array", FieldBrand.has([])); +log("field brand on function", FieldBrand.has(function () {})); + +// A superclass brand is visible on a subclass instance, but a subclass brand is +// NOT visible on a bare superclass instance. +class Base { + #b = 1; + static hasBase(o: any): boolean { + return #b in o; + } +} +class Derived extends Base { + #d = 2; + static hasDerived(o: any): boolean { + return #d in o; + } +} +log("base brand on derived instance", Base.hasBase(new Derived())); +log("derived brand on derived instance", Derived.hasDerived(new Derived())); +log("derived brand on base instance", Derived.hasDerived(new Base())); + +// Two separate evaluations of the same class body produce distinct brands. +function makeClass(): any { + return class { + #k = 1; + static has(o: any): boolean { + return #k in o; + } + }; +} +const First = makeClass(); +const Second = makeClass(); +log("fresh brand own instance", First.has(new First())); +log("fresh brand cross evaluation", First.has(new Second())); diff --git a/test-files/test_gap_9412_entry_tick_order.ts b/test-files/test_gap_9412_entry_tick_order.ts new file mode 100644 index 0000000000..439fcc522b --- /dev/null +++ b/test-files/test_gap_9412_entry_tick_order.ts @@ -0,0 +1,44 @@ +// #9412 control — the ESM half of the `process.nextTick` ordering contract. +// +// Node defers the first `process.nextTick` drain of an ES-module entry until +// the promise-job queue has drained (module evaluation itself runs inside the +// module job's promise chain), so an ESM entry prints +// ["promise1","await1","tick1"] while a CommonJS entry prints +// ["tick1","promise1","await1"]. +// +// Perry models that with a one-shot "ESM evaluation checkpoint". #9412 was that +// the checkpoint also fired for a CommonJS entry, because the CJS wrapper's +// synthetic `node:module` import made codegen classify the entry as ESM. The +// CommonJS side is covered by +// `test-files/test_gap_9412_require_builtin_tick_order.cts`; this +// fixture pins the ESM side so the fix cannot be "stop deferring, always". +// +// This file has a real top-level `import`, so it is a genuine ES module under +// both Node and perry. +import * as nodePath from "node:path"; + +const order: string[] = []; + +process.nextTick(() => order.push("tick1")); +Promise.resolve().then(() => order.push("promise1")); +(async () => { + await null; + order.push("await1"); +})(); +process.nextTick(() => order.push("tick2")); +queueMicrotask(() => order.push("queueMicrotask1")); + +setTimeout(() => { + console.log("import worked: " + (nodePath.sep === "/" || nodePath.sep === "\\")); + console.log("esm entry order: " + JSON.stringify(order)); + + // After the one-shot evaluation checkpoint is spent, ticks lead again — + // in an ES module exactly as in CommonJS. + const later: string[] = []; + process.nextTick(() => later.push("tick")); + Promise.resolve().then(() => later.push("promise")); + queueMicrotask(() => later.push("queueMicrotask")); + setTimeout(() => { + console.log("esm later: " + JSON.stringify(later)); + }, 10); +}, 20); diff --git a/test-files/test_gap_9412_require_builtin_tick_order.cts b/test-files/test_gap_9412_require_builtin_tick_order.cts new file mode 100644 index 0000000000..1eaa004549 --- /dev/null +++ b/test-files/test_gap_9412_require_builtin_tick_order.cts @@ -0,0 +1,63 @@ +// #9412 — after a `require()` of a builtin, `process.nextTick` callbacks ran +// AFTER promise microtasks instead of before. +// +// The entry file is CommonJS (bare `require`, no top-level `import`/`export`), +// so Node runs it with CommonJS semantics: the nextTick queue is drained before +// the promise-job queue. Perry CJS-wrapped the entry, the wrapper injected a +// synthetic `import { createRequire } from 'node:module'` plus an +// `export default`, and that made codegen mark the entry as an ES module — which +// switches on the (correct, for real ESM) "defer the first tick drain until the +// microtask queue is empty" checkpoint. A CommonJS program then got ESM +// ordering. +// +// `.cts`, deliberately: this repo's package is `"type": "module"`, so Node runs +// a plain `.ts` as an ES module and a bare `require` in one dies with +// `require is not defined` before the ordering can be compared at all. The +// extension names the module goal, so both engines agree on CommonJS. The +// parity runner discovers `.cts` since #9418. Keep this file free of top-level +// `import`/`export`. +// +// (Perry decides the goal from CONTENT, not from the nearest package.json, so +// a plain `.ts` with neither `import`/`export` nor `require` is ticks-first +// for perry and ticks-last for Node under `"type": "module"` — a separate, +// pre-existing module-detection divergence, not this one.) + +const path = require("path"); + +const order: string[] = []; + +process.nextTick(() => order.push("tick1")); +Promise.resolve().then(() => order.push("promise1")); +(async () => { + await null; + order.push("await1"); +})(); +process.nextTick(() => order.push("tick2")); +queueMicrotask(() => order.push("queueMicrotask1")); + +// A tick scheduled from inside a tick joins the same drain, ahead of the +// microtask queue; a tick scheduled from inside a microtask runs after the +// microtask queue drains. +process.nextTick(() => { + order.push("tick3"); + process.nextTick(() => order.push("tick3-nested")); +}); +Promise.resolve().then(() => { + order.push("promise2"); + process.nextTick(() => order.push("tick-from-promise")); +}); + +setTimeout(() => { + console.log("require worked: " + (path.sep === "/" || path.sep === "\\")); + console.log("order: " + JSON.stringify(order)); + + // Second turn: the same invariant must hold outside the first drain, where + // no ESM evaluation checkpoint could ever apply. + const later: string[] = []; + process.nextTick(() => later.push("tick")); + Promise.resolve().then(() => later.push("promise")); + queueMicrotask(() => later.push("queueMicrotask")); + setTimeout(() => { + console.log("later: " + JSON.stringify(later)); + }, 10); +}, 20);