diff --git a/changelog.d/8011-root-comparison-operands.md b/changelog.d/8011-root-comparison-operands.md new file mode 100644 index 0000000000..0a3a3b5257 --- /dev/null +++ b/changelog.d/8011-root-comparison-operands.md @@ -0,0 +1,22 @@ +**Fixed: comparison call results going stale while the other operand collected (#7979).** + +Comparison lowering evaluated the left operand, evaluated the right operand, +and then consumed the original left SSA register. For an inline verdict such +as `observed() === expected()`, an allocating right-hand call could run a +copying minor after the left call returned a heap string. The collector rewrote +roots but could not rewrite that bare register, so `js_eq` later dereferenced +retired from-space in `js_jsvalue_equals`. + +All comparison paths now use the shared selective operand-rooting scope. Each +operand that can hold a GC pointer is protected before later operands run, +re-read after evaluation, and kept rooted through the comparison dispatch; +proven non-pointer operands remain in their original registers and emit no +root traffic. + +Codegen tests trace `js_eq`'s arguments back through pure LLVM operations and +assert that an object-valued left operand is re-read below an allocating right +operand, while a proven-number left operand is not. The original +`test_gap_gc_define_property_descriptor_rooting.ts` witness is restored to +inline comparisons and registered in the moving-GC corpus; before the fix its +string operand faults under retired-from-space protection, while the rooted +form remains byte-exact with Node under scheduled moving collections. diff --git a/crates/perry-codegen/src/expr/compare.rs b/crates/perry-codegen/src/expr/compare.rs index f40487c35f..30edb49c9b 100644 --- a/crates/perry-codegen/src/expr/compare.rs +++ b/crates/perry-codegen/src/expr/compare.rs @@ -9,13 +9,14 @@ use perry_hir::types::Type as HirType; use perry_hir::{CompareOp, Expr}; use crate::nanbox::POINTER_MASK_I64; +use crate::rooting::with_operands_rooted; use crate::type_analysis::{ expr_may_return_boxed_value_from_raw_f64_fallback, is_bigint_expr, is_bool_expr, is_numeric_expr, is_string_expr, }; use crate::types::{DOUBLE, I1, I32, I64, I8}; -use super::{lower_expr, unbox_str_handle, unbox_to_i64, FnCtx}; +use super::{unbox_str_handle, unbox_to_i64, FnCtx}; /// Repsel Phase 3a shared dispatch for the canonical-Str compare arms: /// lower both operands' bits, branch on "both heap `STRING_TAG`", call @@ -549,586 +550,571 @@ fn lower_string_strict_eq_inline( pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { match expr { Expr::Compare { op, left, right } => { - // BigInt comparison fast path: NaN-tagged BIGINT_TAG values - // are unordered under fcmp (NaN), so `a > b` on two bigints - // always returns false. Route through js_bigint_cmp which - // returns -1/0/1 for the three bigint ordering outcomes. - // - // For RELATIONAL ops (`<`, `<=`, `>`, `>=`) this direct cmp is only - // valid when BOTH operands are statically BigInt — `js_bigint_cmp` - // dereferences both as BigInt pointers. A *mixed* relational like - // `1n < Infinity` or `0n < "1"` needs the full abstract relational - // comparison (BigInt-vs-Number / BigInt-vs-String coercion), so it - // falls through to `js_rel_*` below. Equality (`===`/`==`) keeps the - // either-side gate (its own cross-type handling is unchanged). - let is_relational_op = matches!( - op, - CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge - ); - // The `js_bigint_cmp` fast path is valid ONLY when BOTH operands are - // statically BigInt. The previous equality variant fired when *either* - // side was BigInt and fed `js_bigint_cmp` a non-BigInt operand - // (`0n != undefined`, `0n == ""`), dereferencing an undefined/string - // NaN-box as a BigIntHeader → garbage. Mixed-type BigInt equality now - // falls through to `js_loose_eq` (loose, with full BigInt coercion) / - // `fcmp` (strict, where a type mismatch is correctly never-equal). - // Relational mixed-type already fell through to `js_rel_*`. - let bigint_fast_path = is_bigint_expr(ctx, left) && is_bigint_expr(ctx, right); - if bigint_fast_path { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_handle = unbox_to_i64(blk, &l); - let r_handle = unbox_to_i64(blk, &r); - let cmp = blk.call(I32, "js_bigint_cmp", &[(I64, &l_handle), (I64, &r_handle)]); - let bit = match op { - CompareOp::Lt => blk.icmp_slt(I32, &cmp, "0"), - CompareOp::Le => blk.icmp_sle(I32, &cmp, "0"), - CompareOp::Gt => blk.icmp_sgt(I32, &cmp, "0"), - CompareOp::Ge => blk.icmp_sge(I32, &cmp, "0"), - CompareOp::Eq | CompareOp::LooseEq => blk.icmp_eq(I32, &cmp, "0"), - CompareOp::Ne | CompareOp::LooseNe => blk.icmp_ne(I32, &cmp, "0"), - }; - let tagged = blk.select( - crate::types::I1, - &bit, - I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged)); - } - // Boolean equality fast path: NaN-tagged TAG_TRUE/FALSE - // bits don't compare correctly with fcmp. For - // ===/!== where EITHER side is statically boolean, compare - // the raw i64 bits via icmp. icmp on bits also works for - // any other NaN-tagged value (string ptr, object ptr) when - // the bool literal is on one side — TAG_TRUE bits never - // match a string/pointer, so the result is correctly false. - // STRICT only: for LooseEq/LooseNe, booleans need coercion - // (false == "" → true) which the later js_loose_eq handles. - let either_bool = is_bool_expr(ctx, left) || is_bool_expr(ctx, right); - if either_bool && matches!(op, CompareOp::Eq | CompareOp::Ne) { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_bits = blk.bitcast_double_to_i64(&l); - let r_bits = blk.bitcast_double_to_i64(&r); - let bit = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - blk.icmp_ne(I64, &l_bits, &r_bits) - } else { - blk.icmp_eq(I64, &l_bits, &r_bits) - }; - let tagged = blk.select( - crate::types::I1, - &bit, - I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged)); - } - // Null/Undefined literal fast path: `x === null` / `x === undefined` / - // `x !== null` etc. Both TAG_NULL and TAG_UNDEFINED are NaN-tagged - // doubles, so fcmp is unordered (always false) and the string/js_eq - // fallbacks misclassify these tags as "invalid string → both equal". - // Compare raw i64 bits directly. - // - // For LooseEq/LooseNe (== / !=), null and undefined are loosely - // equal to each other but not to anything else. Handle that by - // routing `x == null` to `(bits == TAG_NULL) | (bits == TAG_UNDEF)`. - let left_is_null = matches!(left.as_ref(), Expr::Null); - let left_is_undef = matches!(left.as_ref(), Expr::Undefined); - let right_is_null = matches!(right.as_ref(), Expr::Null); - let right_is_undef = matches!(right.as_ref(), Expr::Undefined); - let either_nullish_lit = - left_is_null || left_is_undef || right_is_null || right_is_undef; - if either_nullish_lit - && matches!( + // #7979: every arm below used to lower `left`, then lower `right`, + // then consume the original left SSA value. A call-result string + // therefore named retired from-space whenever the right call + // collected. Keep the root around the consuming dispatch too: + // several arms unbox/dereference the values before returning. + with_operands_rooted(ctx, &[left, right], |ctx, operands| { + let l = operands[0].clone(); + let r = operands[1].clone(); + // BigInt comparison fast path: NaN-tagged BIGINT_TAG values + // are unordered under fcmp (NaN), so `a > b` on two bigints + // always returns false. Route through js_bigint_cmp which + // returns -1/0/1 for the three bigint ordering outcomes. + // + // For RELATIONAL ops (`<`, `<=`, `>`, `>=`) this direct cmp is only + // valid when BOTH operands are statically BigInt — `js_bigint_cmp` + // dereferences both as BigInt pointers. A *mixed* relational like + // `1n < Infinity` or `0n < "1"` needs the full abstract relational + // comparison (BigInt-vs-Number / BigInt-vs-String coercion), so it + // falls through to `js_rel_*` below. Equality (`===`/`==`) keeps the + // either-side gate (its own cross-type handling is unchanged). + let is_relational_op = matches!( op, - CompareOp::Eq | CompareOp::Ne | CompareOp::LooseEq | CompareOp::LooseNe - ) - { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_bits = blk.bitcast_double_to_i64(&l); - let r_bits = blk.bitcast_double_to_i64(&r); - let is_loose = matches!(op, CompareOp::LooseEq | CompareOp::LooseNe); - let bit = if is_loose { - // Loose equality: x == null → (x === null) || (x === undefined) - let eq_l_r = blk.icmp_eq(I64, &l_bits, &r_bits); - let cmp_l_null = blk.icmp_eq(I64, &l_bits, crate::nanbox::TAG_NULL_I64); - let cmp_l_undef = blk.icmp_eq(I64, &l_bits, crate::nanbox::TAG_UNDEFINED_I64); - let cmp_r_null = blk.icmp_eq(I64, &r_bits, crate::nanbox::TAG_NULL_I64); - let cmp_r_undef = blk.icmp_eq(I64, &r_bits, crate::nanbox::TAG_UNDEFINED_I64); - let l_nullish = blk.or(crate::types::I1, &cmp_l_null, &cmp_l_undef); - let r_nullish = blk.or(crate::types::I1, &cmp_r_null, &cmp_r_undef); - let both_nullish = blk.and(crate::types::I1, &l_nullish, &r_nullish); - blk.or(crate::types::I1, &eq_l_r, &both_nullish) - } else { - // Strict equality: bit-exact compare - blk.icmp_eq(I64, &l_bits, &r_bits) - }; - let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - blk.xor(crate::types::I1, &bit, "true") - } else { - bit - }; - let tagged = blk.select( - crate::types::I1, - &bit_final, - I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, + CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge ); - return Ok(blk.bitcast_i64_to_double(&tagged)); - } - // Strict equality against a string LITERAL. Decidable inline for - // every runtime shape (see `lower_string_literal_strict_eq`), so it - // pre-empts all the arms below — including the `js_eq` tail that an - // `any`-typed operand like `n.kind` would otherwise take, one call - // pair per comparison. Strict only: loose `==` coerces (`"5" == 5`) - // and stays on `js_loose_eq`. `Expr::WtfString` is excluded — its - // pool bytes are the WTF-8 encoding, not `str::as_bytes`. - let lit_on_right = matches!(right.as_ref(), Expr::String(_)); - let lit_on_left = !lit_on_right && matches!(left.as_ref(), Expr::String(_)); - if (lit_on_right || lit_on_left) && matches!(op, CompareOp::Eq | CompareOp::Ne) { - // Source order: the non-literal operand may have side effects. - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let (val, lit_box, lit) = if lit_on_right { - let Expr::String(s) = right.as_ref() else { - unreachable!("lit_on_right implies Expr::String") - }; - (l, r, s.clone()) - } else { - let Expr::String(s) = left.as_ref() else { - unreachable!("lit_on_left implies Expr::String") + // The `js_bigint_cmp` fast path is valid ONLY when BOTH operands are + // statically BigInt. The previous equality variant fired when *either* + // side was BigInt and fed `js_bigint_cmp` a non-BigInt operand + // (`0n != undefined`, `0n == ""`), dereferencing an undefined/string + // NaN-box as a BigIntHeader → garbage. Mixed-type BigInt equality now + // falls through to `js_loose_eq` (loose, with full BigInt coercion) / + // `fcmp` (strict, where a type mismatch is correctly never-equal). + // Relational mixed-type already fell through to `js_rel_*`. + let bigint_fast_path = is_bigint_expr(ctx, left) && is_bigint_expr(ctx, right); + if bigint_fast_path { + let blk = ctx.block(); + let l_handle = unbox_to_i64(blk, &l); + let r_handle = unbox_to_i64(blk, &r); + let cmp = blk.call(I32, "js_bigint_cmp", &[(I64, &l_handle), (I64, &r_handle)]); + let bit = match op { + CompareOp::Lt => blk.icmp_slt(I32, &cmp, "0"), + CompareOp::Le => blk.icmp_sle(I32, &cmp, "0"), + CompareOp::Gt => blk.icmp_sgt(I32, &cmp, "0"), + CompareOp::Ge => blk.icmp_sge(I32, &cmp, "0"), + CompareOp::Eq | CompareOp::LooseEq => blk.icmp_eq(I32, &cmp, "0"), + CompareOp::Ne | CompareOp::LooseNe => blk.icmp_ne(I32, &cmp, "0"), }; - (r, l, s.clone()) - }; - let bit = lower_string_literal_strict_eq(ctx, &val, &lit_box, &lit); - let blk = ctx.block(); - let bit_final = if matches!(op, CompareOp::Ne) { - blk.xor(I1, &bit, "true") - } else { - bit - }; - let tagged = blk.select( - I1, - &bit_final, - I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged)); - } - // "One side is statically string, other is unknown" - // fallback: `c === Color.Red` where Color is a const - // object. Neither js_eq (bit-compare, wrong for string - // content) nor fcmp (NaN-tagged, always false) works. - // - // Dispatch through js_string_equals after extracting - // both string pointers via js_get_string_pointer_unified. - // That helper returns null for non-string NaN-tagged - // values, which js_string_equals treats as "not equal" - // — the correct answer when the unknown side isn't a - // string at runtime. - let both_strings_check = is_string_expr(ctx, left) && is_string_expr(ctx, right); - // The non-statically-string operand collides through this - // fast path when, at runtime, it is ALSO a non-string. Both - // operands then funnel through `js_get_string_pointer_unified`, - // which returns 0 for any non-string NaN-boxed value (numbers, - // class refs / InjectionTokens, plain objects, …). The - // subsequent `js_string_equals(0, 0)` returns 1 (its - // pointer-identity / both-null branches both report "equal"), - // so two *distinct* non-string values wrongly compare `===`. - // - // This is exactly the NestJS DI `token === name` bug: - // `name` is statically `string` (the destructured - // `dependencyContext.name`) but at runtime holds a class ref - // (e.g. `AppService`), and `token` is `any` holding a - // *different* class ref (`AppController`) — both coerce to 0 - // and the inline `===` reports `true`, throwing - // `UnknownDependencies` and aborting the app. - // - // The static `string` type is therefore a lie here (like the - // #3576 number-vs-object case). When the OTHER operand is - // statically `Any` (its runtime value is unconstrained and may - // be a non-string), this fast path is unsound: route through - // `js_eq`, which content-compares real strings (SSO + heap) AND - // correctly distinguishes class refs / objects by identity. - let other_side_is_any = |other: &Expr| -> bool { - matches!( - crate::type_analysis::static_type_of(ctx, other), - Some(HirType::Any) | None - ) - }; - let one_side_string = !both_strings_check - && ((is_string_expr(ctx, left) - && !is_numeric_expr(ctx, right) - && !is_bool_expr(ctx, right) - && !other_side_is_any(right)) - || (is_string_expr(ctx, right) - && !is_numeric_expr(ctx, left) - && !is_bool_expr(ctx, left) - && !other_side_is_any(left))); - // Only STRICT eq/ne use this string-pointer fast path. Loose `==`/`!=` - // must fall through to `js_loose_eq` below: when one side is a boxed - // String/primitive *wrapper* (a POINTER_TAG object, not a STRING_TAG - // value), `js_get_string_pointer_unified` returns the raw ObjectHeader - // pointer and `js_string_equals` reads it as a bogus string → wrong - // result (`new String("x") == "x"` was `false`). `js_loose_eq` unboxes - // the wrapper first. Strict `=== "lit"` is unaffected (both sides are - // real strings at runtime). #boxed-loose-eq. - if one_side_string && matches!(op, CompareOp::Eq | CompareOp::Ne) { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_handle = blk.call(I64, "js_get_string_pointer_unified", &[(DOUBLE, &l)]); - let r_handle = blk.call(I64, "js_get_string_pointer_unified", &[(DOUBLE, &r)]); - let i32_eq = blk.call( - I32, - "js_string_equals", - &[(I64, &l_handle), (I64, &r_handle)], - ); - let bit = blk.icmp_ne(I32, &i32_eq, "0"); - let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - blk.xor(crate::types::I1, &bit, "true") - } else { - bit - }; - let tagged = blk.select( - crate::types::I1, - &bit_final, - I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged)); - } - // Generic equality fallback: when neither operand is - // statically numeric, dispatch through js_eq which - // handles strings, booleans, objects, null, undefined - // via NaN-tag inspection. Used by `eq` helpers in tests - // that take `any` and pass NaN-tagged values. - let either_non_numeric = !is_numeric_expr(ctx, left) && !is_numeric_expr(ctx, right); - let only_eq = matches!( - op, - CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe - ); - // We still let the more specific paths below win for - // statically-typed string/bool operands; this fallback - // only handles the truly-Any case. - let unknown_l = !is_numeric_expr(ctx, left) - && !is_string_expr(ctx, left) - && !is_bool_expr(ctx, left); - let unknown_r = !is_numeric_expr(ctx, right) - && !is_string_expr(ctx, right) - && !is_bool_expr(ctx, right); - if either_non_numeric && only_eq && unknown_l && unknown_r { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - // Use js_loose_eq for == / != (handles null==undefined, - // cross-type coercion). STRICT `===`/`!==` gets the inline - // prefix instead: the operands that reach here are - // unconstrained, and a generic-container key scan - // (`this.keys[i] === k`) spends its whole cost on this one - // call. Loose `==`'s cross-type coercions are not - // bit-decidable, so it keeps the bare call. - let result_bits = if matches!(op, CompareOp::LooseEq | CompareOp::LooseNe) { + let tagged = blk.select( + crate::types::I1, + &bit, + I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged)); + } + // Boolean equality fast path: NaN-tagged TAG_TRUE/FALSE + // bits don't compare correctly with fcmp. For + // ===/!== where EITHER side is statically boolean, compare + // the raw i64 bits via icmp. icmp on bits also works for + // any other NaN-tagged value (string ptr, object ptr) when + // the bool literal is on one side — TAG_TRUE bits never + // match a string/pointer, so the result is correctly false. + // STRICT only: for LooseEq/LooseNe, booleans need coercion + // (false == "" → true) which the later js_loose_eq handles. + let either_bool = is_bool_expr(ctx, left) || is_bool_expr(ctx, right); + if either_bool && matches!(op, CompareOp::Eq | CompareOp::Ne) { let blk = ctx.block(); let l_bits = blk.bitcast_double_to_i64(&l); let r_bits = blk.bitcast_double_to_i64(&r); - blk.call(I64, "js_loose_eq", &[(I64, &l_bits), (I64, &r_bits)]) - } else { - lower_strict_eq_inline_any(ctx, &l, &r) - }; - let blk = ctx.block(); - let result = blk.bitcast_i64_to_double(&result_bits); - if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); - let inv = blk.xor(crate::types::I1, &cmp, "true"); + let bit = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + blk.icmp_ne(I64, &l_bits, &r_bits) + } else { + blk.icmp_eq(I64, &l_bits, &r_bits) + }; let tagged = blk.select( crate::types::I1, - &inv, + &bit, I64, crate::nanbox::TAG_TRUE_I64, crate::nanbox::TAG_FALSE_I64, ); return Ok(blk.bitcast_i64_to_double(&tagged)); } - return Ok(result); - } - - // String equality fast path: fcmp doesn't work on - // NaN-tagged string pointers (NaN comparisons are - // unordered → always false). When both operands are - // statically strings, dispatch through js_string_equals. - let both_strings = is_string_expr(ctx, left) && is_string_expr(ctx, right); - // Representation-selection Phase 3a: when a canonical-Str local - // is an operand, tag-dispatch inline instead of paying the two - // opaque (SSO-heap-materializing) unified unbox calls: both - // proven heap → direct `js_string_equals(h, h)` on the raw - // handles; any other mix → one `js_jsvalue_equals` call, which - // content-compares heap × SSO without materializing and never - // number-coerces (a lying annotation gets exact `===` - // semantics, strictly closer to spec than the legacy path). - let canonical_str_involved = matches!( - left.as_ref(), Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) - ) || matches!( - right.as_ref(), Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) - ); - if both_strings - && canonical_str_involved - && matches!( - op, - CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe - ) - { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let i32_eq = lower_string_strict_eq_inline(ctx, &l, &r, false); - let blk = ctx.block(); - let bit = blk.icmp_ne(I32, &i32_eq, "0"); - let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - blk.xor(crate::types::I1, &bit, "true") - } else { - bit - }; - let tagged_i64 = blk.select( - crate::types::I1, - &bit_final, - crate::types::I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged_i64)); - } - if both_strings - && matches!( - op, - CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe - ) - { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - // Issue #214: SSO-safe unbox — the inline mask returns - // garbage for SHORT_STRING_TAG values (e.g. SSO results - // from `JSON.parse('["hello"]')[0]`), causing - // `js_string_equals` to deref the inline payload bytes. - // That unbox is now the *fallback* arm: identical bits and - // SSO x SSO are answered inline, which is what keeps a pair of - // short runtime strings (`charAt`, `substring`) from - // materializing two throwaway heap copies per comparison. - let i32_eq = lower_string_strict_eq_inline(ctx, &l, &r, true); - let blk = ctx.block(); - let bit = blk.icmp_ne(I32, &i32_eq, "0"); - let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { - blk.xor(crate::types::I1, &bit, "true") - } else { - bit - }; - let tagged_i64 = blk.select( - crate::types::I1, - &bit_final, - crate::types::I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged_i64)); - } - // String relational fast path: `s1 < s2`, `s1 > s2`, etc. - // fcmp on NaN-tagged pointers is unordered (always false), - // so dispatch through js_string_compare which returns - // -1/0/1 like memcmp. Then test the result against 0 with - // the right icmp predicate. - // Representation-selection Phase 3a: relational counterpart of - // the canonical-Str equality arm above — both proven heap → - // direct `js_string_compare(h, h)`; any other mix → one - // `js_string_compare_value` call (SSO-aware, no heap - // materialization, numbers coerced via their decimal string - // form exactly like the legacy unified path). - if both_strings - && canonical_str_involved - && matches!( - op, - CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge - ) - { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let cmp_i32 = canonical_str_cmp_dispatch( - ctx, - &l, - &r, - "js_string_compare", - "js_string_compare_value", - "strcmp", - ); - let blk = ctx.block(); - let bit = match op { - CompareOp::Lt => blk.icmp_slt(I32, &cmp_i32, "0"), - CompareOp::Le => blk.icmp_sle(I32, &cmp_i32, "0"), - CompareOp::Gt => blk.icmp_sgt(I32, &cmp_i32, "0"), - CompareOp::Ge => blk.icmp_sge(I32, &cmp_i32, "0"), - _ => unreachable!(), - }; - let tagged_i64 = blk.select( - crate::types::I1, - &bit, - crate::types::I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged_i64)); - } - if both_strings - && matches!( - op, - CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge - ) - { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - // Issue #214: SSO-safe unbox. - let l_handle = unbox_str_handle(blk, &l); - let r_handle = unbox_str_handle(blk, &r); - let cmp_i32 = blk.call( - I32, - "js_string_compare", - &[(I64, &l_handle), (I64, &r_handle)], - ); - let bit = match op { - CompareOp::Lt => blk.icmp_slt(I32, &cmp_i32, "0"), - CompareOp::Le => blk.icmp_sle(I32, &cmp_i32, "0"), - CompareOp::Gt => blk.icmp_sgt(I32, &cmp_i32, "0"), - CompareOp::Ge => blk.icmp_sge(I32, &cmp_i32, "0"), - _ => unreachable!(), - }; - let tagged_i64 = blk.select( - crate::types::I1, - &bit, - crate::types::I64, - crate::nanbox::TAG_TRUE_I64, - crate::nanbox::TAG_FALSE_I64, - ); - return Ok(blk.bitcast_i64_to_double(&tagged_i64)); - } - - // Loose equality (==, !=): dispatch through js_loose_eq - // which handles cross-type coercion (null==undefined, - // "1"==1, false==0, etc.). Strict === already handled - // above by the typed fast paths. - if matches!(op, CompareOp::LooseEq | CompareOp::LooseNe) { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_bits = blk.bitcast_double_to_i64(&l); - let r_bits = blk.bitcast_double_to_i64(&r); - let result_bits = blk.call(I64, "js_loose_eq", &[(I64, &l_bits), (I64, &r_bits)]); - if matches!(op, CompareOp::LooseNe) { - let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); - let inv = blk.xor(crate::types::I1, &cmp, "true"); + // Null/Undefined literal fast path: `x === null` / `x === undefined` / + // `x !== null` etc. Both TAG_NULL and TAG_UNDEFINED are NaN-tagged + // doubles, so fcmp is unordered (always false) and the string/js_eq + // fallbacks misclassify these tags as "invalid string → both equal". + // Compare raw i64 bits directly. + // + // For LooseEq/LooseNe (== / !=), null and undefined are loosely + // equal to each other but not to anything else. Handle that by + // routing `x == null` to `(bits == TAG_NULL) | (bits == TAG_UNDEF)`. + let left_is_null = matches!(left.as_ref(), Expr::Null); + let left_is_undef = matches!(left.as_ref(), Expr::Undefined); + let right_is_null = matches!(right.as_ref(), Expr::Null); + let right_is_undef = matches!(right.as_ref(), Expr::Undefined); + let either_nullish_lit = + left_is_null || left_is_undef || right_is_null || right_is_undef; + if either_nullish_lit + && matches!( + op, + CompareOp::Eq | CompareOp::Ne | CompareOp::LooseEq | CompareOp::LooseNe + ) + { + let blk = ctx.block(); + let l_bits = blk.bitcast_double_to_i64(&l); + let r_bits = blk.bitcast_double_to_i64(&r); + let is_loose = matches!(op, CompareOp::LooseEq | CompareOp::LooseNe); + let bit = if is_loose { + // Loose equality: x == null → (x === null) || (x === undefined) + let eq_l_r = blk.icmp_eq(I64, &l_bits, &r_bits); + let cmp_l_null = blk.icmp_eq(I64, &l_bits, crate::nanbox::TAG_NULL_I64); + let cmp_l_undef = + blk.icmp_eq(I64, &l_bits, crate::nanbox::TAG_UNDEFINED_I64); + let cmp_r_null = blk.icmp_eq(I64, &r_bits, crate::nanbox::TAG_NULL_I64); + let cmp_r_undef = + blk.icmp_eq(I64, &r_bits, crate::nanbox::TAG_UNDEFINED_I64); + let l_nullish = blk.or(crate::types::I1, &cmp_l_null, &cmp_l_undef); + let r_nullish = blk.or(crate::types::I1, &cmp_r_null, &cmp_r_undef); + let both_nullish = blk.and(crate::types::I1, &l_nullish, &r_nullish); + blk.or(crate::types::I1, &eq_l_r, &both_nullish) + } else { + // Strict equality: bit-exact compare + blk.icmp_eq(I64, &l_bits, &r_bits) + }; + let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + blk.xor(crate::types::I1, &bit, "true") + } else { + bit + }; let tagged = blk.select( crate::types::I1, - &inv, + &bit_final, I64, crate::nanbox::TAG_TRUE_I64, crate::nanbox::TAG_FALSE_I64, ); return Ok(blk.bitcast_i64_to_double(&tagged)); } - return Ok(blk.bitcast_i64_to_double(&result_bits)); - } - - // An ordered relational compare (`<`, `<=`, `>`, `>=`) whose - // operands aren't BOTH statically numeric needs the full ECMAScript - // Abstract Relational Comparison: ToPrimitive (`{valueOf}`/`Date`), - // lexicographic string compare, BigInt-vs-Number/String coercion, - // and null/boolean/string ToNumber. A bare `fcmp` mishandles all of - // these (NaN-boxed operands are unordered → always `false`). Route - // through the runtime `js_rel_*` helpers, which return a NaN-boxed - // boolean. The statically-numeric case keeps the bare `fcmp` fast - // path below (and Dates are subsumed — they aren't numeric_expr). - let both_numeric = is_numeric_expr(ctx, left) - && is_numeric_expr(ctx, right) - && !expr_may_return_boxed_value_from_raw_f64_fallback(ctx, left) - && !expr_may_return_boxed_value_from_raw_f64_fallback(ctx, right) - && !is_bigint_expr(ctx, left) - && !is_bigint_expr(ctx, right); - if is_relational_op && !both_numeric { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let fname = match op { - CompareOp::Lt => "js_rel_lt", - CompareOp::Le => "js_rel_le", - CompareOp::Gt => "js_rel_gt", - CompareOp::Ge => "js_rel_ge", - _ => unreachable!(), + // Strict equality against a string LITERAL. Decidable inline for + // every runtime shape (see `lower_string_literal_strict_eq`), so it + // pre-empts all the arms below — including the `js_eq` tail that an + // `any`-typed operand like `n.kind` would otherwise take, one call + // pair per comparison. Strict only: loose `==` coerces (`"5" == 5`) + // and stays on `js_loose_eq`. `Expr::WtfString` is excluded — its + // pool bytes are the WTF-8 encoding, not `str::as_bytes`. + let lit_on_right = matches!(right.as_ref(), Expr::String(_)); + let lit_on_left = !lit_on_right && matches!(left.as_ref(), Expr::String(_)); + if (lit_on_right || lit_on_left) && matches!(op, CompareOp::Eq | CompareOp::Ne) { + // Source order: the non-literal operand may have side effects. + let (val, lit_box, lit) = if lit_on_right { + let Expr::String(s) = right.as_ref() else { + unreachable!("lit_on_right implies Expr::String") + }; + (l, r, s.clone()) + } else { + let Expr::String(s) = left.as_ref() else { + unreachable!("lit_on_left implies Expr::String") + }; + (r, l, s.clone()) + }; + let bit = lower_string_literal_strict_eq(ctx, &val, &lit_box, &lit); + let blk = ctx.block(); + let bit_final = if matches!(op, CompareOp::Ne) { + blk.xor(I1, &bit, "true") + } else { + bit + }; + let tagged = blk.select( + I1, + &bit_final, + I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged)); + } + // "One side is statically string, other is unknown" + // fallback: `c === Color.Red` where Color is a const + // object. Neither js_eq (bit-compare, wrong for string + // content) nor fcmp (NaN-tagged, always false) works. + // + // Dispatch through js_string_equals after extracting + // both string pointers via js_get_string_pointer_unified. + // That helper returns null for non-string NaN-tagged + // values, which js_string_equals treats as "not equal" + // — the correct answer when the unknown side isn't a + // string at runtime. + let both_strings_check = is_string_expr(ctx, left) && is_string_expr(ctx, right); + // The non-statically-string operand collides through this + // fast path when, at runtime, it is ALSO a non-string. Both + // operands then funnel through `js_get_string_pointer_unified`, + // which returns 0 for any non-string NaN-boxed value (numbers, + // class refs / InjectionTokens, plain objects, …). The + // subsequent `js_string_equals(0, 0)` returns 1 (its + // pointer-identity / both-null branches both report "equal"), + // so two *distinct* non-string values wrongly compare `===`. + // + // This is exactly the NestJS DI `token === name` bug: + // `name` is statically `string` (the destructured + // `dependencyContext.name`) but at runtime holds a class ref + // (e.g. `AppService`), and `token` is `any` holding a + // *different* class ref (`AppController`) — both coerce to 0 + // and the inline `===` reports `true`, throwing + // `UnknownDependencies` and aborting the app. + // + // The static `string` type is therefore a lie here (like the + // #3576 number-vs-object case). When the OTHER operand is + // statically `Any` (its runtime value is unconstrained and may + // be a non-string), this fast path is unsound: route through + // `js_eq`, which content-compares real strings (SSO + heap) AND + // correctly distinguishes class refs / objects by identity. + let other_side_is_any = |other: &Expr| -> bool { + matches!( + crate::type_analysis::static_type_of(ctx, other), + Some(HirType::Any) | None + ) }; - let res = blk.call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]); - return Ok(res); - } - // Strict ===/!== where the operands are NOT both certainly - // numeric must NOT fall to the bare fcmp tail: a declared - // `Number` local can carry an object at runtime (`var a = 2; - // f(){ a = o; } f(); a === o` — the static type lies, and fcmp - // on NaN-boxed pointers is unordered → permanently false). - // js_eq answers correctly for every runtime shape, including - // the honest number-vs-object case (#3576 probe family). - if matches!(op, CompareOp::Eq | CompareOp::Ne) && !both_numeric { - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let blk = ctx.block(); - let l_bits = blk.bitcast_double_to_i64(&l); - let r_bits = blk.bitcast_double_to_i64(&r); - let result_bits = blk.call(I64, "js_eq", &[(I64, &l_bits), (I64, &r_bits)]); - if matches!(op, CompareOp::Ne) { - let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); - let inv = blk.xor(crate::types::I1, &cmp, "true"); + let one_side_string = !both_strings_check + && ((is_string_expr(ctx, left) + && !is_numeric_expr(ctx, right) + && !is_bool_expr(ctx, right) + && !other_side_is_any(right)) + || (is_string_expr(ctx, right) + && !is_numeric_expr(ctx, left) + && !is_bool_expr(ctx, left) + && !other_side_is_any(left))); + // Only STRICT eq/ne use this string-pointer fast path. Loose `==`/`!=` + // must fall through to `js_loose_eq` below: when one side is a boxed + // String/primitive *wrapper* (a POINTER_TAG object, not a STRING_TAG + // value), `js_get_string_pointer_unified` returns the raw ObjectHeader + // pointer and `js_string_equals` reads it as a bogus string → wrong + // result (`new String("x") == "x"` was `false`). `js_loose_eq` unboxes + // the wrapper first. Strict `=== "lit"` is unaffected (both sides are + // real strings at runtime). #boxed-loose-eq. + if one_side_string && matches!(op, CompareOp::Eq | CompareOp::Ne) { + let blk = ctx.block(); + let l_handle = blk.call(I64, "js_get_string_pointer_unified", &[(DOUBLE, &l)]); + let r_handle = blk.call(I64, "js_get_string_pointer_unified", &[(DOUBLE, &r)]); + let i32_eq = blk.call( + I32, + "js_string_equals", + &[(I64, &l_handle), (I64, &r_handle)], + ); + let bit = blk.icmp_ne(I32, &i32_eq, "0"); + let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + blk.xor(crate::types::I1, &bit, "true") + } else { + bit + }; let tagged = blk.select( crate::types::I1, - &inv, + &bit_final, I64, crate::nanbox::TAG_TRUE_I64, crate::nanbox::TAG_FALSE_I64, ); return Ok(blk.bitcast_i64_to_double(&tagged)); } - return Ok(blk.bitcast_i64_to_double(&result_bits)); - } - let l = lower_expr(ctx, left)?; - let r = lower_expr(ctx, right)?; - let pred = match op { - CompareOp::Eq => "oeq", - // !== uses `une` (unordered or not equal), NOT `one`. - // `one` is "ordered and not equal" which returns false - // when either operand is NaN. JS !== on NaN must return - // true: NaN !== NaN → !(NaN === NaN) → !false → true. - CompareOp::Ne => "une", - CompareOp::Lt => "olt", - CompareOp::Le => "ole", - CompareOp::Gt => "ogt", - CompareOp::Ge => "oge", - // LooseEq/Ne handled above - CompareOp::LooseEq | CompareOp::LooseNe => unreachable!(), - }; - let blk = ctx.block(); - let bit = blk.fcmp(pred, &l, &r); - let tag_true_i64 = crate::nanbox::TAG_TRUE_I64; - let tag_false_i64 = crate::nanbox::TAG_FALSE_I64; - let tagged_i64 = blk.select( - crate::types::I1, - &bit, - crate::types::I64, - tag_true_i64, - tag_false_i64, - ); - Ok(blk.bitcast_i64_to_double(&tagged_i64)) + // Generic equality fallback: when neither operand is + // statically numeric, dispatch through js_eq which + // handles strings, booleans, objects, null, undefined + // via NaN-tag inspection. Used by `eq` helpers in tests + // that take `any` and pass NaN-tagged values. + let either_non_numeric = + !is_numeric_expr(ctx, left) && !is_numeric_expr(ctx, right); + let only_eq = matches!( + op, + CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe + ); + // We still let the more specific paths below win for + // statically-typed string/bool operands; this fallback + // only handles the truly-Any case. + let unknown_l = !is_numeric_expr(ctx, left) + && !is_string_expr(ctx, left) + && !is_bool_expr(ctx, left); + let unknown_r = !is_numeric_expr(ctx, right) + && !is_string_expr(ctx, right) + && !is_bool_expr(ctx, right); + if either_non_numeric && only_eq && unknown_l && unknown_r { + // Use js_loose_eq for == / != (handles null==undefined, + // cross-type coercion). STRICT `===`/`!==` gets the inline + // prefix instead: the operands that reach here are + // unconstrained, and a generic-container key scan + // (`this.keys[i] === k`) spends its whole cost on this one + // call. Loose `==`'s cross-type coercions are not + // bit-decidable, so it keeps the bare call. + let result_bits = if matches!(op, CompareOp::LooseEq | CompareOp::LooseNe) { + let blk = ctx.block(); + let l_bits = blk.bitcast_double_to_i64(&l); + let r_bits = blk.bitcast_double_to_i64(&r); + blk.call(I64, "js_loose_eq", &[(I64, &l_bits), (I64, &r_bits)]) + } else { + lower_strict_eq_inline_any(ctx, &l, &r) + }; + let blk = ctx.block(); + let result = blk.bitcast_i64_to_double(&result_bits); + if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); + let inv = blk.xor(crate::types::I1, &cmp, "true"); + let tagged = blk.select( + crate::types::I1, + &inv, + I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged)); + } + return Ok(result); + } + + // String equality fast path: fcmp doesn't work on + // NaN-tagged string pointers (NaN comparisons are + // unordered → always false). When both operands are + // statically strings, dispatch through js_string_equals. + let both_strings = is_string_expr(ctx, left) && is_string_expr(ctx, right); + // Representation-selection Phase 3a: when a canonical-Str local + // is an operand, tag-dispatch inline instead of paying the two + // opaque (SSO-heap-materializing) unified unbox calls: both + // proven heap → direct `js_string_equals(h, h)` on the raw + // handles; any other mix → one `js_jsvalue_equals` call, which + // content-compares heap × SSO without materializing and never + // number-coerces (a lying annotation gets exact `===` + // semantics, strictly closer to spec than the legacy path). + let canonical_str_involved = matches!( + left.as_ref(), Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) + ) || matches!( + right.as_ref(), Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) + ); + if both_strings + && canonical_str_involved + && matches!( + op, + CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe + ) + { + let i32_eq = lower_string_strict_eq_inline(ctx, &l, &r, false); + let blk = ctx.block(); + let bit = blk.icmp_ne(I32, &i32_eq, "0"); + let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + blk.xor(crate::types::I1, &bit, "true") + } else { + bit + }; + let tagged_i64 = blk.select( + crate::types::I1, + &bit_final, + crate::types::I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged_i64)); + } + if both_strings + && matches!( + op, + CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe + ) + { + // Issue #214: SSO-safe unbox — the inline mask returns + // garbage for SHORT_STRING_TAG values (e.g. SSO results + // from `JSON.parse('["hello"]')[0]`), causing + // `js_string_equals` to deref the inline payload bytes. + // That unbox is now the *fallback* arm: identical bits and + // SSO x SSO are answered inline, which is what keeps a pair of + // short runtime strings (`charAt`, `substring`) from + // materializing two throwaway heap copies per comparison. + let i32_eq = lower_string_strict_eq_inline(ctx, &l, &r, true); + let blk = ctx.block(); + let bit = blk.icmp_ne(I32, &i32_eq, "0"); + let bit_final = if matches!(op, CompareOp::Ne | CompareOp::LooseNe) { + blk.xor(crate::types::I1, &bit, "true") + } else { + bit + }; + let tagged_i64 = blk.select( + crate::types::I1, + &bit_final, + crate::types::I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged_i64)); + } + // String relational fast path: `s1 < s2`, `s1 > s2`, etc. + // fcmp on NaN-tagged pointers is unordered (always false), + // so dispatch through js_string_compare which returns + // -1/0/1 like memcmp. Then test the result against 0 with + // the right icmp predicate. + // Representation-selection Phase 3a: relational counterpart of + // the canonical-Str equality arm above — both proven heap → + // direct `js_string_compare(h, h)`; any other mix → one + // `js_string_compare_value` call (SSO-aware, no heap + // materialization, numbers coerced via their decimal string + // form exactly like the legacy unified path). + if both_strings + && canonical_str_involved + && matches!( + op, + CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge + ) + { + let cmp_i32 = canonical_str_cmp_dispatch( + ctx, + &l, + &r, + "js_string_compare", + "js_string_compare_value", + "strcmp", + ); + let blk = ctx.block(); + let bit = match op { + CompareOp::Lt => blk.icmp_slt(I32, &cmp_i32, "0"), + CompareOp::Le => blk.icmp_sle(I32, &cmp_i32, "0"), + CompareOp::Gt => blk.icmp_sgt(I32, &cmp_i32, "0"), + CompareOp::Ge => blk.icmp_sge(I32, &cmp_i32, "0"), + _ => unreachable!(), + }; + let tagged_i64 = blk.select( + crate::types::I1, + &bit, + crate::types::I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged_i64)); + } + if both_strings + && matches!( + op, + CompareOp::Lt | CompareOp::Le | CompareOp::Gt | CompareOp::Ge + ) + { + let blk = ctx.block(); + // Issue #214: SSO-safe unbox. + let l_handle = unbox_str_handle(blk, &l); + let r_handle = unbox_str_handle(blk, &r); + let cmp_i32 = blk.call( + I32, + "js_string_compare", + &[(I64, &l_handle), (I64, &r_handle)], + ); + let bit = match op { + CompareOp::Lt => blk.icmp_slt(I32, &cmp_i32, "0"), + CompareOp::Le => blk.icmp_sle(I32, &cmp_i32, "0"), + CompareOp::Gt => blk.icmp_sgt(I32, &cmp_i32, "0"), + CompareOp::Ge => blk.icmp_sge(I32, &cmp_i32, "0"), + _ => unreachable!(), + }; + let tagged_i64 = blk.select( + crate::types::I1, + &bit, + crate::types::I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged_i64)); + } + + // Loose equality (==, !=): dispatch through js_loose_eq + // which handles cross-type coercion (null==undefined, + // "1"==1, false==0, etc.). Strict === already handled + // above by the typed fast paths. + if matches!(op, CompareOp::LooseEq | CompareOp::LooseNe) { + let blk = ctx.block(); + let l_bits = blk.bitcast_double_to_i64(&l); + let r_bits = blk.bitcast_double_to_i64(&r); + let result_bits = + blk.call(I64, "js_loose_eq", &[(I64, &l_bits), (I64, &r_bits)]); + if matches!(op, CompareOp::LooseNe) { + let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); + let inv = blk.xor(crate::types::I1, &cmp, "true"); + let tagged = blk.select( + crate::types::I1, + &inv, + I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged)); + } + return Ok(blk.bitcast_i64_to_double(&result_bits)); + } + + // An ordered relational compare (`<`, `<=`, `>`, `>=`) whose + // operands aren't BOTH statically numeric needs the full ECMAScript + // Abstract Relational Comparison: ToPrimitive (`{valueOf}`/`Date`), + // lexicographic string compare, BigInt-vs-Number/String coercion, + // and null/boolean/string ToNumber. A bare `fcmp` mishandles all of + // these (NaN-boxed operands are unordered → always `false`). Route + // through the runtime `js_rel_*` helpers, which return a NaN-boxed + // boolean. The statically-numeric case keeps the bare `fcmp` fast + // path below (and Dates are subsumed — they aren't numeric_expr). + let both_numeric = is_numeric_expr(ctx, left) + && is_numeric_expr(ctx, right) + && !expr_may_return_boxed_value_from_raw_f64_fallback(ctx, left) + && !expr_may_return_boxed_value_from_raw_f64_fallback(ctx, right) + && !is_bigint_expr(ctx, left) + && !is_bigint_expr(ctx, right); + if is_relational_op && !both_numeric { + let blk = ctx.block(); + let fname = match op { + CompareOp::Lt => "js_rel_lt", + CompareOp::Le => "js_rel_le", + CompareOp::Gt => "js_rel_gt", + CompareOp::Ge => "js_rel_ge", + _ => unreachable!(), + }; + let res = blk.call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]); + return Ok(res); + } + // Strict ===/!== where the operands are NOT both certainly + // numeric must NOT fall to the bare fcmp tail: a declared + // `Number` local can carry an object at runtime (`var a = 2; + // f(){ a = o; } f(); a === o` — the static type lies, and fcmp + // on NaN-boxed pointers is unordered → permanently false). + // js_eq answers correctly for every runtime shape, including + // the honest number-vs-object case (#3576 probe family). + if matches!(op, CompareOp::Eq | CompareOp::Ne) && !both_numeric { + let blk = ctx.block(); + let l_bits = blk.bitcast_double_to_i64(&l); + let r_bits = blk.bitcast_double_to_i64(&r); + let result_bits = blk.call(I64, "js_eq", &[(I64, &l_bits), (I64, &r_bits)]); + if matches!(op, CompareOp::Ne) { + let cmp = blk.icmp_eq(I64, &result_bits, crate::nanbox::TAG_TRUE_I64); + let inv = blk.xor(crate::types::I1, &cmp, "true"); + let tagged = blk.select( + crate::types::I1, + &inv, + I64, + crate::nanbox::TAG_TRUE_I64, + crate::nanbox::TAG_FALSE_I64, + ); + return Ok(blk.bitcast_i64_to_double(&tagged)); + } + return Ok(blk.bitcast_i64_to_double(&result_bits)); + } + let pred = match op { + CompareOp::Eq => "oeq", + // !== uses `une` (unordered or not equal), NOT `one`. + // `one` is "ordered and not equal" which returns false + // when either operand is NaN. JS !== on NaN must return + // true: NaN !== NaN → !(NaN === NaN) → !false → true. + CompareOp::Ne => "une", + CompareOp::Lt => "olt", + CompareOp::Le => "ole", + CompareOp::Gt => "ogt", + CompareOp::Ge => "oge", + // LooseEq/Ne handled above + CompareOp::LooseEq | CompareOp::LooseNe => unreachable!(), + }; + let blk = ctx.block(); + let bit = blk.fcmp(pred, &l, &r); + let tag_true_i64 = crate::nanbox::TAG_TRUE_I64; + let tag_false_i64 = crate::nanbox::TAG_FALSE_I64; + let tagged_i64 = blk.select( + crate::types::I1, + &bit, + crate::types::I64, + tag_true_i64, + tag_false_i64, + ); + Ok(blk.bitcast_i64_to_double(&tagged_i64)) + }) } // -------- Objects (Phase B.4) -------- diff --git a/crates/perry-codegen/src/expr/compare_tests.rs b/crates/perry-codegen/src/expr/compare_tests.rs index 7ee561456b..4d296306fc 100644 --- a/crates/perry-codegen/src/expr/compare_tests.rs +++ b/crates/perry-codegen/src/expr/compare_tests.rs @@ -23,6 +23,8 @@ use perry_hir::{CompareOp, Expr, Stmt}; /// ~90-line `CompileOptions` / `Module` harness. use crate::temp_root_coverage::main_ir_for as ir_for; +use super::slice8_rooting_tests::{call_operand_of, producer_line}; + const X: u32 = 1; const Y: u32 = 2; const R: u32 = 3; @@ -69,6 +71,77 @@ fn cmp_ir(name: &str, op: CompareOp, lhs: Expr, rhs: Expr) -> String { const JS_EQ_CALL: &str = "call i64 @js_eq("; const JS_LOOSE_EQ_CALL: &str = "call i64 @js_loose_eq("; +/// `makeLeft() === makeRight()` has to keep the first call result alive while +/// the second call runs. Object literals give the IR test the same two +/// allocating, pointer-valued temporaries without depending on call lowering: +/// before #7979, `js_eq`'s left operand traces back to the FIRST allocation; +/// after the fix it traces back to a root re-read below the second one. +/// +/// Follow the full pure-op chain instead of checking the `bitcast` handed to +/// `js_eq`: that bitcast is emitted below both allocations even when its input +/// is the stale pre-collection register, which made a one-level ordering check +/// green against the bug. +#[test] +fn strict_eq_rereads_its_left_operand_below_an_allocating_right_operand() { + let ir = cmp_ir( + "streq_rooted_operands", + CompareOp::Eq, + Expr::Object(vec![("left".to_string(), Expr::Number(1.0))]), + Expr::Object(vec![("right".to_string(), Expr::Number(2.0))]), + ); + let left = call_operand_of(&ir, "js_eq", 0); + let right = call_operand_of(&ir, "js_eq", 1); + let left_producer = producer_line(&ir, &left); + let right_producer = producer_line(&ir, &right); + assert!( + left_producer > right_producer, + "js_eq's left operand ({left}) is produced at line {left_producer}, above the right \ + operand ({right}) at line {right_producer}. The right allocation can collect, so the \ + left value must be rooted before it and re-read below it.\n{ir}" + ); +} + +/// The complementary cost assertion: even with an allocating right operand, +/// a proven-number left operand cannot be invalidated by relocation and must +/// stay in its original register. A blanket "root every comparison" fix would +/// move its producer below the right allocation and fail this test. +#[test] +fn strict_eq_reuses_a_non_pointer_left_operand_across_an_allocating_right_operand() { + let ir = ir_for( + "streq_reused_primitive", + vec![ + Stmt::Let { + id: X, + name: "x".to_string(), + ty: Type::Number, + mutable: false, + init: Some(Expr::Number(1.0)), + }, + Stmt::Let { + id: R, + name: "r".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(Expr::LocalGet(X)), + right: Box::new(Expr::Object(vec![("right".to_string(), Expr::Number(2.0))])), + }), + }, + ], + ); + let left = call_operand_of(&ir, "js_eq", 0); + let right = call_operand_of(&ir, "js_eq", 1); + let left_producer = producer_line(&ir, &left); + let right_producer = producer_line(&ir, &right); + assert!( + left_producer < right_producer, + "a non-pointer numeric operand cannot become stale, so it should stay in the \ + register produced at line {left_producer}, above the right allocation at line \ + {right_producer}. Rooting/re-reading it adds traffic without protecting anything.\n{ir}" + ); +} + #[test] fn strict_eq_against_a_string_literal_emits_the_inline_dispatch_and_no_js_eq_call() { let ir = cmp_ir( diff --git a/test-files/test_gap_gc_define_property_descriptor_rooting.ts b/test-files/test_gap_gc_define_property_descriptor_rooting.ts index d7ba7d100c..d2df57cc05 100644 --- a/test-files/test_gap_gc_define_property_descriptor_rooting.ts +++ b/test-files/test_gap_gc_define_property_descriptor_rooting.ts @@ -170,31 +170,17 @@ function expectedIndexed(prefix: string, valuePrefix: string, count: number): st return parts.join("|"); } -// NOTE — why each side is bound to a `const` instead of being compared inline. -// -// `console.log("x", f() === g() ? …)` leaves `f()`'s result as an SSA temporary -// that is live across `g()`. Under this witness configuration `g()` allocates -// through several loop back-edges, so it collects, and the temporary names -// from-space: the run faults inside `js_jsvalue_equals` (frame `js_eq` <- `main`) -// on BOTH a pristine build and this branch. That is a SEPARATE, pre-existing -// codegen root-dominance defect — the class -// `scripts/gc_root_dominance_check.py` exists for — and it has nothing to do -// with `Object.defineProperty`. Binding both sides first keeps this program a -// witness for ONE defect. See the issue filed alongside #7963. -const groupByObserved = objectGroupBy(); -const groupByExpected = expectedObjectGroupBy(); -console.log("objectGroupBy", groupByObserved === groupByExpected ? "ok" : "BAD"); - -const oneAtATimeObserved = definePropertyOneAtATime(); -const oneAtATimeExpected = expectedIndexed("prop-", "value-", 12); +// Keep the verdict operands inline: each left call result must remain rooted +// while the allocating right call runs (#7979). This is both the natural probe +// style and the runtime witness for comparison operand rooting. +console.log("objectGroupBy", objectGroupBy() === expectedObjectGroupBy() ? "ok" : "BAD"); console.log( "definePropertyOneAtATime", - oneAtATimeObserved === oneAtATimeExpected ? "ok" : "BAD", + definePropertyOneAtATime() === expectedIndexed("prop-", "value-", 12) ? "ok" : "BAD", ); - -const accessorObserved = definePropertyWithAllocatingDescriptorGetters(); -const accessorExpected = expectedIndexed("key-", "v", 12); console.log( "definePropertyAccessorDescriptor", - accessorObserved === accessorExpected ? "ok" : "BAD", + definePropertyWithAllocatingDescriptorGetters() === expectedIndexed("key-", "v", 12) + ? "ok" + : "BAD", ); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index e6dafc8811..850dd77493 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -740,6 +740,19 @@ test_gap_gc_class_field_receiver_rooting test_gap_gc_index_set_bounded_globalthis_ta_rooting test_gap_gc_namespace_and_computed_dispatch_rooting +# #7949 (PR #7962): Rust-side containers retained raw JS values across +# allocating callbacks and property operations. These two witnesses shipped +# with that fix but were never registered, leaving their moving-GC coverage +# dark and the registration lint red on main. +test_gap_gc_container_value_rooting +test_gap_gc_define_properties_key_rooting + +# #7979: comparison operands are lowered left-to-right, so the first call +# result must remain rooted while an allocating second call runs. This is the +# #7963 defineProperty witness restored to its natural inline verdict shape; +# retired-from-space protection made the pre-fix js_eq stale read fault. +test_gap_gc_define_property_descriptor_rooting + # #7766 (PR #7778): the element-binding clone through a function boundary. # Lying callers force the slow path while the honest arm's clone reads raw # f64 slots off relocatable objects — live on the evacuating arms (9 copying