Skip to content

Commit 1502447

Browse files
author
Ralph Küpper
committed
fix(path): #7621 — path.* arms read an SSO string's inline bytes as a header
`path.resolve("/root", computedShortString)` threw ERR_INVALID_ARG_TYPE where node returns the path. Every `path.*` codegen arm unboxed its operand with `unbox_to_i64` (`bitcast double -> i64; and POINTER_MASK`) and handed the low 48 bits to a runtime entry that dereferences them as `*const StringHeader`. That is the header for a HEAP string and the CHARACTERS for a small-string-optimized one (SHORT_STRING_TAG, <= SHORT_STRING_MAX_LEN = 5 inline bytes) — so a literal worked (interned onto the heap) and a computed short string did not. Bisected by length: 5 bytes throws, 6 works. Twelve arms were affected, not the five the issue named: resolve, resolve's fold step, join, win32.join, normalize, extname, dirname, basename, basename(p, ext), isAbsolute, parse, matchesGlob, plus the win32 sub-namespace equivalents. `parse` and `matchesGlob` were SILENTLY wrong rather than throwing. Single-operand arms now call `js_path_arg_header`, which materializes only the SSO case and reproduces the old mask bit for bit for heap strings AND every non-string — so each entry point keeps its own established non-string behaviour (throw, or `unwrap_or_default`). It is deliberately not `js_get_string_pointer_unified`, which coerces numbers to strings and would turn `path.isAbsolute(5)` from a throw into `false`. Two-operand arms take both operands NaN-boxed and unbox inside one runtime call (`js_path_*_value`). Codegen cannot close that window itself: materializing the first operand allocates, and `rooting::with_operands_rooted` hands the lowering registers rather than slots, so the second operand's register is stale with no re-read to reach for. The runtime entry roots the first operand across the second's materialization via `RuntimeHandle::across_const`. Validated locally: the new gap test is byte-identical to node 26.5.1 and exits 0 (it threw at line 2 before the fix); byte-identical again under PERRY_GC_ZEAL=1 + PERRY_GC_PROTECT_FROMSPACE=1 with 402k copying minors observed; test_parity_path, test_gap_node_path, test_gap_6371_path_lexical_semantics, test_gap_node_fs and test_cli_simulation unchanged.
1 parent 418d548 commit 1502447

8 files changed

Lines changed: 497 additions & 39 deletions

File tree

crates/perry-codegen/src/expr/array_methods.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
278278
Expr::StaticPluginResolve(_) => Ok(double_literal(0.0)),
279279

280280
// -------- More cheap stubs --------
281+
// #7621: `js_path_arg_header`, not `unbox_to_i64` — the plain mask hands
282+
// an SSO string's inline CHARACTERS to a `*StringHeader` consumer.
281283
Expr::PathNormalize(p) => {
282284
let p_box = lower_expr(ctx, p)?;
283285
let blk = ctx.block();
284-
let p_handle = unbox_to_i64(blk, &p_box);
286+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
285287
let result = blk.call(I64, "js_path_normalize", &[(I64, &p_handle)]);
286288
Ok(nanbox_string_inline(blk, &result))
287289
}
288290
Expr::PathResolve(p) => {
289291
let p_box = lower_expr(ctx, p)?;
290292
let blk = ctx.block();
291-
let p_handle = unbox_to_i64(blk, &p_box);
293+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
292294
let result = blk.call(I64, "js_path_resolve", &[(I64, &p_handle)]);
293295
Ok(nanbox_string_inline(blk, &result))
294296
}

crates/perry-codegen/src/expr/arrays_finds.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,12 @@ pub(crate) fn lower(
556556
Expr::PathExtname(p) => {
557557
let p_box = lower_expr(ctx, p)?;
558558
let blk = ctx.block();
559-
let p_handle = unbox_to_i64(blk, &p_box);
559+
// #7621: `js_path_arg_header`, not `unbox_to_i64` — the plain mask
560+
// hands an SSO string's inline CHARACTERS to a `*StringHeader`
561+
// consumer. Single-operand arms need no rooting: the materialising
562+
// call is immediately followed by the consumer, with nothing
563+
// between them that can collect.
564+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
560565
let result = blk.call(I64, "js_path_extname", &[(I64, &p_handle)]);
561566
Ok(nanbox_string_inline(blk, &result))
562567
}
@@ -591,26 +596,29 @@ pub(crate) fn lower(
591596
let p_box = vals[0].clone();
592597
let pat_box = vals[1].clone();
593598
let blk = ctx.block();
594-
let p_handle = unbox_to_i64(blk, &p_box);
595-
let pat_handle = unbox_to_i64(blk, &pat_box);
596599
let i32_v = blk.call(
597600
I32,
598-
"js_path_matches_glob",
599-
&[(I64, &p_handle), (I64, &pat_handle)],
601+
"js_path_matches_glob_value",
602+
&[(DOUBLE, &p_box), (DOUBLE, &pat_box)],
600603
);
601604
Ok(i32_bool_to_nanbox(blk, &i32_v))
602605
})
603606
}
607+
// #7621: both operands go to the runtime NaN-BOXED. Unboxing them here
608+
// would mean two `js_path_arg_header` calls, and the first one
609+
// ALLOCATES for an SSO operand — a collection point with the second
610+
// operand still live in a bare register. This API cannot close that
611+
// window (`with_operands_rooted` yields registers, not slots, and
612+
// `RootedSlot` has no `read` by design), so the pair is unboxed inside
613+
// one runtime call that roots across its own materialisation.
604614
Expr::PathResolveJoin(a, b) => rooting::with_operands_rooted(ctx, &[a, b], |ctx, vals| {
605615
let a_box = vals[0].clone();
606616
let b_box = vals[1].clone();
607617
let blk = ctx.block();
608-
let a_handle = unbox_to_i64(blk, &a_box);
609-
let b_handle = unbox_to_i64(blk, &b_box);
610618
let result = blk.call(
611619
I64,
612-
"js_path_resolve_join",
613-
&[(I64, &a_handle), (I64, &b_handle)],
620+
"js_path_resolve_join_value",
621+
&[(DOUBLE, &a_box), (DOUBLE, &b_box)],
614622
);
615623
Ok(nanbox_string_inline(blk, &result))
616624
}),

crates/perry-codegen/src/expr/instance_misc1.rs

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -939,11 +939,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
939939
// -------- path.join(a, b) -> string --------
940940
// The HIR variant is binary; multi-arg path.join lowers to
941941
// chained PathJoin in the HIR.
942+
// #7621: both operands reach the runtime NaN-BOXED, so this arm emits
943+
// NO unbox at all. Unboxing here would need two `js_path_arg_header`
944+
// calls, and the first ALLOCATES for an SSO operand — a collection
945+
// point with the second operand still in a bare register, i.e. exactly
946+
// the window the #7615 migration exists to remove. Handing both
947+
// NaN-boxed doubles to one runtime entry deletes the window from
948+
// codegen rather than protecting it: `js_path_join_value` roots operand
949+
// 1 across its own materialisation with `RuntimeHandle::across_const`.
950+
// See perry-runtime/src/path/value_args.rs.
942951
Expr::PathJoin(a, b) => rooting::with_operands_rooted(ctx, &[a, b], |ctx, vals| {
943952
let blk = ctx.block();
944-
let a_handle = unbox_to_i64(blk, &vals[0]);
945-
let b_handle = unbox_to_i64(blk, &vals[1]);
946-
let result = blk.call(I64, "js_path_join", &[(I64, &a_handle), (I64, &b_handle)]);
953+
let result = blk.call(
954+
I64,
955+
"js_path_join_value",
956+
&[(DOUBLE, &vals[0]), (DOUBLE, &vals[1])],
957+
);
947958
Ok(nanbox_string_inline(blk, &result))
948959
}),
949960

@@ -953,12 +964,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
953964
// PathWin32Join in the HIR.
954965
Expr::PathWin32Join(a, b) => rooting::with_operands_rooted(ctx, &[a, b], |ctx, vals| {
955966
let blk = ctx.block();
956-
let a_handle = unbox_to_i64(blk, &vals[0]);
957-
let b_handle = unbox_to_i64(blk, &vals[1]);
967+
// #7621: NaN-boxed pair, no unbox here — see `Expr::PathJoin`.
958968
let result = blk.call(
959969
I64,
960-
"js_path_win32_join",
961-
&[(I64, &a_handle), (I64, &b_handle)],
970+
"js_path_win32_join_value",
971+
&[(DOUBLE, &vals[0]), (DOUBLE, &vals[1])],
962972
);
963973
Ok(nanbox_string_inline(blk, &result))
964974
}),
@@ -998,7 +1008,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
9981008
_ => unreachable!(),
9991009
};
10001010
let blk = ctx.block();
1001-
let h = unbox_to_i64(blk, &lowered[0]);
1011+
// #7621: `js_path_arg_header`, not `unbox_to_i64` — the
1012+
// plain mask hands an SSO string's inline CHARACTERS to
1013+
// a `*StringHeader` consumer. It is emitted INSIDE the
1014+
// rooted region, and needs no window of its own: the
1015+
// materialising call is immediately followed by its
1016+
// consumer with nothing between them, and the helper
1017+
// only allocates — it cannot re-enter user code or
1018+
// enumerate an object, so per #7198 it cannot initiate a
1019+
// moving collection and `with_operands_rooted_across_call`
1020+
// would be pure cost here.
1021+
let h = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &lowered[0])]);
10021022
let result = blk.call(I64, fn_name, &[(I64, &h)]);
10031023
Ok(nanbox_string_inline(blk, &result))
10041024
}
@@ -1016,34 +1036,46 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
10161036
Ok(nanbox_string_inline(blk, &result))
10171037
}
10181038
PathWin32Method::BasenameExt | PathWin32Method::ResolveJoin => {
1039+
// #7621: two-operand methods hand BOTH operands to the
1040+
// runtime NaN-boxed, so this arm emits no unbox at all
1041+
// — see the `Expr::PathJoin` comment above for why that
1042+
// is stronger than protecting the window here.
10191043
let fn_name = match method {
1020-
PathWin32Method::BasenameExt => "js_path_win32_basename_ext",
1021-
PathWin32Method::ResolveJoin => "js_path_win32_resolve_join",
1044+
PathWin32Method::BasenameExt => "js_path_win32_basename_ext_value",
1045+
PathWin32Method::ResolveJoin => "js_path_win32_resolve_join_value",
10221046
_ => unreachable!(),
10231047
};
10241048
let blk = ctx.block();
1025-
let a = unbox_to_i64(blk, &lowered[0]);
1026-
let b = unbox_to_i64(blk, &lowered[1]);
1027-
let result = blk.call(I64, fn_name, &[(I64, &a), (I64, &b)]);
1049+
let result = blk.call(
1050+
I64,
1051+
fn_name,
1052+
&[(DOUBLE, &lowered[0]), (DOUBLE, &lowered[1])],
1053+
);
10281054
Ok(nanbox_string_inline(blk, &result))
10291055
}
10301056
PathWin32Method::IsAbsolute => {
10311057
let blk = ctx.block();
1032-
let h = unbox_to_i64(blk, &lowered[0]);
1058+
// #7621: SSO-safe unbox — see the Dirname arm above.
1059+
let h = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &lowered[0])]);
10331060
let i32_v = blk.call(I32, "js_path_win32_is_absolute", &[(I64, &h)]);
10341061
Ok(i32_bool_to_nanbox(blk, &i32_v))
10351062
}
10361063
PathWin32Method::MatchesGlob => {
10371064
let blk = ctx.block();
1038-
let p = unbox_to_i64(blk, &lowered[0]);
1039-
let pat = unbox_to_i64(blk, &lowered[1]);
1040-
let i32_v =
1041-
blk.call(I32, "js_path_win32_matches_glob", &[(I64, &p), (I64, &pat)]);
1065+
// #7621: NaN-boxed pair, no unbox here.
1066+
let i32_v = blk.call(
1067+
I32,
1068+
"js_path_win32_matches_glob_value",
1069+
&[(DOUBLE, &lowered[0]), (DOUBLE, &lowered[1])],
1070+
);
10421071
Ok(i32_bool_to_nanbox(blk, &i32_v))
10431072
}
10441073
PathWin32Method::Parse => {
10451074
let blk = ctx.block();
1046-
let h = unbox_to_i64(blk, &lowered[0]);
1075+
// #7621: SSO-safe unbox. Before this,
1076+
// `path.win32.parse(shortComputed).base` was silently ""
1077+
// rather than a throw.
1078+
let h = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &lowered[0])]);
10471079
let result = blk.call(I64, "js_path_win32_parse", &[(I64, &h)]);
10481080
Ok(nanbox_pointer_inline(blk, &result))
10491081
}
@@ -1237,7 +1269,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
12371269
Expr::PathDirname(p) => {
12381270
let p_box = lower_expr(ctx, p)?;
12391271
let blk = ctx.block();
1240-
let p_handle = unbox_to_i64(blk, &p_box);
1272+
// #7621: SSO-safe unbox (see path/value_args.rs).
1273+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
12411274
let result = blk.call(I64, "js_path_dirname", &[(I64, &p_handle)]);
12421275
Ok(nanbox_string_inline(blk, &result))
12431276
}
@@ -1714,21 +1747,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
17141747
Expr::PathBasename(p) => {
17151748
let p_box = lower_expr(ctx, p)?;
17161749
let blk = ctx.block();
1717-
let p_handle = unbox_to_i64(blk, &p_box);
1750+
// #7621: SSO-safe unbox (see path/value_args.rs).
1751+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
17181752
let result = blk.call(I64, "js_path_basename", &[(I64, &p_handle)]);
17191753
Ok(nanbox_string_inline(blk, &result))
17201754
}
17211755
Expr::PathBasenameExt(p, ext) => {
17221756
// path.basename(path, ext) — strips trailing `ext` suffix.
1723-
// Runtime: js_path_basename_ext(path_ptr, ext_ptr) -> *StringHeader.
1757+
// #7621: the runtime entry takes both operands NaN-boxed and
1758+
// unboxes them under a root, so this arm emits no unbox — see
1759+
// `Expr::PathJoin`.
17241760
rooting::with_operands_rooted(ctx, &[p, ext], |ctx, vals| {
17251761
let blk = ctx.block();
1726-
let p_handle = unbox_to_i64(blk, &vals[0]);
1727-
let e_handle = unbox_to_i64(blk, &vals[1]);
17281762
let result = blk.call(
17291763
I64,
1730-
"js_path_basename_ext",
1731-
&[(I64, &p_handle), (I64, &e_handle)],
1764+
"js_path_basename_ext_value",
1765+
&[(DOUBLE, &vals[0]), (DOUBLE, &vals[1])],
17321766
);
17331767
Ok(nanbox_string_inline(blk, &result))
17341768
})
@@ -1737,7 +1771,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
17371771
// path.parse(p) -> object with { dir, base, ext, name, root }
17381772
let p_box = lower_expr(ctx, p)?;
17391773
let blk = ctx.block();
1740-
let p_handle = unbox_to_i64(blk, &p_box);
1774+
// #7621: SSO-safe unbox (see path/value_args.rs). Before this,
1775+
// `path.parse(shortComputed).base` was silently "" rather than a
1776+
// throw, because js_path_parse defaults an unreadable header.
1777+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
17411778
let result = blk.call(I64, "js_path_parse", &[(I64, &p_handle)]);
17421779
Ok(nanbox_pointer_inline(blk, &result))
17431780
}

crates/perry-codegen/src/expr/misc_methods.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
207207
Expr::PathIsAbsolute(p) => {
208208
let p_box = lower_expr(ctx, p)?;
209209
let blk = ctx.block();
210-
let p_handle = unbox_to_i64(blk, &p_box);
210+
// #7621: SSO-safe unbox — see crates/perry-runtime/src/path/value_args.rs.
211+
let p_handle = blk.call(I64, "js_path_arg_header", &[(DOUBLE, &p_box)]);
211212
let i32_res = blk.call(I32, "js_path_is_absolute", &[(I64, &p_handle)]);
212213
Ok(i32_bool_to_nanbox(blk, &i32_res))
213214
}

crates/perry-codegen/src/runtime_decls/strings.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,6 +1528,20 @@ pub fn declare_phase_b_strings(module: &mut LlModule) {
15281528
module.declare_function("js_path_to_namespaced_path_value", DOUBLE, &[DOUBLE]);
15291529
module.declare_function("js_path_matches_glob", I32, &[I64, I64]);
15301530
module.declare_function("js_path_resolve_join", I64, &[I64, I64]);
1531+
// #7621 — SSO-safe operand entries. `js_path_arg_header` replaces the
1532+
// `unbox_to_i64` mask on every single-operand arm; the `_value` pair
1533+
// entries unbox BOTH operands inside the runtime so the first one can be
1534+
// rooted across the second's materialisation. See
1535+
// perry-runtime/src/path/value_args.rs.
1536+
module.declare_function("js_path_arg_header", I64, &[DOUBLE]);
1537+
module.declare_function("js_path_join_value", I64, &[DOUBLE, DOUBLE]);
1538+
module.declare_function("js_path_win32_join_value", I64, &[DOUBLE, DOUBLE]);
1539+
module.declare_function("js_path_resolve_join_value", I64, &[DOUBLE, DOUBLE]);
1540+
module.declare_function("js_path_win32_resolve_join_value", I64, &[DOUBLE, DOUBLE]);
1541+
module.declare_function("js_path_basename_ext_value", I64, &[DOUBLE, DOUBLE]);
1542+
module.declare_function("js_path_win32_basename_ext_value", I64, &[DOUBLE, DOUBLE]);
1543+
module.declare_function("js_path_matches_glob_value", I32, &[DOUBLE, DOUBLE]);
1544+
module.declare_function("js_path_win32_matches_glob_value", I32, &[DOUBLE, DOUBLE]);
15311545
module.declare_function("js_object_from_entries", DOUBLE, &[DOUBLE]);
15321546
module.declare_function("js_string_match", I64, &[I64, I64]);
15331547
module.declare_function("js_string_match_all", I64, &[I64, I64]);

crates/perry-runtime/src/path.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,5 +1862,8 @@ pub extern "C" fn js_path_win32_relative(
18621862
string_to_js(&parts.join("\\"))
18631863
}
18641864

1865+
/// NaN-boxed operand entry points (#7621) — see the child module's docs.
1866+
pub mod value_args;
1867+
18651868
#[cfg(test)]
18661869
mod tests;

0 commit comments

Comments
 (0)