diff --git a/changelog.d/11643-inline-number-to-string.md b/changelog.d/11643-inline-number-to-string.md new file mode 100644 index 0000000000..3f3736ca31 --- /dev/null +++ b/changelog.d/11643-inline-number-to-string.md @@ -0,0 +1,37 @@ +perf(codegen): number-to-string builds a small integer's text at the call site, and its result keeps its string proof (#10762). + +- `String(n)`, `` `${n}` ``, `n.toString()` and `"" + n` on an operand the type + analysis proves numeric now compute the SSO bits of an integer in + `-9999..=99999` inline, with no call. The digits come from a fixed-point + split (`abs * ceil(2^32 / 10^4)`, then `* 10` per digit, exact for every + `abs < 100000`), about 30 branch-free instructions. Every other value + (fractions, `NaN`, the infinities, larger integers, and a declared `number` + that holds something else at run time) takes the original runtime call on + a cold arm, so the text is unchanged, and the bits match + `small_integer_sso_bits` exactly. An operand without a numeric proof keeps + the plain call, so the code is emitted only where it is expected to fire. +- `const s = String(n)`, `` `${n}` `` and `"" + n` (a `+` with a string-literal + operand) record a runtime-derived `String` proof for `s`. Before, the local + lost the proof its initializer carries, and `s.charCodeAt(i)`, `s.length` + and the other string lowerings fell to the generic method site, although + `String(n).charCodeAt(i)` written inline took the fast path. A declared + `string` is still not a proof (#7837). +- The inline `charCodeAt` reads an all-ASCII SSO receiver's byte straight out + of the value. Before, an SSO receiver, which is what every short number's text + now is, went to the slow arm, which materialized a heap copy through the + intern table (about 175 instructions) to read one byte. + +Measured (`callgrind`, instructions per iteration fitted over 20k→120k, x86-64, +`PERRY_NO_AUTO_OPTIMIZE=1`, `k = i % 1000`, the text consumed by +`charCodeAt(0) + length`): `String(n)` 449 → 169, `` `${n}` `` 462 → 169, +`String(-k)` 644 → 199, a fraction 1,251 → 987, and a 7-digit integer (a heap +string) 623 → 560 over 1M→3M. Consumed only by `.length`: `String(n)` +163 → 131, `"" + n` 186 → 133, `` `${n}` `` 172 → 131, `n.toString()` +160 → 133. A bare loop is 23. + +Tests: `number_to_string_inline_tests` (each spelling emits the inline arm; an +`any` operand does not; `s.charCodeAt` on each coerced local reaches the SSO +arm), sabotage-checked, and `test_gap_10762_inline_number_to_string` (every +integer in the inline range against a reference, the range edges, lying +annotations, SSO `charCodeAt` including non-ASCII and bad indexes; +byte-identical to Node). No version bump. diff --git a/changelog.d/11643-map-own-override-flag-builtin-installs.md b/changelog.d/11643-map-own-override-flag-builtin-installs.md new file mode 100644 index 0000000000..72bb7928e4 --- /dev/null +++ b/changelog.d/11643-map-own-override-flag-builtin-installs.md @@ -0,0 +1,44 @@ +perf(runtime): populating `globalThis` no longer puts every `Map`/`Set`/`Date` builtin call on the slow side of the own-override guard (#10697). + +The #10943 guard in front of a proven `Map`/`Set`/`Date` builtin call answers +"no own override" from one load of `PERRY_OWN_NAMED_PROP_INSTALLED`, and asks +the authoritative `hasOwn` predicate only once any non-ordinary cell has taken +a named property. The runtime's own lazy `globalThis` population set that flag: +it installs statics on constructor intrinsics such as `%TypedArray%` (closure +cells), `constructor` on `Array.prototype` (an array cell) and aliases such as +`Number.parseFloat`, and each of those stores passes the exotic-store gauntlet +that arms it. Any program that referenced `globalThis` or a lazily installed +global then paid about 600 instructions of `js_receiver_may_own_named_method` +→ `js_object_has_own` (with a key-string coercion) on every `m.get(k)` and +`m.set(k, v)`. That is the "count by category" shape #10697 measured at 4.2x +node. + +- The runtime's own builtin definitions (`define_builtin_data_property` and all + of `populate_global_this_builtins`) run inside `as_builtin_definition`. Inside + it, an install arms the exported flag only when its owner is a Map, Set or + Date cell, or its header cannot be read. Those are the only receivers whose + guard answer comes from the flag: an array answers from its own header and + named-property storage, and a declared-`Map` receiver of any other kind is + brand-checked into generic dispatch. User installs arm exactly as before. +- The universal dispatcher's `own_user_method_value` now reads a separate + internal flag that every install still arms, builtin or not, so its answers + are unchanged. +- Small-map lookups (≤ 8 entries) answer a bit-identical key of any type from + the inlined hot lane. A Map never holds two SameValueZero-equal keys and + stored keys are normalized, so an identity hit is the match. Before, only a + plain-number key could use that lane, and every string lookup paid the + out-of-line call to `find_key_index_cold` first. + +Measured (`callgrind`, instructions per iteration fitted over 20k→120k, x86-64, +`PERRY_NO_AUTO_OPTIMIZE=1`, program references `globalThis`): four constant +string keys, get-or-default then set, 1,866 → 373; a plain `m.get(CATS[i & 3])` +947 → 206, of which 111 is the array read itself. Output identical to Node. + +Tests: `a_builtin_install_on_an_intrinsic_does_not_arm_the_guard` (272 arms +from one intrinsic install before; 0 now, while installs onto a Map, builtin +or user, still arm), `a_small_map_answers_an_identical_key_without_the_cold_path` +(8 of 8 identical lookups went cold before; 0 now, and content matches still +go cold), both sabotage-checked, and +`test_gap_10697_own_override_after_global_population` (own overrides on +Map/Set/Date/Array still win after population, and the populated builtins still +work). No version bump. diff --git a/changelog.d/11643-random-uuid-format.md b/changelog.d/11643-random-uuid-format.md new file mode 100644 index 0000000000..bde442de4e --- /dev/null +++ b/changelog.d/11643-random-uuid-format.md @@ -0,0 +1,20 @@ +perf(uuid): `randomUUID()` formats without bounds checks or a UTF-8 re-validation (#10523). + +#10523's main cost, one `getrandom` system call per UUID, was fixed by the +per-thread entropy cache (#11351): 120,000 UUIDs now make 944 calls, one per 128, +the batching Node uses. This removes most of what remained in user space: + +- `Hyphenated::new` wrote each byte's two hex digits at a running index with a + per-byte hyphen test, so every store was bounds-checked. A constant table of + digit positions unrolls to straight-line stores. +- `js_crypto_random_uuid` and `js_crypto_random_uuidv7` copy the 36 bytes + through the new `Hyphenated::as_bytes`. `as_str` validated known-ASCII bytes + as UTF-8 on every UUID only for the string constructor to copy them. + +Measured (`callgrind`, whole-process instructions ÷ UUIDs, `node:crypto` +`randomUUID()`): 1,085 → 1,005 per UUID at 200k, 1,119 → 933 at 1M. The two +hot functions fell from about 367 to 290 instructions per UUID, and the 56 of +UTF-8 validation are gone. + +Tests: `hyphenated_layout_is_exact` pins the digit order and hyphen positions +byte for byte, and that `as_bytes` equals `as_str`'s bytes. No version bump. diff --git a/crates/perry-codegen/src/expr/logical_collections.rs b/crates/perry-codegen/src/expr/logical_collections.rs index 891aae37bf..0892ca0f13 100644 --- a/crates/perry-codegen/src/expr/logical_collections.rs +++ b/crates/perry-codegen/src/expr/logical_collections.rs @@ -1188,13 +1188,40 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // and `${i}` allocate nothing for it. The result is therefore // SSO-or-heap, not heap — see `proven_heap_string_operand`. Expr::StringCoerce(operand) => { + let numeric = crate::type_analysis::is_numeric_expr(ctx, operand); let v = lower_expr(ctx, operand)?; + // A number operand's small-integer text is built inline (#10762). + if numeric { + return crate::expr::number_to_string_inline::emit_number_to_string_inline( + ctx, + &v, + |ctx| { + Ok(ctx + .block() + .call(DOUBLE, "js_string_coerce_box", &[(DOUBLE, &v)])) + }, + ); + } Ok(ctx .block() .call(DOUBLE, "js_string_coerce_box", &[(DOUBLE, &v)])) } Expr::TemplateStringCoerce(operand) => { + let numeric = crate::type_analysis::is_numeric_expr(ctx, operand); let v = lower_expr(ctx, operand)?; + if numeric { + return crate::expr::number_to_string_inline::emit_number_to_string_inline( + ctx, + &v, + |ctx| { + Ok(ctx.block().call( + DOUBLE, + "js_template_string_coerce_box", + &[(DOUBLE, &v)], + )) + }, + ); + } // S2: a string operand is answered inline; see `ic_fast_split.rs`. Ok(crate::expr::ic_fast_split::emit_template_string_coerce( ctx, &v, diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index af0a51c247..6ff2580ec5 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -3084,6 +3084,9 @@ mod math_simple; pub(crate) mod method_site; mod misc_methods; mod new_dynamic; +pub(crate) mod number_to_string_inline; +#[cfg(test)] +mod number_to_string_inline_tests; mod objects_arrays_lit; pub(crate) mod os_uri_dates; pub(crate) mod property_get; diff --git a/crates/perry-codegen/src/expr/number_to_string_inline.rs b/crates/perry-codegen/src/expr/number_to_string_inline.rs new file mode 100644 index 0000000000..6f84bc5a5b --- /dev/null +++ b/crates/perry-codegen/src/expr/number_to_string_inline.rs @@ -0,0 +1,125 @@ +//! Inline small-integer number-to-string (#10762). +//! +//! `String(n)`, `` `${n}` ``, `n.toString()` and `"" + n` on a number all +//! reach `perry-runtime`'s `small_integer_sso_bits` for an integer in +//! `-9999..=99999`: the answer is an SSO immediate, computed with no +//! allocation. What was left was the call itself and the tag ladder in front +//! of it. This emits the same computation at the call site for operands the +//! type analysis says are numbers, and keeps the runtime call on a cold arm +//! for everything else, including a declared-`number` slot that holds +//! something else at run time. +//! +//! The bits are identical to the runtime's, so the two arms are +//! interchangeable: most significant digit in byte 0, a leading `-` below the +//! digits, and the length at `SHORT_STRING_LEN_SHIFT`. `-0` converts to `0` +//! and compares equal to `0.0`, so it prints `"0"` like the runtime. + +use super::FnCtx; +use crate::nanbox::{double_literal, i64_literal, SHORT_STRING_TAG}; +use crate::types::{DOUBLE, I1, I32, I64}; + +/// Bit offset of an SSO string's length byte. Must match +/// `perry-runtime::value::tags::SHORT_STRING_LEN_SHIFT`. +const SHORT_STRING_LEN_SHIFT: u64 = 40; + +/// Emit the SSO bits of `v` inline when it is an integral double in +/// `-9999..=99999`, and `slow(ctx)` otherwise. Returns the merged NaN-boxed +/// string. `slow` runs with the builder positioned in the cold block and must +/// return a DOUBLE. +pub(crate) fn emit_number_to_string_inline( + ctx: &mut FnCtx<'_>, + v: &str, + slow: impl FnOnce(&mut FnCtx<'_>) -> anyhow::Result, +) -> anyhow::Result { + let int_idx = ctx.new_block("num2str.int"); + let fast_idx = ctx.new_block("num2str.fast"); + let slow_idx = ctx.new_block("num2str.slow"); + let merge_idx = ctx.new_block("num2str.merge"); + let int_label = ctx.block_label(int_idx); + let fast_label = ctx.block_label(fast_idx); + let slow_label = ctx.block_label(slow_idx); + let merge_label = ctx.block_label(merge_idx); + + // Range first: `fptosi` of an out-of-range value is poison, so it runs + // only behind this branch. Every comparison with a NaN — a NaN-boxed + // non-number included — is false. + { + let blk = ctx.block(); + let lo = blk.fcmp("oge", v, &double_literal(-9_999.0)); + let hi = blk.fcmp("ole", v, &double_literal(99_999.0)); + let in_range = blk.and(I1, &lo, &hi); + blk.cond_br(&in_range, &int_label, &slow_label); + } + + ctx.current_block = int_idx; + let int = { + let blk = ctx.block(); + let int = blk.fptosi(DOUBLE, v, I32); + let back = blk.sitofp(I32, &int, DOUBLE); + let integral = blk.fcmp("oeq", &back, v); + blk.cond_br(&integral, &fast_label, &slow_label); + int + }; + + ctx.current_block = fast_idx; + let fast = { + let blk = ctx.block(); + let neg = blk.icmp_slt(I32, &int, "0"); + let negated = blk.sub(I32, "0", &int); + let abs32 = blk.select(I1, &neg, I32, &negated, &int); + let abs = blk.zext(I32, &abs32, I64); + // Five digits, most significant first, by fixed-point division: + // `abs * ceil(2^32 / 10^4)` puts `abs / 10^4` in the high word and + // the scaled remainder in the low word, and each `* 10` of the low + // word shifts the next digit up. Exact for every `abs < 100000` + // (checked exhaustively), one multiply per digit where the plain + // `/ 10`, `% 10` pairs cost about five instructions each. Leading + // zeros are digits too, so they are shifted out below. + let low_mask = i64_literal(0xFFFF_FFFF); + let mut scaled = blk.mul(I64, &abs, "429497"); + let mut packed = blk.lshr(I64, &scaled, "32"); + for byte in 1..5u32 { + let low = blk.and(I64, &scaled, &low_mask); + scaled = blk.mul(I64, &low, "10"); + let digit = blk.lshr(I64, &scaled, "32"); + let shifted = blk.shl(I64, &digit, &(byte * 8).to_string()); + packed = blk.or(I64, &packed, &shifted); + } + let ascii = blk.or(I64, &packed, &i64_literal(0x30_3030_3030)); + let mut ndig = "1".to_string(); + for bound in ["10", "100", "1000", "10000"] { + let ge = blk.icmp_uge(I64, &abs, bound); + let one = blk.zext(I1, &ge, I64); + ndig = blk.add(I64, &ndig, &one); + } + let zeros = blk.sub(I64, "5", &ndig); + let shift = blk.shl(I64, &zeros, "3"); + let payload = blk.lshr(I64, &ascii, &shift); + let with_sign = blk.shl(I64, &payload, "8"); + let with_sign = blk.or(I64, &with_sign, &(b'-' as u64).to_string()); + let signed_len = blk.add(I64, &ndig, "1"); + let payload = blk.select(I1, &neg, I64, &with_sign, &payload); + let len = blk.select(I1, &neg, I64, &signed_len, &ndig); + let len = blk.shl(I64, &len, &SHORT_STRING_LEN_SHIFT.to_string()); + let bits = blk.or(I64, &payload, &len); + let bits = blk.or(I64, &bits, &i64_literal(SHORT_STRING_TAG)); + let boxed = blk.bitcast_i64_to_double(&bits); + blk.br(&merge_label); + boxed + }; + + ctx.current_block = slow_idx; + let slow_val = slow(ctx)?; + let slow_end = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = merge_idx; + let fast_label_end = ctx.block_label(fast_idx); + Ok(ctx.block().phi( + DOUBLE, + &[ + (fast.as_str(), fast_label_end.as_str()), + (slow_val.as_str(), slow_end.as_str()), + ], + )) +} diff --git a/crates/perry-codegen/src/expr/number_to_string_inline_tests.rs b/crates/perry-codegen/src/expr/number_to_string_inline_tests.rs new file mode 100644 index 0000000000..eba2b894df --- /dev/null +++ b/crates/perry-codegen/src/expr/number_to_string_inline_tests.rs @@ -0,0 +1,206 @@ +//! #10762: which lowering the number-to-string spellings and a following +//! `charCodeAt` select. The runtime answers are pinned by +//! `test_gap_10762_inline_number_to_string.ts`; these pin that the inline arms +//! are actually emitted, and emitted only where the operand is a number. + +use crate::{compile_module, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{BinaryOp, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +const NUM_ID: u32 = 1; +const STR_ID: u32 = 2; +const INLINE_FAST: &str = "num2str.fast"; +const SSO_CHAR_CODE: &str = "cca.sso_fast"; + +fn ir_opts() -> CompileOptions { + CompileOptions { + emit_ir_only: true, + output_type: "executable".to_string(), + ..Default::default() + } +} + +fn param(ty: Type) -> Param { + Param { + id: NUM_ID, + name: "n".to_string(), + ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +fn module_with(params: Vec, body: Vec) -> Module { + Module { + name: "num2str_inline.ts".to_string(), + imports: Vec::new(), + exports: Vec::new(), + classes: Vec::new(), + interfaces: Vec::new(), + type_aliases: Vec::new(), + enums: Vec::new(), + globals: Vec::new(), + functions: vec![Function { + id: 1, + name: "probe".to_string(), + type_params: Vec::new(), + params, + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: true, + captures: Vec::new(), + decorators: Vec::new(), + }], + script_global_functions: Vec::new(), + references_global_this: false, + annexb_global_undefined_names: Vec::new(), + init_is_strict: false, + init: Vec::new(), + classic_for_lexical_bindings: std::collections::HashSet::new(), + exported_native_instances: Vec::new(), + exported_func_return_native_instances: Vec::new(), + exported_objects: Vec::new(), + exported_functions: Vec::new(), + widgets: Vec::new(), + uses_fetch: false, + uses_webassembly: false, + extern_funcs: Vec::new(), + init_was_unrolled: false, + has_top_level_await: false, + init_kind: ModuleInitKind::Eager, + async_step_closures: std::collections::HashSet::new(), + closure_display_names: std::collections::HashMap::new(), + class_display_names: std::collections::HashMap::new(), + closure_source_text: std::collections::HashMap::new(), + class_source_text: std::collections::HashMap::new(), + async_generator_funcs: std::collections::HashSet::new(), + local_source_spans: std::collections::HashMap::new(), + gen_param_prologue_len: std::collections::HashMap::new(), + } +} + +/// The whole module's IR: whichever clones of the body a typed parameter +/// produces, the lowering under test is in it. +fn ir(params: Vec, body: Vec) -> String { + let module = module_with(params, body); + String::from_utf8(compile_module(&module, ir_opts()).unwrap()).expect("LLVM IR is UTF-8") +} + +fn number_n() -> Expr { + // `n * 1` — numeric by construction, whatever the parameter's proof. + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(Expr::LocalGet(NUM_ID)), + right: Box::new(Expr::Integer(1)), + } +} + +fn method(object: Expr, property: &str, args: Vec) -> Expr { + Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(object), + property: property.to_string(), + byte_offset: 0, + }), + args, + type_args: Vec::new(), + byte_offset: 0, + } +} + +fn returns(e: Expr) -> Vec { + vec![Stmt::Return(Some(e))] +} + +/// Every spelling of a number's conversion gets the inline arm. +/// +/// Sabotage: removing any one call site's `emit_number_to_string_inline` +/// leaves its IR without `num2str.fast`. +#[test] +fn each_spelling_of_a_number_conversion_is_inlined() { + let spellings = [ + ("String(n)", Expr::StringCoerce(Box::new(number_n()))), + ("`${n}`", Expr::TemplateStringCoerce(Box::new(number_n()))), + ("n.toString()", method(number_n(), "toString", Vec::new())), + ( + "\"\" + n", + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::String(String::new())), + right: Box::new(number_n()), + }, + ), + ]; + for (label, expr) in spellings { + let ir = ir(vec![param(Type::Number)], returns(expr)); + assert!( + ir.contains(INLINE_FAST), + "{label} must build its text inline:\n{ir}" + ); + } +} + +/// An operand the analysis cannot call a number keeps the plain call: the +/// inline arm is code size spent on every site, so it is only emitted where it +/// is expected to fire. +#[test] +fn an_unproven_operand_keeps_the_runtime_call() { + let ir = ir( + vec![param(Type::Any)], + returns(Expr::StringCoerce(Box::new(Expr::LocalGet(NUM_ID)))), + ); + assert!( + !ir.contains(INLINE_FAST), + "an `any` operand must not be inlined:\n{ir}" + ); + assert!(ir.contains("@js_string_coerce_box(")); +} + +/// `const s = String(n); s.charCodeAt(0)` (and `${n}`, `"" + n`) keeps the +/// string proof its initializer carries, so it takes the inline `charCodeAt`, including the +/// arm that reads a short string's byte out of the value. +/// +/// Sabotage: without these initializers in `proven_type_from_init` the call +/// goes to the generic method site and `cca.sso_fast` is absent; without the +/// SSO arm the inline lowering has no `cca.sso_fast` block. +#[test] +fn a_string_coerced_local_reads_char_codes_inline() { + let concat = Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::String(String::new())), + right: Box::new(number_n()), + }; + let inits = [ + ("String(n)", Expr::StringCoerce(Box::new(number_n()))), + ("`${n}`", Expr::TemplateStringCoerce(Box::new(number_n()))), + ("\"\" + n", concat), + ]; + for (label, init) in inits { + let body = vec![ + Stmt::Let { + id: STR_ID, + name: "s".to_string(), + ty: Type::String, + mutable: false, + init: Some(init), + }, + Stmt::Return(Some(method( + Expr::LocalGet(STR_ID), + "charCodeAt", + vec![Expr::Integer(0)], + ))), + ]; + let ir = ir(vec![param(Type::Number)], body); + assert!( + ir.contains(SSO_CHAR_CODE), + "s = {label}; s.charCodeAt must be inline:\n{ir}" + ); + } +} diff --git a/crates/perry-codegen/src/lower_call/property_get/number_string.rs b/crates/perry-codegen/src/lower_call/property_get/number_string.rs index 3d7e9ce3d5..9caad137a1 100644 --- a/crates/perry-codegen/src/lower_call/property_get/number_string.rs +++ b/crates/perry-codegen/src/lower_call/property_get/number_string.rs @@ -208,10 +208,26 @@ pub(crate) fn try_lower_number_string_methods( }) .unwrap_or(false); if !has_user_to_string { + let numeric = args.is_empty() && crate::type_analysis::is_numeric_expr(ctx, object); let v = lower_expr(ctx, object)?; for a in args { let _ = lower_expr(ctx, a)?; } + // A number receiver's small-integer text is built inline (#10762). + if numeric { + return crate::expr::number_to_string_inline::emit_number_to_string_inline( + ctx, + &v, + |ctx| { + Ok(ctx.block().call( + DOUBLE, + "js_jsvalue_to_string_method_box", + &[(DOUBLE, &v)], + )) + }, + ) + .map(Some); + } let blk = ctx.block(); // #3146: an explicit `.toString()` member call must throw a // TypeError on a nullish receiver, unlike abstract ToString diff --git a/crates/perry-codegen/src/lower_string_concat.rs b/crates/perry-codegen/src/lower_string_concat.rs index 950722a419..fd71b10154 100644 --- a/crates/perry-codegen/src/lower_string_concat.rs +++ b/crates/perry-codegen/src/lower_string_concat.rs @@ -597,6 +597,24 @@ fn coerce_concat_body( &[(DOUBLE, l_box), (DOUBLE, r_box)], )); } + // `"" + n` on a number: the small-integer text is built inline + // (#10762); anything else takes the fused call below. + if matches!(left, Expr::String(prefix) if prefix.is_empty()) + && crate::type_analysis::is_numeric_expr(ctx, right) + { + return crate::expr::number_to_string_inline::emit_number_to_string_inline( + ctx, + r_box, + |ctx| { + let l_handle = str_operand_handle_tag_dispatched(ctx, left, l_box); + Ok(ctx.block().call( + DOUBLE, + "js_string_concat_value_box", + &[(I64, &l_handle), (DOUBLE, r_box)], + )) + }, + ); + } // Literal prefix + proven-small value: the per-site table keeps the // hot key off the runtime entirely (`concat_site_cache.rs`). if let Some(value) = diff --git a/crates/perry-codegen/src/lower_string_method/char_code_at.rs b/crates/perry-codegen/src/lower_string_method/char_code_at.rs index 2e1f6417e6..3c884ef1c0 100644 --- a/crates/perry-codegen/src/lower_string_method/char_code_at.rs +++ b/crates/perry-codegen/src/lower_string_method/char_code_at.rs @@ -25,6 +25,12 @@ use crate::lower_string_concat::str_operand_handle_tag_dispatched; const STRING_HEADER_UTF16_LEN_OFFSET: &str = "0"; const STRING_HEADER_BYTE_LEN_OFFSET: &str = "4"; const STRING_HEADER_SIZE: &str = "20"; +/// Bit offset of an SSO string's length byte; must match +/// `perry-runtime::value::tags::SHORT_STRING_LEN_SHIFT`. +const SHORT_STRING_LEN_SHIFT: &str = "40"; +/// The high bit of each of an SSO string's five payload bytes. Clear in all +/// five means the payload is ASCII. +const SHORT_STRING_HIGH_BITS: &str = "551911719040"; // 0x80_8080_8080 /// Inline `s.charCodeAt(i)` for a heap-tagged, all-ASCII receiver. /// @@ -40,10 +46,11 @@ const STRING_HEADER_SIZE: &str = "20"; /// The guard chain reproduces exactly what `js_string_char_code_at` + /// `js_string_index_to_i32` would compute, and anything it cannot prove /// branches to those same two calls: -/// * `STRING_TAG` receiver — an SSO short string, a lying `string` -/// annotation, or `undefined` all take the slow arm, which still routes -/// through `str_operand_handle_tag_dispatched` (SSO materialization -/// included); +/// * `STRING_TAG` receiver — a lying `string` annotation or `undefined` +/// take the slow arm, which still routes through +/// `str_operand_handle_tag_dispatched`. An all-ASCII SSO receiver reads +/// its byte straight out of the value (#10762); a non-ASCII one takes the +/// slow arm; /// * handle ≥ 4096 — the runtime's `is_valid_string_ptr` magnitude check; /// * `0.0 <= index < 2^31-1` — ORDERED comparisons, so a NaN-boxed index /// (a string, a bool, `undefined`, or a genuine NaN) fails both and takes @@ -90,13 +97,54 @@ pub(super) fn lower_char_code_at_inline( let hdr_idx = ctx.new_block("cca.hdr"); let fast_idx = ctx.new_block("cca.fast"); + let sso_check_idx = ctx.new_block("cca.sso_check"); + let sso_idx = ctx.new_block("cca.sso"); + let sso_fast_idx = ctx.new_block("cca.sso_fast"); let slow_idx = ctx.new_block("cca.slow"); let merge_idx = ctx.new_block("cca.merge"); let hdr_label = ctx.block_label(hdr_idx); let fast_label = ctx.block_label(fast_idx); + let sso_check_label = ctx.block_label(sso_check_idx); + let sso_label = ctx.block_label(sso_idx); + let sso_fast_label = ctx.block_label(sso_fast_idx); let slow_label = ctx.block_label(slow_idx); let merge_label = ctx.block_label(merge_idx); - ctx.block().cond_br(&entry_ok, &hdr_label, &slow_label); + ctx.block().cond_br(&entry_ok, &hdr_label, &sso_check_label); + + // SSO receiver (#10762): `String(n)`, `${n}` and `"" + n` return a short + // number's text as an SSO immediate, and the slow arm below materialized + // a heap copy of it through the intern table (about 175 instructions) to + // read one byte. The bytes are in the value itself. An all-ASCII payload + // has one UTF-16 code unit per byte, so byte `index` is the answer; a + // non-ASCII payload (UTF-8 sequences) keeps the slow arm. + ctx.current_block = sso_check_idx; + let is_sso = ctx + .block() + .icmp_eq(I64, &tag, crate::nanbox::SHORT_STRING_TAG_TOP16_I64); + let sso_ok = ctx.block().and(I1, &is_sso, &idx_ok); + ctx.block().cond_br(&sso_ok, &sso_label, &slow_label); + + // Dominated by the index range test, so `fptosi` is in range. + ctx.current_block = sso_idx; + let sso_idx_i32 = ctx.block().fptosi(DOUBLE, idx_d, I32); + let sso_idx_i64 = ctx.block().zext(I32, &sso_idx_i32, I64); + let sso_len_word = ctx.block().lshr(I64, &bits, SHORT_STRING_LEN_SHIFT); + let sso_len = ctx.block().and(I64, &sso_len_word, "255"); + let sso_in_bounds = ctx.block().icmp_ult(I64, &sso_idx_i64, &sso_len); + let high_bits = ctx.block().and(I64, &bits, SHORT_STRING_HIGH_BITS); + let sso_ascii = ctx.block().icmp_eq(I64, &high_bits, "0"); + let sso_fast_ok = ctx.block().and(I1, &sso_in_bounds, &sso_ascii); + ctx.block() + .cond_br(&sso_fast_ok, &sso_fast_label, &slow_label); + + // `index < len <= 5`, so the shift is at most 32. + ctx.current_block = sso_fast_idx; + let sso_shift = ctx.block().shl(I64, &sso_idx_i64, "3"); + let sso_word = ctx.block().lshr(I64, &bits, &sso_shift); + let sso_byte = ctx.block().and(I64, &sso_word, "255"); + let sso_val = ctx.block().uitofp(I64, &sso_byte, DOUBLE); + let sso_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); // Header block: ASCII + in-range test. Dominated by `handle >= 4096`, so // the two loads are safe; dominated by the index range test, so `fptosi` @@ -150,8 +198,12 @@ pub(super) fn lower_char_code_at_inline( ctx.block().br(&merge_label); ctx.current_block = merge_idx; - Some( - ctx.block() - .phi(DOUBLE, &[(&fast_val, &fast_pred), (&slow_val, &slow_pred)]), - ) + Some(ctx.block().phi( + DOUBLE, + &[ + (&fast_val, &fast_pred), + (&sso_val, &sso_pred), + (&slow_val, &slow_pred), + ], + )) } diff --git a/crates/perry-codegen/src/type_analysis/refine.rs b/crates/perry-codegen/src/type_analysis/refine.rs index 56e0d32189..7d7af1f7a1 100644 --- a/crates/perry-codegen/src/type_analysis/refine.rs +++ b/crates/perry-codegen/src/type_analysis/refine.rs @@ -233,6 +233,26 @@ pub(crate) fn proven_type_from_init(ctx: &FnCtx<'_>, init: &Expr) -> Option { Some(HirType::String) } + // `String(x)` and `${x}` return a string primitive or throw, whatever + // `x` is (#10762). Without this, `const s = String(n)` lost the proof + // its own initializer carries, and `s.charCodeAt(i)` fell to the + // generic method site while `String(n).charCodeAt(i)` took the inline + // string lowering. The value may be SSO, not only a heap string; the + // string lowerings tag-dispatch a proven local for exactly that. + Expr::StringCoerce(_) | Expr::TemplateStringCoerce(_) => Some(HirType::String), + // `+` with a string literal on either side concatenates: a string or a + // throw, whatever the other operand is (`"" + n`, `"k" + i`). Only a + // literal counts; a declared `string` operand is not a proof (#7837). + Expr::Binary { + op: BinaryOp::Add, + left, + right, + } if [left, right] + .iter() + .any(|side| matches!(side.as_ref(), Expr::String(_) | Expr::WtfString(_))) => + { + Some(HirType::String) + } // `Symbol()` identities are system-allocated and never relocated; // `Symbol.for()` identities are process-lifetime `Box` allocations. // Recording the constructor provenance (rather than trusting a diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index b0eb604561..2cce6d83ee 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1421,28 +1421,32 @@ pub(crate) unsafe fn compact_if_holey(map: *mut MapHeader) { /// paths into one body, and the register pressure of those cold paths costs /// every lookup the full prologue/epilogue (eight callee-saved GPRs and four /// FP registers on arm64 — the profile put a third of the function's self -/// time there). The lane here answers the two shapes the numeric side-table -/// exists for — a plain (untagged, non-NaN, non-zero) number key against a -/// small map's entries by bit identity, or against the dense integer range -/// table — and returns `None` for everything else so [`find_key_index_cold`] -/// decides it. A dense-range miss is definitive for its span (every insert, +/// time there). The lane here answers a small map's lookup by bit identity for +/// any key, a plain (untagged, non-NaN, non-zero) number key's miss there, and +/// a plain number against the dense integer range table, and returns `None` +/// for everything else so [`find_key_index_cold`] decides it. A dense-range miss is definitive for its span (every insert, /// delete, clear and GC rewrite keeps the table exact), exactly as in the cold /// path; a key outside the span goes to the hashed index there. #[inline(always)] unsafe fn find_key_index_hot(map: *const MapHeader, key: f64) -> Option { let used = (*map).used; let key_bits = key.to_bits(); - if !is_plain_nonzero_number_bits(key_bits) { - return None; - } if used <= SIDE_TABLE_THRESHOLD { + // A bit-identical entry is THE match for a key of any type: a Map + // never holds two SameValueZero-equal keys, and both sides are already + // normalized (see `find_identical_key`). The common string-keyed shape + // looks a key up with the very value it was inserted with, so it is + // answered here without the out-of-line call (#10697). A miss is + // definitive only for a plain number; any other key may still be + // content-equal to an entry, which the cold path decides. let entries = entries_ptr(map); - for i in 0..used { - if ptr::read(entries.add((i as usize) * 2)).to_bits() == key_bits { - return Some(i as i32); - } + if let Some(i) = find_identical_key(entries, used, key_bits) { + return Some(i); } - return Some(-1); + return is_plain_nonzero_number_bits(key_bits).then_some(-1); + } + if !is_plain_nonzero_number_bits(key_bits) { + return None; } let index = (*map).store.as_ref().map(|store| &store.numeric)?; let dense = index.dense.as_ref()?; @@ -1466,12 +1470,21 @@ pub(crate) unsafe fn find_key_index(map: *const MapHeader, key: f64) -> i32 { find_key_index_cold(map, key) } +#[cfg(test)] +crate::perry_thread_local! { + /// Test-only: lookups [`find_key_index_hot`] handed to the cold path, so a + /// test can assert which lane answered (#10697). + pub(crate) static COLD_LOOKUPS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Every lookup shape [`find_key_index_hot`] declines: tagged, zero and NaN /// keys, string content hashing, the pointer-identity index, the hashed /// numeric index, and the generic linear compare. Out of line on purpose — /// see the hot lane. #[inline(never)] unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { + #[cfg(test)] + COLD_LOOKUPS.with(|n| n.set(n.get() + 1)); let used = (*map).used; let key_bits = key.to_bits(); diff --git a/crates/perry-runtime/src/map/string_key.rs b/crates/perry-runtime/src/map/string_key.rs index 24ba76dbbf..85d35c6728 100644 --- a/crates/perry-runtime/src/map/string_key.rs +++ b/crates/perry-runtime/src/map/string_key.rs @@ -185,6 +185,50 @@ mod tests { f64::from_bits(JSValue::try_short_string(s.as_bytes()).unwrap().bits()) } + /// A small map answers a lookup made with the very key value it stored, + /// SSO or heap, from the inlined hot lane without the out-of-line cold + /// call; a content-equal but distinct heap key, and every miss, still + /// reach the cold path and get the same answers (#10697). + /// + /// Sabotage: restoring the hot lane's plain-number-only admission sends + /// all eight identical lookups to the cold path, and `cold` reads 8. + #[test] + fn a_small_map_answers_an_identical_key_without_the_cold_path() { + let mut map = js_map_alloc(4); + let keys = [ + sso("alpha"), + sso("beta"), + heap("category-gamma"), + heap("category-delta"), + ]; + for (i, &k) in keys.iter().enumerate() { + map = js_map_set(map, k, i as f64); + } + let cold = || super::super::COLD_LOOKUPS.with(std::cell::Cell::get); + let before = cold(); + for _ in 0..2 { + for (i, &k) in keys.iter().enumerate() { + assert_eq!(js_map_get(map, k), i as f64); + } + } + assert_eq!( + cold() - before, + 0, + "identical keys must not leave the hot lane" + ); + + let before = cold(); + assert_eq!(js_map_get(map, heap("category-gamma")), 2.0); + assert_eq!(js_map_get(map, heap("alpha")), 0.0); + assert_eq!(js_map_get(map, sso("gamma")).to_bits(), TAG_UNDEFINED); + assert_eq!(js_map_get(map, 7.5).to_bits(), TAG_UNDEFINED); + assert_eq!( + cold() - before, + 3, + "content matches and non-number misses go cold" + ); + } + /// Every (key, entry) pairing the lane decides agrees with the generic /// comparison it replaces: same content across SSO / heap / distinct heap /// allocations, and every flavour of mismatch. diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index d6baa110a1..14eac0f6f2 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -1895,7 +1895,9 @@ pub(crate) fn define_builtin_data_property( } super::object_ops::define_property_force_store_value(obj, key, value); } else { - js_object_set_field_by_name(obj, key, value); + super::own_override::as_builtin_definition(|| { + js_object_set_field_by_name(obj, key, value); + }); } } set_builtin_property_attrs(obj as usize, name, attrs); diff --git a/crates/perry-runtime/src/object/exotic_expando.rs b/crates/perry-runtime/src/object/exotic_expando.rs index 309ddcaaea..60732983ca 100644 --- a/crates/perry-runtime/src/object/exotic_expando.rs +++ b/crates/perry-runtime/src/object/exotic_expando.rs @@ -209,7 +209,7 @@ pub(crate) fn value_store(kind: ExoticKind, addr: usize, key: &str, bits: u64) { // that gauntlet, so the guard's predicate answered "no own override" and // the builtin still won. Arming at the store itself is the funnel the // gauntlet was chosen to approximate. - crate::object::own_override::note_exotic_named_prop_install(); + crate::object::own_override::note_exotic_named_prop_install(addr); match kind { ExoticKind::Error => { crate::node_submodules::set_error_user_prop(addr, key, f64::from_bits(bits)) diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index cc40b799b0..e15a773e73 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -327,7 +327,7 @@ pub extern "C" fn js_object_set_field_by_name( // it over-approximates, and the only cost of that is the guard's slow // side. Named keys only: an index write is not a method shadow. if !key.is_null() { - crate::object::own_override::note_exotic_named_prop_install(); + crate::object::own_override::note_exotic_named_prop_install(obj as usize); } // A Buffer is an ordinary object in Node (a Uint8Array), so `buf.foo = v` // stores an own property — and an own key SHADOWS the same-named prototype diff --git a/crates/perry-runtime/src/object/global_this/populate.rs b/crates/perry-runtime/src/object/global_this/populate.rs index 7c5a7b3689..bbc1cb1ec3 100644 --- a/crates/perry-runtime/src/object/global_this/populate.rs +++ b/crates/perry-runtime/src/object/global_this/populate.rs @@ -11,6 +11,14 @@ use super::*; /// `var arrayProto = Array.prototype` chained read inside /// `runInContext`. pub(crate) fn populate_global_this_builtins(singleton_at_entry: *mut ObjectHeader) { + // Every install below is a builtin definition, which arms the own-override + // guard only on a Map, Set or Date owner (#10697). + super::super::own_override::as_builtin_definition(|| { + populate_global_this_builtins_inner(singleton_at_entry) + }) +} + +fn populate_global_this_builtins_inner(singleton_at_entry: *mut ObjectHeader) { if singleton_at_entry.is_null() { return; } diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 721b3bf24c..5096353482 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -211,6 +211,8 @@ mod object_literal_ops; pub(crate) mod object_ops; pub(crate) mod own_override; #[cfg(test)] +mod own_override_builtin_install_tests; +#[cfg(test)] mod own_override_push_tests; pub(crate) use object_ops::{ensure_key_in_keys_array, install_builtin_getter}; mod object_ops_frozen; diff --git a/crates/perry-runtime/src/object/own_override.rs b/crates/perry-runtime/src/object/own_override.rs index e4d63b9c6b..df71f5f13f 100644 --- a/crates/perry-runtime/src/object/own_override.rs +++ b/crates/perry-runtime/src/object/own_override.rs @@ -55,7 +55,8 @@ use std::sync::atomic::Ordering; -/// Has any non-`ObjectHeader` cell ever taken a named property? +/// Has any non-`ObjectHeader` cell ever taken a named property that could +/// shadow a builtin on a guarded receiver? /// /// Set-only. See the module docs for why it is armed early and never cleared. /// Exported so EMITTED CODE can test it inline. The guard's common case is @@ -66,15 +67,107 @@ use std::sync::atomic::Ordering; /// emitted guard reads with one monotonic load and a not-taken branch, with /// the call left behind it for the case that is almost never taken. /// +/// The runtime's own builtin definitions do not arm it unless their owner is +/// a Map, Set or Date cell (#10697; see [`as_builtin_definition`]). [`OWN_NAMED_PROP_EVER`] +/// is armed by every install, builtin or not, for the dispatcher. +/// /// A `u32` rather than a bool so the emitted load matches the barrier gate's /// alignment and width; only zero / non-zero is meaningful. #[no_mangle] pub static PERRY_OWN_NAMED_PROP_INSTALLED: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); -/// Armed from the top of `field_set_by_name`'s exotic-store gauntlet. +/// Has any non-`ObjectHeader` cell ever taken a named property at all, +/// including the runtime's own builtin definitions? Set-only, like +/// [`PERRY_OWN_NAMED_PROP_INSTALLED`], and armed wherever that flag was armed +/// before #10697, so [`own_user_method_value`] answers exactly as it did. +static OWN_NAMED_PROP_EVER: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + +crate::perry_thread_local! { + /// Set while the runtime runs its own builtin definitions (#10697). A + /// flag, never an address. + static BUILTIN_INTRINSIC_INSTALL: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +#[cfg(test)] +crate::perry_thread_local! { + /// Test-only: installs on this thread that armed the guard's flag. The + /// flag itself is process-global and set-only, so another test has + /// usually armed it already and it cannot be observed changing (#10697). + pub(crate) static ARMS_NOTED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Run `install`, one of the runtime's own builtin definitions, so that the +/// named-property installs it makes arm [`PERRY_OWN_NAMED_PROP_INSTALLED`] +/// only when their owner is a Map, Set or Date cell (#10697). +/// +/// Populating `globalThis` installs statics on constructor intrinsics such as +/// `%TypedArray%` (closure cells), `constructor` on `Array.prototype` (an +/// array cell), and aliases such as `Number.parseFloat`. Each of those stores +/// passes the exotic gauntlet that arms the flag. One lazy population anywhere +/// in a program then sent every guarded `Map`/`Set`/`Date` builtin call +/// through the authoritative `hasOwn` predicate: about 600 instructions on +/// each `m.get(k)`, 4.2x node in #10697's count-by-category shape. +/// +/// The flag answers for the emitted guard and its predicate only, and those +/// protect proven Map, Set, Array and Date receivers. An array answers from +/// its own header and named-property storage, never from this flag, and a +/// declared-`Map` receiver of any other kind is brand-checked into generic +/// dispatch. So only an install onto a Map, Set or Date cell can matter to +/// it, and [`note_exotic_named_prop_install`] still arms for those, and for an +/// owner whose header cannot be read, inside this scope too. A builtin +/// definition installs the builtin itself in any case, never a user override +/// of it. [`OWN_NAMED_PROP_EVER`] is armed exactly as before. +/// +/// Thread-local rather than save-and-restore of the global flag, so an arm by +/// another thread in the same window is never undone. Nests. +pub(crate) fn as_builtin_definition(install: impl FnOnce() -> R) -> R { + let outer = BUILTIN_INTRINSIC_INSTALL.with(|flag| flag.replace(true)); + // Restored on unwind too: a leaked `true` would stop user installs from + // arming, which is the silent wrong value the flag exists to prevent. + struct Restore(bool); + impl Drop for Restore { + fn drop(&mut self) { + BUILTIN_INTRINSIC_INSTALL.with(|flag| flag.set(self.0)); + } + } + let _restore = Restore(outer); + install() +} + +/// Can a named property on `owner` shadow a builtin that the emitted guard +/// answers from [`PERRY_OWN_NAMED_PROP_INSTALLED`]? `owner` is an address or +/// NaN-boxed pointer bits. Unreadable owners answer yes. +fn owner_is_flag_guarded(owner: usize) -> bool { + let bits = owner as u64; + let addr = if bits >> 48 == 0x7FFD { + (bits & crate::value::POINTER_MASK) as usize + } else { + owner + }; + // SAFETY: `try_read_gc_header` validates the address before reading. + match unsafe { crate::value::addr_class::try_read_gc_header(addr) } { + Some(header) => matches!( + header.obj_type, + crate::gc::GC_TYPE_MAP | crate::gc::GC_TYPE_SET | crate::gc::GC_TYPE_DATE_CELL + ), + None => true, + } +} + +/// Armed where a non-ordinary `owner` takes a named property: the top of +/// `field_set_by_name`'s exotic-store gauntlet and the exotic expando store. #[inline] -pub(crate) fn note_exotic_named_prop_install() { +pub(crate) fn note_exotic_named_prop_install(owner: usize) { + if !OWN_NAMED_PROP_EVER.load(Ordering::Relaxed) { + OWN_NAMED_PROP_EVER.store(true, Ordering::Relaxed); + } + if BUILTIN_INTRINSIC_INSTALL.with(std::cell::Cell::get) && !owner_is_flag_guarded(owner) { + return; + } + #[cfg(test)] + ARMS_NOTED.with(|n| n.set(n.get() + 1)); // Relaxed is enough: a stale `false` can only be read by a thread that has // not yet observed the store, and that thread's own receivers cannot be // the one just written (the write happens-before any publication of the @@ -258,9 +351,11 @@ unsafe fn authoritative_has_own_key(recv: f64, key: *mut crate::StringHeader) -> /// # Safety /// `recv` is any NaN-boxed value; `name` is this call's method name. pub(crate) unsafe fn own_user_method_value(recv: f64, name: &str) -> Option { - // The same relaxed arm the emitted guard consults: nothing anywhere has - // ever put a named property on a non-object cell, so nothing can shadow. - if PERRY_OWN_NAMED_PROP_INSTALLED.load(Ordering::Relaxed) == 0 { + // Nothing anywhere has ever put a named property on a non-object cell, so + // nothing can shadow. The flag the runtime's own builtin installs also arm + // (#10697), so this reads exactly as it did before they stopped arming the + // emitted guard's. + if !OWN_NAMED_PROP_EVER.load(Ordering::Relaxed) { return None; } resolve_own_user_method(recv, name) diff --git a/crates/perry-runtime/src/object/own_override_builtin_install_tests.rs b/crates/perry-runtime/src/object/own_override_builtin_install_tests.rs new file mode 100644 index 0000000000..d8ba9a9610 --- /dev/null +++ b/crates/perry-runtime/src/object/own_override_builtin_install_tests.rs @@ -0,0 +1,85 @@ +//! #10697: the runtime's own builtin installs do not arm the own-override +//! guard unless their owner is a Map, Set or Date cell; user installs do. + +use super::own_override::ARMS_NOTED; +use crate::object::descriptor_state::{define_builtin_data_property, PropertyAttrs}; + +extern "C" fn intrinsic_fixture( + _closure: *const crate::closure::ClosureHeader, + _this: crate::closure::JsThis, +) -> f64 { + 0.0 +} + +static INTRINSIC_FIXTURE: crate::closure::JsFunctionInfo = crate::closure::JsFunctionInfo::of( + intrinsic_fixture as crate::codegen_abi::JsBody0, +); + +fn key(name: &str) -> *const crate::StringHeader { + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32) +} + +fn arms() -> usize { + ARMS_NOTED.with(std::cell::Cell::get) +} + +/// Sabotage: installing through `js_object_set_field_by_name` directly, as +/// `define_builtin_data_property` did, makes `arms` read 1. +#[test] +fn a_builtin_install_on_an_intrinsic_does_not_arm_the_guard() { + let _lock = crate::gc::global_side_table_test_lock(); + let ctor = crate::closure::js_closure_alloc(&INTRINSIC_FIXTURE, 0); + let before = arms(); + define_builtin_data_property( + ctor as *mut crate::object::ObjectHeader, + key("from"), + 1.0, + "from".to_string(), + PropertyAttrs::new(true, false, true), + ); + assert_eq!( + arms() - before, + 0, + "a builtin static on a constructor must not arm" + ); + // `Array.prototype` is an array cell and takes `constructor` this way. + let proto = crate::array::js_array_alloc(0); + let before = arms(); + define_builtin_data_property( + proto as *mut crate::object::ObjectHeader, + key("constructor"), + 1.0, + "constructor".to_string(), + PropertyAttrs::new(true, false, true), + ); + assert_eq!( + arms() - before, + 0, + "`constructor` on an array prototype must not arm" + ); + // A Map owner is a kind the guard answers from the flag: still armed. + let map = crate::map::js_map_alloc(0); + let before = arms(); + define_builtin_data_property( + map as *mut crate::object::ObjectHeader, + key("size2"), + 1.0, + "size2".to_string(), + PropertyAttrs::new(true, false, true), + ); + assert!( + arms() > before, + "a builtin install onto a Map must still arm" + ); + // The scope must not outlive the install. + let before = arms(); + crate::object::js_object_set_field_by_name( + map as *mut crate::object::ObjectHeader, + key("get"), + 1.0, + ); + assert!( + arms() > before, + "an own `get` on a Map must still arm the guard" + ); +} diff --git a/crates/perry-stdlib/src/crypto/random.rs b/crates/perry-stdlib/src/crypto/random.rs index d4d788d2a6..6a95916d5f 100644 --- a/crates/perry-stdlib/src/crypto/random.rs +++ b/crates/perry-stdlib/src/crypto/random.rs @@ -134,8 +134,10 @@ pub unsafe extern "C" fn js_crypto_random_uuid(options_bits: f64) -> *mut String } else { perry_uuid::v4() }; - let uuid_str = uuid.as_str(); - js_string_from_bytes(uuid_str.as_ptr(), uuid_str.len() as u32) + // The bytes directly: `as_str` would re-validate 36 known-ASCII bytes as + // UTF-8 on every UUID only for the string constructor to copy them. + let uuid_bytes = uuid.as_bytes(); + js_string_from_bytes(uuid_bytes.as_ptr(), uuid_bytes.len() as u32) } /// Generate an RFC 9562 version 7 UUID — a 48-bit millisecond Unix @@ -147,8 +149,8 @@ pub unsafe extern "C" fn js_crypto_random_uuid(options_bits: f64) -> *mut String #[no_mangle] pub extern "C" fn js_crypto_random_uuidv7() -> *mut StringHeader { let uuid = perry_uuid::v7(); - let uuid_str = uuid.as_str(); - js_string_from_bytes(uuid_str.as_ptr(), uuid_str.len() as u32) + let uuid_bytes = uuid.as_bytes(); + js_string_from_bytes(uuid_bytes.as_ptr(), uuid_bytes.len() as u32) } /// Validates `randomUUID`'s options bag and returns `disableEntropyCache`. diff --git a/crates/perry-uuid/src/lib.rs b/crates/perry-uuid/src/lib.rs index 7d2e639c00..2d7a06bbaa 100644 --- a/crates/perry-uuid/src/lib.rs +++ b/crates/perry-uuid/src/lib.rs @@ -66,15 +66,15 @@ pub struct Hyphenated([u8; 36]); impl Hyphenated { fn new(bytes: [u8; 16]) -> Self { const HEX: &[u8; 16] = b"0123456789abcdef"; + // Where each byte's two digits start, hyphens skipped. A constant + // table instead of a running index with a per-byte hyphen test: the + // loop unrolls to straight-line stores with no bounds checks, which was + // a third of `randomUUID()`'s instructions (#10523). + const AT: [usize; 16] = [0, 2, 4, 6, 9, 11, 14, 16, 19, 21, 24, 26, 28, 30, 32, 34]; let mut out = [b'-'; 36]; - let mut at = 0; - for (i, b) in bytes.into_iter().enumerate() { - if matches!(i, 4 | 6 | 8 | 10) { - at += 1; - } - out[at] = HEX[(b >> 4) as usize]; - out[at + 1] = HEX[(b & 15) as usize]; - at += 2; + for i in 0..16 { + out[AT[i]] = HEX[(bytes[i] >> 4) as usize]; + out[AT[i] + 1] = HEX[(bytes[i] & 15) as usize]; } Hyphenated(out) } @@ -82,6 +82,11 @@ impl Hyphenated { // Only ASCII hex digits and hyphens are ever written. std::str::from_utf8(&self.0).unwrap() } + /// The 36 ASCII bytes, without `as_str`'s UTF-8 validation, for a caller + /// that copies them into a string of its own. + pub fn as_bytes(&self) -> &[u8; 36] { + &self.0 + } } #[cfg(any(feature = "v4", feature = "v7"))] impl std::fmt::Display for Hyphenated { @@ -173,6 +178,19 @@ mod tests { } } } + // #10523: the table-driven formatter writes every byte's digits in RFC + // 9562 order around the four hyphens; `as_bytes` is `as_str`'s bytes. + #[cfg(any(feature = "v4", feature = "v7"))] + #[test] + fn hyphenated_layout_is_exact() { + let bytes: [u8; 16] = std::array::from_fn(|i| (i as u8) * 17); + let h = super::Hyphenated::new(bytes); + assert_eq!(h.as_str(), "00112233-4455-6677-8899-aabbccddeeff"); + assert_eq!(h.as_bytes(), h.as_str().as_bytes()); + let h = + super::Hyphenated::new([0xf0, 0x0f, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 0xff]); + assert_eq!(h.as_str(), "f00f0102-0304-0506-0708-090a0b0c0dff"); + } #[cfg(feature = "v4")] #[test] fn v4_layout_and_fresh_entropy() { diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index b783333ef9..3e4483498c 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -2742,6 +2742,24 @@ "name": "WINDOW_ROOTS", "verdict": "not_a_gc_pointer", "why": "Window-root registry maps numeric window handles to numeric root-widget handles; neither value is a JavaScript heap pointer." + }, + { + "file": "crates/perry-runtime/src/map.rs", + "name": "COLD_LOOKUPS", + "verdict": "test_only", + "why": "#[cfg(test)] Cell counting Map lookups the hot lane handed to find_key_index_cold, so a test can assert which lane answered (#10697). A count, never a pointer; absent from shipped binaries." + }, + { + "file": "crates/perry-runtime/src/object/own_override.rs", + "name": "ARMS_NOTED", + "verdict": "test_only", + "why": "#[cfg(test)] Cell counting installs that armed the own-override guard flag on this thread (#10697), because the process-global flag itself is set-only and usually already armed by another test. A count, never a pointer; absent from shipped binaries." + }, + { + "file": "crates/perry-runtime/src/object/own_override.rs", + "name": "BUILTIN_INTRINSIC_INSTALL", + "verdict": "not_a_gc_pointer", + "why": "Cell set while the runtime runs its own builtin definitions (as_builtin_definition, #10697), so installs onto non-Map/Set/Date owners do not arm PERRY_OWN_NAMED_PROP_INSTALLED. A flag, never an address." } ], "_FRONTIER_README": "Identity-pinned debt ratchet over new perry-ui* candidates and otherwise-unclassified core raw/Perry TLS declarations (see the census docstring, “The identity-pinned frontier”). A new uncovered holder fails until it is scanned, receives a researched holders verdict, or is deliberately pinned as debt. Moving a researched false positive to holders graduates it from this list. A fixed or classified holder makes its old frontier pin stale, so the receipt must be deleted.", diff --git a/test-files/test_gap_10697_own_override_after_global_population.ts b/test-files/test_gap_10697_own_override_after_global_population.ts new file mode 100644 index 0000000000..45977cccd7 --- /dev/null +++ b/test-files/test_gap_10697_own_override_after_global_population.ts @@ -0,0 +1,40 @@ +// #10697: populating globalThis installs the runtime's builtins onto its +// intrinsics. Those installs no longer arm the own-override guard that proven +// Map/Set/Date/Array builtin calls consult, so a program that touches a lazy +// global keeps the direct builtin call. A user's own override must still win +// afterwards, and the populated builtins must still be callable. + +// Force the lazy population first (an intrinsic, an alias, a global function). +const g: any = globalThis; +console.log(typeof g.Uint8Array, typeof g.unescape, Number.parseFloat === g.parseFloat); +console.log(Uint8Array.from([1, 2, 3]).join(","), Number.parseFloat("2.5"), g.unescape("%41")); +console.log(Array.prototype.constructor === Array, [].constructor === Array); + +// Plain builtin calls on proven receivers, after population. +const m = new Map(); +const cats = ["alpha", "beta", "gamma", "delta"]; +for (let i = 0; i < 40; i++) m.set(cats[i & 3], (m.get(cats[i & 3]) || 0) + 1); +console.log([...m].join(";"), m.has("beta"), m.get("delta")); +const s = new Set([1, 2, 3]); +console.log(s.has(2), s.size); +const d = new Date(0); +console.log(d.getTime(), d.toISOString()); +const a = [3, 1, 2]; +console.log(a.indexOf(1), a.includes(3), a.slice(1).join(",")); + +// Own overrides installed AFTER population must still beat the builtin. +const m2 = new Map([["k", 1]]); +(m2 as any).get = (k: string) => "own-get:" + k; +console.log(m2.get("k")); +const s2 = new Set([1]); +(s2 as any).has = (v: number) => "own-has:" + v; +console.log(s2.has(1)); +const d2 = new Date(0); +(d2 as any).getTime = () => "own-getTime"; +console.log(d2.getTime()); +const a2 = [1, 2, 3]; +(a2 as any).indexOf = (v: number) => "own-indexOf:" + v; +console.log(a2.indexOf(2)); + +// And a Map created before the override still uses the builtin. +console.log(m.get("alpha"), m.get("missing")); diff --git a/test-files/test_gap_10762_inline_number_to_string.ts b/test-files/test_gap_10762_inline_number_to_string.ts new file mode 100644 index 0000000000..e9fdf058e1 --- /dev/null +++ b/test-files/test_gap_10762_inline_number_to_string.ts @@ -0,0 +1,75 @@ +// #10762: `String(n)`, `${n}`, `n.toString()` and `"" + n` build a number +// operand's small-integer text inline at the call site, and `s.charCodeAt(i)` +// reads an ASCII short string's byte straight out of the value. Both have a +// runtime fallback; this pins the boundary between the two arms and the +// fallbacks themselves. + +function spell(n: number): string { + const a = String(n); + const b = `${n}`; + const c = n.toString(); + const d = "" + n; + const agree = a === b && b === c && c === d; + return agree ? a + "|" + a.length : "DISAGREE " + [a, b, c, d].join(","); +} + +// Around every edge of the inline range (-9999..=99999) and each digit count. +const edges: number[] = [ + 0, -0, 1, -1, 9, 10, -9, -10, 99, 100, -99, -100, 999, 1000, -999, -1000, + 9999, 10000, -9999, -10000, 99999, 100000, -99999, 99998.5, -9998.5, + 0.5, -0.5, 1e-7, 1e21, NaN, Infinity, -Infinity, 2147483648, -2147483649, +]; +for (const n of edges) console.log(n, spell(n)); + +// Every integer the inline arm covers, checked against a digit-by-digit +// reference; only a count and the first mismatch are printed. +function reference(n: number): string { + if (n === 0) return "0"; + let rest = Math.abs(n); + let out = ""; + while (rest > 0) { + out = String.fromCharCode(48 + (rest % 10)) + out; + rest = Math.floor(rest / 10); + } + return n < 0 ? "-" + out : out; +} +let checked = 0; +let firstBad = ""; +for (let n = -9999; n <= 99999; n++) { + const got = String(n); + if (got !== reference(n) || `${n}` !== got || n.toString() !== got || "" + n !== got) { + if (firstBad === "") firstBad = n + " -> " + got; + } + checked++; +} +console.log("checked", checked, firstBad === "" ? "all match" : "first mismatch " + firstBad); + +// A `number` annotation is not enforced at run time: the inline arm must +// decline anything that is not a plain double and take the full conversion. +const liars: any[] = ["abc", "12", true, null, undefined, 12n, [1, 2], { a: 1 }]; +for (const v of liars) { + const n: number = v; + console.log(String(n), `${n}`, "" + n); +} +// (`"" + valueOfOnly` is left out: it prints "seven" instead of "7" on main +// as well, a separate ToPrimitive-hint bug in the declared-number concat.) +const valueOfOnly: number = { valueOf: () => 7, toString: () => "seven" } as any; +console.log(String(valueOfOnly), `${valueOfOnly}`); + +// Integer-valued results from arithmetic, loop counters and Int32 locals. +let acc = 0; +for (let i = -3; i < 4; i++) acc += String(i * 7).length + `${i | 0}`.length; +console.log(acc, String(2 ** 16), `${(3 * 33333) | 0}`, (65535 & 0xffff).toString()); + +// charCodeAt on short strings: numbers, ASCII words, non-ASCII, bad indexes. +const shorts = [String(42), `${-7}`, "" + 12345, "abcde", "é", "aé", "€", ""]; +for (const s of shorts) { + const codes: (number | string)[] = []; + for (let i = -1; i <= s.length; i++) { + const c = s.charCodeAt(i); + codes.push(Number.isNaN(c) ? "NaN" : c); + } + console.log(JSON.stringify(s), codes.join(","), s.charCodeAt(0.9), s.charCodeAt(1.5)); +} +const idxLiar: number = "1" as any; +console.log(String(123).charCodeAt(idxLiar), String(123).charCodeAt(NaN));