diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9febbaaf67..d8cbb74102 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3491,15 +3491,9 @@ jobs: # Proper fix: hoist well-known-binding lookup out of # build_optimized_libs into link.rs so it runs even when # auto-optimize is skipped. Tracked separately. - # #8475: the three fastify-importing snippets cannot run under the - # harness's PERRY_NO_AUTO_OPTIMIZE=1 speed flag — `perry compile` - # hard-errors on `import 'fastify'` there, because the prebuilt - # stdlib is not built with `external-fastify-pump` and the request - # loop would hang. Excluded on the same terms as the HTTP aggregate - # above until the harness can run this class in a second, - # auto-optimizing pass. COVERAGE GAP, tracked in #8475 — not a - # statement that these snippets work. - cmd_exclude_gallery: "./scripts/run_doc_tests.sh --verbose --skip-xcompile --filter-exclude ui/gallery.ts --filter-exclude stdlib/http/snippets.ts --filter-exclude getting-started/npm_packages.ts --filter-exclude stdlib/http/fastify_json.ts --filter-exclude stdlib/overview/snippets.ts" + # Fastify examples declare `requires: auto-optimize`; the harness + # builds their specialized libraries and counts them in this run. + cmd_exclude_gallery: "./scripts/run_doc_tests.sh --verbose --skip-xcompile --filter-exclude ui/gallery.ts --filter-exclude stdlib/http/snippets.ts" cmd_gallery: "./scripts/run_doc_tests.sh --verbose --skip-xcompile --filter ui/gallery.ts" # Repeat `--xcompile-only-target=…` per target rather than a # single comma-delimited value because PowerShell splits even @@ -3542,9 +3536,7 @@ jobs: shell: pwsh # The well-known HTTP aggregate remains excluded on every host # while its no-auto ext-archive routing issue is tracked. - # #8475: same fastify / PERRY_NO_AUTO_OPTIMIZE exclusion as the - # macOS entry above. COVERAGE GAP, tracked there. - cmd_exclude_gallery: "./scripts/run_doc_tests.ps1 --verbose --skip-xcompile --filter-exclude ui/gallery.ts --filter-exclude stdlib/http/snippets.ts --filter-exclude getting-started/npm_packages.ts --filter-exclude stdlib/http/fastify_json.ts --filter-exclude stdlib/overview/snippets.ts" + cmd_exclude_gallery: "./scripts/run_doc_tests.ps1 --verbose --skip-xcompile --filter-exclude ui/gallery.ts --filter-exclude stdlib/http/snippets.ts" cmd_gallery: "./scripts/run_doc_tests.ps1 --verbose --skip-xcompile --filter ui/gallery.ts" cmd_xcompile_blocking: "./scripts/run_doc_tests.ps1 --verbose --xcompile-only --xcompile-only-target=web --xcompile-only-target=wasm" cmd_xcompile_advisory: "./scripts/run_doc_tests.ps1 --verbose --xcompile-only" diff --git a/changelog.d/9808-element-shape-transfer-gate.md b/changelog.d/9808-element-shape-transfer-gate.md new file mode 100644 index 0000000000..b7d3b19b3f --- /dev/null +++ b/changelog.d/9808-element-shape-transfer-gate.md @@ -0,0 +1,25 @@ +**A relocated array whose element-shape proof does not exist no longer takes +the side table to find that out** (#9792). + +`transfer_element_shape` runs from `layout_transfer` for every relocated +array — growth forwarding, a copying minor, an old-gen defrag. It already +computes `had_bit` for free from two header words it has read anyway, and +then took `ELEMENT_SHAPES`' `RefCell` and hashed both addresses regardless, +for two removes that on the overwhelmingly common path remove nothing. It now +returns when neither address advertises a proof. + +The gate has exactly one safe shape and both halves are pinned by +`a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof`. +Skipping on `!had_bit` alone would be wrong: a destination that still +advertises a proof describes storage the move has just replaced, so that case +keeps the full fail-closed path. + +What skipping leaves behind is a record at an address whose bit is clear, and +that state was already part of the design rather than new: the bit is the sole +authority for a read (`element_shape_proof` returns `None` before touching the +table, and `note_element_store` is gated on the same bit), and `establish` +draws every identity from `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever +record sits at the address — precisely so a survivor cannot donate its epoch +to the next array proven there. `prune_dead_element_shape_owners` drops it on +the next collection, the same footprint-only guarantee a fail-closed transfer +already relied on. The test asserts both defences directly. diff --git a/changelog.d/9818-primitive-string-property-reads.md b/changelog.d/9818-primitive-string-property-reads.md new file mode 100644 index 0000000000..9e2a86d644 --- /dev/null +++ b/changelog.d/9818-primitive-string-property-reads.md @@ -0,0 +1,10 @@ +Fix primitive string property reads with computed keys. Named properties now +consult `String.prototype` and preserve the original method value, so reflective +read-then-call code and method identity checks work. Inherited accessors receive +the primitive string as `this`; object and symbol keys follow `ToPropertyKey`. +Character indices and `length` keep precedence over prototype properties, and +boxed strings retain their own-property lookup before their custom prototype. + +Cover typed and untyped receivers, short strings, borrowed methods, inherited +accessors, symbol keys, key coercion, and prototype mutation in runtime unit +tests and a Node parity fixture. Direct method-call lowering is unchanged. diff --git a/changelog.d/9819-regex-flags-no-alloc.md b/changelog.d/9819-regex-flags-no-alloc.md new file mode 100644 index 0000000000..431d87e484 --- /dev/null +++ b/changelog.d/9819-regex-flags-no-alloc.md @@ -0,0 +1,25 @@ +### Performance + +- **Constructing a `RegExp` no longer allocates its flags string.** A JS regex + literal evaluates to a fresh `RegExp` object every time it is reached, and + `js_regexp_new` materialized the canonical flags twice per construction: once + as a Rust `String` from `validate_and_canonicalize_flags`, and once as a fresh + GC `StringHeader` for `flags_ptr`. On the claude-code TUI that is **161,897 + constructions per 400-character reply** (`PERRY_REGEX_DIAG`) — ~5.2 MB of + identical one- and two-byte GC strings per reply, ~44 MB on a 3300-character + one, and ~1.4 million allocations. + + Neither copy is needed. There are eight legal flags, each may appear once, so + the canonical form is at most eight ASCII bytes and now lives inline in a + `CanonicalFlags` value instead of on the heap. And JS strings are immutable + with no identity semantics, so when the caller's flags text already IS the + canonical text — which it is for a literal, whose flags the author wrote in + spec order — the header shares the caller's string rather than duplicating + it. Only a non-canonical spelling (`/x/ig` → `"gi"`) or a computed + `new RegExp(p, f)` still materializes one; the new `flags_alloc` counter in + `PERRY_REGEX_DIAG` reports how often that happens. + + This is a **below-the-line** allocation fix by the campaign's own ~10 % rule: + at ~2-3 % of arena traffic per turn it cannot change the collection schedule, + and the cc rig is expected to read flat. It is worth doing because the + allocation is pure waste, not because it moves a benchmark. diff --git a/changelog.d/9820-fastify-doc-test-coverage.md b/changelog.d/9820-fastify-doc-test-coverage.md new file mode 100644 index 0000000000..4ba08e5f82 --- /dev/null +++ b/changelog.d/9820-fastify-doc-test-coverage.md @@ -0,0 +1,6 @@ +Restore the three Fastify documentation examples to the host doc-test CI run. +The examples declare `requires: auto-optimize` in their banners, which lets the +harness rebuild their specialized runtime libraries while ordinary examples +continue using prebuilt archives. Required examples participate in the normal +pass/fail report; compiler failures remain gate failures. Their existing +compile-only setting avoids starting servers or connecting to external services. diff --git a/changelog.d/9822-retained-growth-verifier.md b/changelog.d/9822-retained-growth-verifier.md new file mode 100644 index 0000000000..f67793ceb8 --- /dev/null +++ b/changelog.d/9822-retained-growth-verifier.md @@ -0,0 +1 @@ +- Fix a false evacuation-verifier abort when a copying minor encounters a retained, non-moving array-growth alias, such as Solid's effect dependency array. Verification still follows the full forwarding chain and rejects nursery evacuation originals; old-page evacuation retains its strict checks. diff --git a/changelog.d/9823-for-in-deferred-shadow-set.md b/changelog.d/9823-for-in-deferred-shadow-set.md new file mode 100644 index 0000000000..81cb3cd4ac --- /dev/null +++ b/changelog.d/9823-for-in-deferred-shadow-set.md @@ -0,0 +1,28 @@ +**`for-in` no longer allocates a heap string and a hash entry for every own +name at every prototype level** (#9823). + +`js_for_in_keys_value` kept a `HashSet` of every own name — enumerable +or not — at every level of the prototype chain, so that a name owned closer to +the receiver hides the same name further along it (ECMA-262 14.7.5, 12.6.4-2). +It built that set unconditionally, which meant materialising a second key array +per level (all own names, on top of the enumerable ones) and turning every name +at every level into an owned `String` purely so it could be hashed. + +That set can only filter a level at or below the first prototype, and a level +that contributes no enumerable keys of its own never consults it. It is now +built on demand — at the moment a prototype level actually has an enumerable +key to filter — from exactly the levels already walked, so the emitted key +sequence is unchanged. + +On the compiled claude-code TUI, one 400-character reply: **159,947 `String` +allocations and 159,947 hash inserts become zero**, and the key arrays +materialised per call halve from 4.00 to 2.00. Across 17,281 `for-in` loops in +that reply, **no key was emitted from a prototype level at all**, so the set +that cost all of that filtered nothing. The strings totalled 1.91 MB, which is +why an allocation-byte ranking never surfaced this: the cost was 160,000 +mallocs, memcpys, hashes and frees, not the bytes they held. The collection +schedule is unchanged (41 vs 43 copying minors, 46 vs 48 budgeted full-cycle +steps). + +`PERRY_ENUM_DIAG=` reports the counters above. `PERRY_FORIN_LAZY_SHADOW=0` +restores the eager set. diff --git a/crates/perry-doc-tests/src/main.rs b/crates/perry-doc-tests/src/main.rs index adc3837802..94f1ac009b 100644 --- a/crates/perry-doc-tests/src/main.rs +++ b/crates/perry-doc-tests/src/main.rs @@ -15,6 +15,8 @@ use serde::Serialize; mod image_diff; mod lint; +#[cfg(test)] +mod tests; #[derive(Parser, Debug)] #[command(name = "doc-tests", about = "Perry documentation-example test harness")] @@ -357,6 +359,7 @@ struct Example { platforms: BTreeSet, targets: BTreeSet, compile_only: bool, + requires_auto_optimize: bool, widget_bundle_id: Option, } @@ -402,6 +405,7 @@ fn discover_examples(root: &Path) -> Result> { platforms: banner.platforms, targets: banner.targets, compile_only: banner.compile_only, + requires_auto_optimize: banner.requires_auto_optimize, widget_bundle_id: banner.widget_bundle_id, }); } @@ -419,6 +423,7 @@ struct Banner { /// single-program timeout. Catches API/TS drift without the /// integration-test overhead. compile_only: bool, + requires_auto_optimize: bool, /// Required for any `*-widget` / `wearos-tile` target — passed as /// `--app-bundle-id` on the perry compile invocation. widget_bundle_id: Option, @@ -455,6 +460,18 @@ fn read_banner(path: &Path) -> Result { if v.eq_ignore_ascii_case("false") || v == "0" || v.eq_ignore_ascii_case("no") { b.compile_only = true; } + } else if let Some(rest) = body.strip_prefix("requires:") { + for requirement in rest.split(',').map(str::trim) { + match requirement { + "auto-optimize" => b.requires_auto_optimize = true, + _ => { + return Err(anyhow!( + "{}: unknown doc-example requirement `{requirement}`", + path.display() + )) + } + } + } } else if let Some(rest) = body.strip_prefix("widget-bundle-id:") { let v = rest.trim(); if !v.is_empty() { @@ -494,7 +511,7 @@ fn run_one( }); if !no_compile { - if let Err(e) = compile(perry_bin, &ex.path, &bin_path) { + if let Err(e) = compile(perry_bin, ex, &bin_path) { return ExampleReport { file: rel.to_string(), kind: ex.kind, @@ -733,6 +750,7 @@ fn cross_compile_one( }; let mut cmd = Command::new(perry_bin); + configure_compile_environment(&mut cmd, ex); cmd.arg("compile") .arg(&ex.path) .arg("--target") @@ -807,13 +825,25 @@ fn cross_compile_one( } } -fn compile(perry_bin: &Path, src: &Path, out: &Path) -> Result<()> { - let out_status = Command::new(perry_bin) - .arg(src) +fn configure_compile_environment(cmd: &mut Command, example: &Example) { + // The host wrappers select prebuilt libraries for the ordinary examples. + // Fastify requires a specialized stdlib with its request pump, so remove + // the override only from this compiler child. Never mutate the harness's + // environment: subsequent examples still benefit from the prebuilt libs. + if example.requires_auto_optimize { + cmd.env_remove("PERRY_NO_AUTO_OPTIMIZE"); + } +} + +fn compile(perry_bin: &Path, example: &Example, out: &Path) -> Result<()> { + let mut cmd = Command::new(perry_bin); + configure_compile_environment(&mut cmd, example); + let out_status = cmd + .arg(&example.path) .arg("-o") .arg(out) .output() - .with_context(|| format!("launching perry for {}", src.display()))?; + .with_context(|| format!("launching perry for {}", example.path.display()))?; if !out_status.status.success() { return Err(anyhow!( "perry {}: {}", diff --git a/crates/perry-doc-tests/src/tests.rs b/crates/perry-doc-tests/src/tests.rs new file mode 100644 index 0000000000..ddb8edd022 --- /dev/null +++ b/crates/perry-doc-tests/src/tests.rs @@ -0,0 +1,41 @@ +use super::*; + +#[test] +fn fastify_examples_request_specialized_compilation_without_being_skipped() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../docs/examples"); + for file in [ + "getting-started/npm_packages.ts", + "stdlib/http/fastify_json.ts", + "stdlib/overview/snippets.ts", + ] { + let banner = read_banner(&root.join(file)).unwrap(); + assert!( + banner.requires_auto_optimize, + "{file}: must rebuild the Fastify pump" + ); + assert!( + banner.compile_only, + "{file}: requires external services to run" + ); + for host in ["macos", "linux", "windows"] { + assert!( + banner.platforms.contains(host), + "{file}: {host} must compile it" + ); + } + } +} + +#[test] +fn unknown_requirement_is_an_error_instead_of_silently_disabling_coverage() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("typo.ts"); + std::fs::write( + &path, + "// requires: auto-optmize\nconsole.log('example');\n", + ) + .unwrap(); + let error = read_banner(&path).unwrap_err().to_string(); + assert!(error.contains("typo.ts")); + assert!(error.contains("unknown doc-example requirement `auto-optmize`")); +} diff --git a/crates/perry-doc-tests/tests/compiler_environment.rs b/crates/perry-doc-tests/tests/compiler_environment.rs new file mode 100644 index 0000000000..2d9884a143 --- /dev/null +++ b/crates/perry-doc-tests/tests/compiler_environment.rs @@ -0,0 +1,109 @@ +#![cfg(unix)] + +use std::os::unix::fs::PermissionsExt; +use std::process::Command; + +fn run_examples(reject_required: bool) -> (std::process::Output, serde_json::Value, String) { + let dir = tempfile::tempdir().unwrap(); + let examples = dir.path().join("examples"); + std::fs::create_dir(&examples).unwrap(); + std::fs::write( + examples.join("required.ts"), + "// requires: auto-optimize\n// run: false\nconsole.log('required');\n", + ) + .unwrap(); + std::fs::write( + examples.join("ordinary.ts"), + "// run: false\nconsole.log('ordinary');\n", + ) + .unwrap(); + std::fs::write( + examples.join("z_after.ts"), + "// run: false\nconsole.log('after');\n", + ) + .unwrap(); + let compiler = dir.path().join("compiler.sh"); + std::fs::write( + &compiler, + r#"#!/bin/sh +case "$1" in + */required.ts) + if test "${PERRY_NO_AUTO_OPTIMIZE+x}" = x; then + echo 'required example inherited PERRY_NO_AUTO_OPTIMIZE' >&2 + exit 7 + fi + echo required:auto >> "$PERRY_DOC_TEST_COMPILER_LOG" + if test "$PERRY_DOC_TEST_REJECT_REQUIRED" = 1; then + echo 'required example compilation failed' >&2 + exit 17 + fi + ;; + */ordinary.ts|*/z_after.ts) + if test "$PERRY_NO_AUTO_OPTIMIZE" != 1; then + echo 'ordinary example lost its prebuilt-archive setting' >&2 + exit 8 + fi + echo "$(basename "$1" .ts):prebuilt" >> "$PERRY_DOC_TEST_COMPILER_LOG" + ;; + *) exit 9 ;; +esac +"#, + ) + .unwrap(); + std::fs::set_permissions(&compiler, std::fs::Permissions::from_mode(0o755)).unwrap(); + let report = dir.path().join("report.json"); + let log = dir.path().join("compiler.log"); + let output = Command::new(env!("CARGO_BIN_EXE_doc-tests")) + .current_dir(env!("CARGO_MANIFEST_DIR")) + .args(["--skip-xcompile", "--examples-dir"]) + .arg(&examples) + .arg("--perry") + .arg(&compiler) + .arg("--json") + .arg(&report) + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_DOC_TEST_COMPILER_LOG", &log) + .env( + "PERRY_DOC_TEST_REJECT_REQUIRED", + if reject_required { "1" } else { "0" }, + ) + .output() + .unwrap(); + let report = serde_json::from_slice(&std::fs::read(report).unwrap()).unwrap(); + (output, report, std::fs::read_to_string(log).unwrap()) +} + +#[test] +fn required_compilation_removes_only_its_own_no_auto_override() { + let (output, report, calls) = run_examples(false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stdout) + ); + assert_eq!(report["passed"], 3); + assert_eq!(report["failed"], 0); + assert_eq!(report["skipped"], 0); + assert_eq!( + calls, + "ordinary:prebuilt\nrequired:auto\nz_after:prebuilt\n" + ); +} + +#[test] +fn required_compilation_failures_are_counted_and_fail_the_harness() { + let (output, report, calls) = run_examples(true); + assert_eq!(output.status.code(), Some(1)); + assert_eq!(report["passed"], 2); + assert_eq!(report["failed"], 1); + assert_eq!(report["skipped"], 0); + assert_eq!(report["results"][1]["status"], "compile_fail"); + assert!(report["results"][1]["detail"] + .as_str() + .unwrap() + .contains("exit=17")); + assert_eq!( + calls, + "ordinary:prebuilt\nrequired:auto\nz_after:prebuilt\n" + ); +} diff --git a/crates/perry-runtime/src/array/element_shape.rs b/crates/perry-runtime/src/array/element_shape.rs index 0158dbad33..29f0f245cc 100644 --- a/crates/perry-runtime/src/array/element_shape.rs +++ b/crates/perry-runtime/src/array/element_shape.rs @@ -672,6 +672,25 @@ pub(crate) fn transfer_element_shape(old_user: usize, new_user: usize) { // exactly the versioning a consumer guards on. let had_bit = array_gc_header(old_user as *const ArrayHeader) .is_some_and(|old_header| header_has_bit(old_header)); + // #9792: neither address advertises a proof, so there is nothing to + // move and nothing to fail closed about — the `clear_bit` below would + // clear a bit that is already clear. Skipping is what the siblings in + // `gc::layout_tables` do with their emptiness flag, decided here from + // the header words this function has already read rather than from a + // side-table probe. + // + // What it leaves behind is a record at an address whose bit is clear, + // and that state is already part of the design: the bit is the sole + // authority for a read (`element_shape_proof` returns `None` without + // touching the table), and `establish` draws every identity from + // `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever record sits at + // the address, precisely so a survivor cannot donate its epoch to the + // next array established there. `prune_dead_element_shape_owners` + // drops it on the next collection, which is the same footprint-only + // guarantee a fail-closed transfer already relied on. + if !had_bit && !header_has_bit(new_header) { + return; + } let moved = ELEMENT_SHAPES.with(|m| { let mut map = m.borrow_mut(); map.remove(&new_user); @@ -795,6 +814,16 @@ pub(crate) unsafe fn test_element_shape_bit_set(arr: *const ArrayHeader) -> bool array_gc_header(arr).is_some_and(|header| header_has_bit(header)) } +/// Clear the advertising bit and leave the record in the table — the survivor +/// state a skipped [`transfer_element_shape`] produces, and the one the +/// bit-is-authority rule has to hold up under. +#[cfg(test)] +pub(crate) unsafe fn test_clear_element_shape_bit_only(arr: *mut ArrayHeader) { + if let Some(header) = array_gc_header(arr) { + clear_bit(header); + } +} + #[cfg(test)] #[path = "element_shape_tests.rs"] mod tests; diff --git a/crates/perry-runtime/src/array/element_shape_tests.rs b/crates/perry-runtime/src/array/element_shape_tests.rs index 80199a91a0..fa93b4f76c 100644 --- a/crates/perry-runtime/src/array/element_shape_tests.rs +++ b/crates/perry-runtime/src/array/element_shape_tests.rs @@ -541,6 +541,71 @@ fn a_fail_closed_transfer_leaves_no_record_for_the_next_array_to_inherit() { ); } +#[test] +fn a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof() { + // #9792: `transfer_element_shape` runs for every relocated array, and its + // `had_bit` verdict is free — two header words it has already read. When + // neither address advertises a proof it takes the side table anyway, for + // two removes that can only remove what no reader could reach. The gate + // that skips that has exactly ONE safe shape, and this pins both halves. + // + // Arm 1 — neither bit set: skipping is correct, and the record it leaves + // behind stays unreadable and cannot donate its identity. + // Arm 2 — the DESTINATION still advertises one: skipping would leave a + // live proof describing storage that has just been overwritten, so the + // gate must NOT fire on `!had_bit` alone. + let _serialized = test_serialize(); + + // Arm 1. + let src = built_from_pushes(CLASS_A, 2); + let dst = built_from_pushes(CLASS_A, 2); + let survivor = proof(dst).expect("proven").epoch; + unsafe { clear_element_shape(src) }; + unsafe { test_clear_element_shape_bit_only(dst) }; + assert!( + test_element_shape_record_exists(dst as usize), + "the fixture must actually leave a record behind the cleared bit" + ); + + transfer_element_shape(src as usize, dst as usize); + + assert!( + proof(dst).is_none(), + "the bit is the sole authority for a read, so a record behind a \ + cleared bit must not read as a proof" + ); + let reproven = unsafe { ensure_element_shape(dst) }.expect("still homogeneous"); + assert_ne!( + reproven.epoch, survivor, + "establishing draws a fresh identity from ELEMENT_SHAPE_PROOF_SEQ, so \ + a survivor record can never donate its epoch" + ); + + // Arm 2: source proves nothing, destination still advertises a proof. + let src2 = built_from_pushes(CLASS_A, 2); + let dst2 = built_from_pushes(CLASS_A, 2); + unsafe { clear_element_shape(src2) }; + assert!( + unsafe { test_element_shape_bit_set(dst2) }, + "the fixture must leave the destination advertising a proof" + ); + + transfer_element_shape(src2 as usize, dst2 as usize); + + unsafe { + assert!( + !test_element_shape_bit_set(dst2), + "a transfer whose source proved nothing must still fail the \ + destination closed — gating on `!had_bit` alone would leave a \ + live proof over storage the move has just replaced" + ); + } + assert!( + !test_element_shape_record_exists(dst2 as usize), + "and it must take the destination's record with it" + ); +} + // --------------------------------------------------------------------------- // Lifecycle hooks — what stops a recycled address inheriting a stale identity // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 9724feb04c..b9f8519701 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1552,7 +1552,7 @@ pub(super) fn run_copied_minor_attempt( if gc_verify_evacuation_enabled() { let phase_start = trace_phase_start(trace); let valid_ptrs = build_valid_pointer_set(); - verify_evacuated_no_stale_forwarded_refs(&valid_ptrs); + verify_evacuated_no_stale_forwarded_refs(EvacuationVerifier::copying_minor(&valid_ptrs)); trace_phase_record(trace, "evacuation_verify", phase_start); } diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index b34e9eff3e..66777e520a 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -1411,7 +1411,9 @@ impl GcCycleState { trace_phase_record(&mut self.trace, "reference_rewrite", phase_start); if gc_verify_evacuation_enabled() { let phase_start = trace_phase_start(&self.trace); - verify_evacuated_no_stale_forwarded_refs(valid_ptrs); + verify_evacuated_no_stale_forwarded_refs(EvacuationVerifier::all_forwarded( + valid_ptrs, + )); trace_phase_record(&mut self.trace, "evacuation_verify", phase_start); } let released = diff --git a/crates/perry-runtime/src/gc/diag_sites.rs b/crates/perry-runtime/src/gc/diag_sites.rs index 4044afb4e0..462d55afcb 100644 --- a/crates/perry-runtime/src/gc/diag_sites.rs +++ b/crates/perry-runtime/src/gc/diag_sites.rs @@ -374,7 +374,7 @@ pub(super) fn report_charges(label: &str) { // render path turns into O(length) allocations per call, and the only way to // tell WHICH names those are is to count them at the fork. -thread_local! { +crate::perry_thread_local! { /// `".prototype." -> (calls, receiver_utf16_chars)`. static PRIMITIVE_DISPATCH: RefCell> = RefCell::new(HashMap::new()); diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index 054bee16f7..c40c29cf9c 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -728,7 +728,7 @@ pub(super) enum RuntimeRootVisitMode<'a> { valid_ptrs: &'a ValidPointerSet, }, Verify { - valid_ptrs: &'a ValidPointerSet, + verifier: EvacuationVerifier<'a>, surface: &'static str, }, Copy { @@ -807,12 +807,9 @@ impl<'a> RuntimeRootVisitor<'a> { } } - pub(super) fn for_verify(valid_ptrs: &'a ValidPointerSet, surface: &'static str) -> Self { + pub(super) fn for_verify(verifier: EvacuationVerifier<'a>, surface: &'static str) -> Self { Self { - mode: RuntimeRootVisitMode::Verify { - valid_ptrs, - surface, - }, + mode: RuntimeRootVisitMode::Verify { verifier, surface }, root_source_stats: None, young_scope: false, } @@ -909,11 +906,8 @@ impl<'a> RuntimeRootVisitor<'a> { RuntimeRootVisitMode::Rewrite { valid_ptrs } => { try_rewrite_nanboxed_value(bits, valid_ptrs) } - RuntimeRootVisitMode::Verify { - valid_ptrs, - surface, - } => { - if let Some(new_bits) = try_rewrite_nanboxed_value(bits, valid_ptrs) { + RuntimeRootVisitMode::Verify { verifier, surface } => { + if let Some(new_bits) = verifier.stale_nanboxed_value(bits) { panic_stale_forwarded_reference(surface, 0, bits, new_bits); } None @@ -941,11 +935,8 @@ impl<'a> RuntimeRootVisitor<'a> { collector.rewrite_value_bits(bits) } RuntimeRootVisitMode::Rewrite { valid_ptrs } => try_rewrite_value(bits, valid_ptrs), - RuntimeRootVisitMode::Verify { - valid_ptrs, - surface, - } => { - if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) { + RuntimeRootVisitMode::Verify { verifier, surface } => { + if let Some(new_bits) = verifier.stale_value(bits) { panic_stale_forwarded_reference(surface, 0, bits, new_bits); } None @@ -981,11 +972,8 @@ impl<'a> RuntimeRootVisitor<'a> { RuntimeRootVisitMode::CopyingMark { collector } => collector.visit_raw_addr(addr), RuntimeRootVisitMode::CopyingRewrite { collector } => collector.rewrite_raw_addr(addr), RuntimeRootVisitMode::Rewrite { valid_ptrs } => try_rewrite_raw_addr(addr, valid_ptrs), - RuntimeRootVisitMode::Verify { - valid_ptrs, - surface, - } => { - if let Some(new_addr) = try_rewrite_raw_addr(addr, valid_ptrs) { + RuntimeRootVisitMode::Verify { verifier, surface } => { + if let Some(new_addr) = verifier.stale_raw_addr(addr) { panic_stale_forwarded_reference( surface, 0, @@ -1012,11 +1000,8 @@ impl<'a> RuntimeRootVisitor<'a> { RuntimeRootVisitMode::CopyingCheck { .. } => None, RuntimeRootVisitMode::CopyingMark { .. } => None, RuntimeRootVisitMode::CopyingRewrite { collector } => collector.rewrite_raw_addr(addr), - RuntimeRootVisitMode::Verify { - valid_ptrs, - surface, - } => { - if let Some(new_addr) = try_rewrite_raw_addr(addr, valid_ptrs) { + RuntimeRootVisitMode::Verify { verifier, surface } => { + if let Some(new_addr) = verifier.stale_raw_addr(addr) { panic_stale_forwarded_reference(surface, 0, addr as u64, new_addr as u64); } None diff --git a/crates/perry-runtime/src/gc/tests/forwarding_verification.rs b/crates/perry-runtime/src/gc/tests/forwarding_verification.rs new file mode 100644 index 0000000000..5e74aa4d40 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/forwarding_verification.rs @@ -0,0 +1,152 @@ +use super::super::*; +use super::support::*; + +// Solid's effect.sources array grows after promotion. Its owning computation +// retains the old array address, which remains a supported growth alias. +#[test] +fn copying_verifier_accepts_retained_array_growth_alias_in_old_field() { + let _guard = CopyingNurseryTestGuard::new(2); + let _verify_guard = VerifyEvacuationTestGuard::on(); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let (stub, _) = unsafe { alloc_old_test_array(1) }; + let (holder, field) = unsafe { alloc_old_test_object(1) }; + unsafe { + layout_init_pointer_free(stub as *mut u8); + layout_init_pointer_free(holder as *mut u8); + crate::object::store_object_field_slot(holder, 0, ptr_bits(stub as usize)); + } + let grown = crate::array::js_array_push_f64(stub, 42.0); + assert_ne!(stub, grown); + assert!(crate::arena::pointer_in_old_gen(stub as usize)); + assert!(crate::arena::pointer_in_old_gen(grown as usize)); + js_shadow_slot_set(0, ptr_bits(holder as usize)); + let young = young_leaf(); + js_shadow_slot_set(1, ptr_bits(young)); + + let trace = collect_minor_trace(GcTriggerKind::Direct); + + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!(trace.phase_us.contains_key("evacuation_verify")); + assert_ne!(js_shadow_slot_get(1), ptr_bits(young)); + assert_eq!(unsafe { *field }, ptr_bits(stub as usize)); + assert_eq!(crate::array::js_array_get_f64(stub, 1), 42.0); +} + +#[test] +fn copying_verifier_accepts_retained_growth_chains_across_root_formats() { + let _guard = CopyingNurseryTestGuard::new(0); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let (stub, _) = unsafe { alloc_old_test_array(1) }; + let next = crate::array::js_array_grow(stub, 2); + let target = crate::array::js_array_grow(next, 4); + assert_ne!(stub, next); + assert_ne!(next, target); + let valid_ptrs = build_valid_pointer_set(); + let verifier = EvacuationVerifier::copying_minor(&valid_ptrs); + let bits = ptr_bits(stub as usize); + + // The ordinary rewrite and non-copying verifier still canonicalize every + // hop, including old arrays moved during old-page evacuation. + assert_eq!( + try_rewrite_value(bits, &valid_ptrs), + Some(ptr_bits(target as usize)) + ); + assert_eq!( + EvacuationVerifier::all_forwarded(&valid_ptrs).stale_value(bits), + Some(ptr_bits(target as usize)) + ); + let mut visitor = RuntimeRootVisitor::for_verify(verifier, "retained growth root"); + assert_eq!(visitor.visit_nanbox_bits(bits), None); + assert_eq!(visitor.visit_heap_word_bits(stub as usize as u64), None); + assert_eq!( + visitor.visit_tagged_raw_addr(stub as usize, POINTER_TAG), + None + ); + assert_eq!(visitor.visit_metadata_raw_addr(stub as usize), None); + verify_copy_only_scanner_bits(bits, verifier, "retained copy-only root"); + let mut context = verifier; + perry_ffi_verify_root( + f64::from_bits(bits), + &mut context as *mut EvacuationVerifier<'_> as *mut c_void, + ); +} + +#[test] +fn copying_verifier_rejects_from_space_hops_even_through_retained_stubs() { + let _guard = CopyingNurseryTestGuard::new(0); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let (stub, _) = unsafe { alloc_old_test_array(1) }; + let from_space = crate::array::js_array_alloc(1); + let (target, _) = unsafe { alloc_old_test_array(1) }; + assert!(super::super::fromspace_scan::is_from_space( + crate::arena::classify_heap_space(from_space as usize) + )); + unsafe { + // Sabotage: array growth forbids an old -> young forwarding edge. + // The verifier must still reject it if it somehow occurs, even when + // the first hop is a retained old array and only the second is stale. + set_forwarding_address( + header_from_user_ptr(stub as *const u8), + from_space as *mut u8, + ); + } + let valid_ptrs = build_valid_pointer_set(); + let verifier = EvacuationVerifier::copying_minor(&valid_ptrs); + assert_eq!( + verifier.stale_raw_addr(stub as usize), + Some(from_space as usize), + "a retained stub must not reference even an unforwarded young array" + ); + unsafe { + set_forwarding_address( + header_from_user_ptr(from_space as *const u8), + target as *mut u8, + ); + } + for source in [stub, from_space] { + let bits = ptr_bits(source as usize); + assert_eq!( + verifier.stale_raw_addr(source as usize), + Some(target as usize) + ); + assert_eq!( + verifier.stale_value(source as usize as u64), + Some(target as usize as u64) + ); + assert_eq!( + verifier.stale_nanboxed_value(bits), + Some(ptr_bits(target as usize)) + ); + for format in 0..4 { + let failure = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut visitor = RuntimeRootVisitor::for_verify(verifier, "from-space control"); + match format { + 0 => { + visitor.visit_nanbox_bits(bits); + } + 1 => { + visitor.visit_heap_word_bits(source as usize as u64); + } + 2 => { + visitor.visit_tagged_raw_addr(source as usize, POINTER_TAG); + } + _ => { + visitor.visit_metadata_raw_addr(source as usize); + } + } + })); + assert!( + failure.is_err(), + "root format {format} must reject a stale hop" + ); + } + let failure = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| unsafe { + verify_slot(&bits, verifier, "from-space heap control"); + })); + assert!(failure.is_err()); + } + // Leave a valid retained alias for subsequent tests' heap walks. + unsafe { + set_forwarding_address(header_from_user_ptr(stub as *const u8), target as *mut u8); + } +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 2d3300e1cd..186fe29187 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -22,6 +22,7 @@ mod error_side_tables; mod evacuation; mod forwarded_stub_membership; mod forwarding_target_validation; +mod forwarding_verification; mod fromspace_protect; mod fromspace_scan; mod global_bootstrap; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs index 26aaa9fc0d..bcf72ccefd 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs @@ -976,7 +976,7 @@ fn test_evacuation_verify_detects_stale_forwarded_root_slot() { js_shadow_slot_set(0, fixture.nursery_bits); assert_panics_with("shadow stack roots", || { - verify_mutable_root_slots(&fixture.valid_ptrs); + verify_mutable_root_slots(EvacuationVerifier::all_forwarded(&fixture.valid_ptrs)); }); js_shadow_frame_pop(shadow); @@ -994,8 +994,10 @@ fn test_evacuation_verify_detects_stale_forwarded_runtime_scanner_slot() { ); assert_panics_with("runtime mutable root scanner", || { - let mut visitor = - RuntimeRootVisitor::for_verify(&fixture.valid_ptrs, "runtime mutable root scanner"); + let mut visitor = RuntimeRootVisitor::for_verify( + EvacuationVerifier::all_forwarded(&fixture.valid_ptrs), + "runtime mutable root scanner", + ); promise_mutable_root_scanner(&mut visitor); }); @@ -1020,7 +1022,7 @@ fn test_evacuation_verify_detects_stale_forwarded_dirty_range_slot() { } assert_panics_with("remembered dirty ranges", || { - verify_remembered_dirty_ranges(&valid_ptrs); + verify_remembered_dirty_ranges(EvacuationVerifier::all_forwarded(&valid_ptrs)); }); remembered_set_clear(); @@ -1036,7 +1038,11 @@ fn test_evacuation_verify_detects_stale_forwarded_heap_field() { let header = header_from_user_ptr(old_obj as *const u8); (*header).gc_flags |= GC_FLAG_MARKED; assert_panics_with("heap fields", || { - verify_heap_object_fields(header, &fixture.valid_ptrs, "heap fields"); + verify_heap_object_fields( + header, + EvacuationVerifier::all_forwarded(&fixture.valid_ptrs), + "heap fields", + ); }); (*header).gc_flags &= !GC_FLAG_MARKED; } @@ -1051,7 +1057,7 @@ fn test_evacuation_verify_copy_only_pinned_root_allows_non_forwarded_target() { } verify_copy_only_scanner_bits( POINTER_TAG | (user as u64 & POINTER_MASK), - &valid_ptrs, + EvacuationVerifier::all_forwarded(&valid_ptrs), "copy-only root scanner", ); unsafe { @@ -1065,7 +1071,7 @@ fn test_evacuation_verify_copy_only_root_rejects_forwarded_target() { assert_panics_with("copy-only root scanner", || { verify_copy_only_scanner_bits( fixture.nursery_bits, - &fixture.valid_ptrs, + EvacuationVerifier::all_forwarded(&fixture.valid_ptrs), "copy-only root scanner", ); }); diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs index 10ee93d3fe..4e14a61002 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs @@ -503,7 +503,7 @@ fn test_class_inheritance_side_table_roots_mark_and_rewrite() { // A verify pass must not panic now that the slots point at the live // (non-forwarded) evacuated objects. scan_class_inheritance_roots_mut(&mut RuntimeRootVisitor::for_verify( - &valid_ptrs, + EvacuationVerifier::all_forwarded(&valid_ptrs), "class inheritance side-table roots (test)", )); diff --git a/crates/perry-runtime/src/gc/verify.rs b/crates/perry-runtime/src/gc/verify.rs index 0332bcf3b8..9d6bb0b26f 100644 --- a/crates/perry-runtime/src/gc/verify.rs +++ b/crates/perry-runtime/src/gc/verify.rs @@ -26,9 +26,11 @@ pub(super) fn try_rewrite_nanboxed_value(bits: u64, valid_ptrs: &ValidPointerSet /// #8174: refuses a forwarding target that is not a heap object start, in /// lockstep with [`CopyingNurseryCollector::rewrite_raw_addr`](super::copying). /// -/// The lockstep is the point. This function is what the VERIFY pass runs -/// (`RuntimeRootVisitMode::Verify`), and it panics whenever it can rewrite a -/// slot the rewrite pass left alone. Tightening only the rewrite pass would +/// The lockstep is the point. The VERIFY pass shares this forwarding walker +/// and panics when it finds a stale alias the rewrite pass left alone. +/// Copying verification permits retained array-growth aliases through +/// [`EvacuationVerifier`], without changing source/target validation. +/// Tightening only the rewrite pass would /// therefore have turned a silently-corrupt rewrite into a `PERRY_GC_VERIFY_ /// EVACUATION` abort blaming an innocent scanner — the two walkers must reach /// the same verdict or the verifier is measuring the difference between them @@ -36,6 +38,82 @@ pub(super) fn try_rewrite_nanboxed_value(bits: u64, valid_ptrs: &ValidPointerSet /// stronger than the copier's heap-region test, so this only changes the case /// where a genuinely LIVE forwarded object's target word is corrupt. pub(super) fn try_rewrite_raw_addr(ptr_addr: usize, valid_ptrs: &ValidPointerSet) -> Option { + follow_forwarding_raw_addr(ptr_addr, valid_ptrs, |_, _| true) +} + +/// The copying minor retains non-moving array-growth stubs. Other evacuation +/// paths rewrite every forwarding alias before releasing moved originals. +/// Carry that distinction through every verifier surface, including FFI roots. +#[derive(Clone, Copy)] +pub(super) struct EvacuationVerifier<'a> { + valid_ptrs: &'a ValidPointerSet, + copying_minor: bool, +} + +impl<'a> EvacuationVerifier<'a> { + pub(super) fn all_forwarded(valid_ptrs: &'a ValidPointerSet) -> Self { + Self { + valid_ptrs, + copying_minor: false, + } + } + + /// Must run before the copying minor resets from-space and flips survivors. + pub(super) fn copying_minor(valid_ptrs: &'a ValidPointerSet) -> Self { + Self { + valid_ptrs, + copying_minor: true, + } + } + + pub(super) fn stale_raw_addr(self, addr: usize) -> Option { + follow_forwarding_raw_addr(addr, self.valid_ptrs, |source, target| { + if !self.copying_minor { + return true; + } + // Only array growth creates permanent forwarding aliases. The + // copying minor neither moves nor frees these non-moving sources. + // Both ends must be retained arrays. An old -> young growth edge + // is forbidden even if the young target was never forwarded. + // Follow the whole chain so an indirect unsafe hop is also caught. + !self.retained_growth_array(source) || !self.retained_growth_array(target) + }) + } + + fn retained_growth_array(self, addr: usize) -> bool { + if !self.valid_ptrs.contains(&addr) { + return false; + } + let header = unsafe { header_from_user_ptr(addr as *const u8) }; + (unsafe { (*header).obj_type == GC_TYPE_ARRAY }) + && matches!( + crate::arena::classify_heap_space(addr), + crate::arena::HeapSpace::Old + | crate::arena::HeapSpace::Longlived + | crate::arena::HeapSpace::PromotedYoung + ) + } + + pub(super) fn stale_value(self, bits: u64) -> Option { + let word = decode_root_word(bits)?; + Some(word.encode(self.stale_raw_addr(word.addr())?)) + } + + pub(super) fn stale_nanboxed_value(self, bits: u64) -> Option { + let tag = bits & TAG_MASK; + if tag != POINTER_TAG && tag != STRING_TAG && tag != BIGINT_TAG { + return None; + } + let addr = self.stale_raw_addr((bits & POINTER_MASK) as usize)?; + Some(tag | (addr as u64 & POINTER_MASK)) + } +} + +fn follow_forwarding_raw_addr( + ptr_addr: usize, + valid_ptrs: &ValidPointerSet, + must_rewrite: impl Fn(usize, usize) -> bool, +) -> Option { if ptr_addr == 0 { return None; } @@ -57,8 +135,8 @@ pub(super) fn try_rewrite_raw_addr(ptr_addr: usize, valid_ptrs: &ValidPointerSet if !accept_forwarding_target(next) { return None; } + rewrote |= must_rewrite(current, next); current = next; - rewrote = true; } } rewrote.then_some(current) @@ -87,9 +165,13 @@ pub(super) unsafe fn rewrite_slot(slot: *mut u64, valid_ptrs: &ValidPointerSet) } #[inline] -pub(super) unsafe fn verify_slot(slot: *const u64, valid_ptrs: &ValidPointerSet, surface: &str) { +pub(super) unsafe fn verify_slot( + slot: *const u64, + verifier: EvacuationVerifier<'_>, + surface: &str, +) { let bits = *slot; - if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_value(bits) { panic_stale_forwarded_reference(surface, slot as usize, bits, new_bits); } } @@ -1122,7 +1204,7 @@ pub(super) fn verify_minor_unmarked_young_children_report(phase: &str) { pub(super) unsafe fn verify_heap_object_fields( header: *mut GcHeader, - valid_ptrs: &ValidPointerSet, + verifier: EvacuationVerifier<'_>, surface: &'static str, ) { let flags = (*header).gc_flags; @@ -1131,7 +1213,7 @@ pub(super) unsafe fn verify_heap_object_fields( } visit_gc_rewrite_slots(header, |slot| unsafe { slot.record_layout_read(); - verify_slot(slot.slot as *const u64, valid_ptrs, surface); + verify_slot(slot.slot as *const u64, verifier, surface); }); } @@ -1263,13 +1345,13 @@ pub(super) fn rewrite_mutable_registered_roots_with_sources( visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } -pub(super) fn verify_mutable_root_slots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_mutable_root_slots(verifier: EvacuationVerifier<'_>) { visit_mutable_root_slots(|slot| unsafe { let bits = slot.read(); if bits == 0 { return; } - if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_value(bits) { let surface = match slot.kind { MutableRootSlotKind::ShadowStack => "shadow stack roots", MutableRootSlotKind::NativeStack => "native stack-map roots", @@ -1280,9 +1362,9 @@ pub(super) fn verify_mutable_root_slots(valid_ptrs: &ValidPointerSet) { }); } -pub(super) fn verify_mutable_registered_roots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_mutable_registered_roots(verifier: EvacuationVerifier<'_>) { let scanners: Vec = MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let mut visitor = RuntimeRootVisitor::for_verify(valid_ptrs, "runtime mutable root scanner"); + let mut visitor = RuntimeRootVisitor::for_verify(verifier, "runtime mutable root scanner"); for entry in scanners { (entry.scanner)(&mut visitor); } @@ -1291,72 +1373,67 @@ pub(super) fn verify_mutable_registered_roots(valid_ptrs: &ValidPointerSet) { pub(super) fn verify_copy_only_scanner_bits( bits: u64, - valid_ptrs: &ValidPointerSet, + verifier: EvacuationVerifier<'_>, surface: &'static str, ) { - if let Some(new_bits) = try_rewrite_nanboxed_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_nanboxed_value(bits) { panic_stale_forwarded_reference(surface, 0, bits, new_bits); } } -pub(super) struct RegisteredRootVerifyContext { - pub(super) valid_ptrs: *const ValidPointerSet, -} - pub(super) extern "C" fn perry_ffi_verify_root(value: f64, ctx: *mut c_void) { if ctx.is_null() { return; } - let ctx = unsafe { &*(ctx as *const RegisteredRootVerifyContext) }; - if ctx.valid_ptrs.is_null() { - return; - } - let valid_ptrs = unsafe { &*ctx.valid_ptrs }; - verify_copy_only_scanner_bits(value.to_bits(), valid_ptrs, "ffi copy-only root scanner"); + let verifier = unsafe { *(ctx as *const EvacuationVerifier<'_>) }; + verify_copy_only_scanner_bits(value.to_bits(), verifier, "ffi copy-only root scanner"); } -pub(super) fn verify_copy_only_registered_roots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_copy_only_registered_roots(verifier: EvacuationVerifier<'_>) { let scanners: Vec = ROOT_SCANNERS.with(|s| s.borrow().clone()); for scanner in scanners { scanner(&mut |value: f64| { - verify_copy_only_scanner_bits(value.to_bits(), valid_ptrs, "copy-only root scanner"); + verify_copy_only_scanner_bits(value.to_bits(), verifier, "copy-only root scanner"); }); } let ffi_scanners: Vec = FFI_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let mut ctx = RegisteredRootVerifyContext { - valid_ptrs: valid_ptrs as *const ValidPointerSet, - }; - let ctx = &mut ctx as *mut RegisteredRootVerifyContext as *mut c_void; + let mut ctx = verifier; + let ctx = &mut ctx as *mut EvacuationVerifier<'_> as *mut c_void; for scanner in ffi_scanners { scanner(perry_ffi_verify_root, ctx); } } -pub(super) fn verify_remembered_dirty_ranges(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_remembered_dirty_ranges(verifier: EvacuationVerifier<'_>) { let snapshot = remembered_dirty_snapshot(); let mut stats = RememberedSetTraceStats::default(); let mut verify_dirty_slot = |slot: *mut u64, _stats: &mut RememberedSetTraceStats| unsafe { - verify_slot(slot as *const u64, valid_ptrs, "remembered dirty ranges"); + verify_slot(slot as *const u64, verifier, "remembered dirty ranges"); }; - scan_remembered_dirty_slot_ranges(&snapshot, valid_ptrs, &mut stats, &mut verify_dirty_slot); + scan_remembered_dirty_slot_ranges( + &snapshot, + verifier.valid_ptrs, + &mut stats, + &mut verify_dirty_slot, + ); for header_addr in snapshot.fallback_headers { let user_ptr = header_addr + GC_HEADER_SIZE; - if !valid_ptrs.contains(&user_ptr) { + if !verifier.valid_ptrs.contains(&user_ptr) { continue; } unsafe { verify_heap_object_fields( header_addr as *mut GcHeader, - valid_ptrs, + verifier, "remembered fallback headers", ); } } } -pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_heap_objects(verifier: EvacuationVerifier<'_>) { let verify_one = |header: *mut GcHeader| unsafe { let flags = (*header).gc_flags; if flags & GC_FLAG_FORWARDED != 0 { @@ -1373,7 +1450,7 @@ pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { return; } } - verify_heap_object_fields(header, valid_ptrs, "heap fields"); + verify_heap_object_fields(header, verifier, "heap fields"); }; crate::arena::arena_walk_objects(|hp| verify_one(hp as *mut GcHeader)); MALLOC_STATE.with(|s| { @@ -1384,12 +1461,12 @@ pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { }); } -pub(super) fn verify_evacuated_no_stale_forwarded_refs(valid_ptrs: &ValidPointerSet) { - verify_mutable_root_slots(valid_ptrs); - verify_mutable_registered_roots(valid_ptrs); - verify_copy_only_registered_roots(valid_ptrs); - verify_remembered_dirty_ranges(valid_ptrs); - verify_heap_objects(valid_ptrs); +pub(super) fn verify_evacuated_no_stale_forwarded_refs(verifier: EvacuationVerifier<'_>) { + verify_mutable_root_slots(verifier); + verify_mutable_registered_roots(verifier); + verify_copy_only_registered_roots(verifier); + verify_remembered_dirty_ranges(verifier); + verify_heap_objects(verifier); } /// Top-level Phase C4b-γ-2 entry: rewrite every reference site we diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index b4bd5238ee..82b4bdaee9 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -105,6 +105,12 @@ pub struct RegexDiag { /// Sum of pattern bytes seen by `js_regexp_new` (what a content hash or /// copy of the pattern costs per construction). pub new_pattern_bytes: u64, + /// `js_regexp_new` had to allocate a GC string for the canonical flags + /// because the caller's flags string was not already in canonical form. + /// The common case — a regex literal, whose flags text the author wrote in + /// spec order — shares the caller's immutable string instead, so this + /// counter is the per-construction flags allocation that remains. + pub new_flags_allocated: u64, pub compiles_std: u64, pub compiles_fancy: u64, pub compiles_repeat: u64, @@ -238,7 +244,7 @@ impl RegexDiag { "[regex-diag] t={secs:.1}s new={} validated_hit={} site_hit={} pattern_bytes={} \ compiles std={} fancy={} repeat={} cache_clears={} lazy_builds={} lazy_cache_hits={} \ exec={} exec_matched={} capture_slots={} capture_bytes={} test={} test_global={} \ - match={} replace={} replace_matches={} split={}", + match={} replace={} replace_matches={} split={} flags_alloc={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -259,6 +265,7 @@ impl RegexDiag { self.replace_calls, self.replace_matches, self.split_calls, + self.new_flags_allocated, ); // Merge by content (prefix, len, flags): distinct literal sites with // the same pattern are one row. @@ -772,3 +779,168 @@ impl IcDiag { out } } + +// --------------------------------------------------------------------------- +// Enumeration and concatenation: EXECUTIONS per site, not bytes +// --------------------------------------------------------------------------- + +static ENUM_SINK: OnceLock> = OnceLock::new(); +static ENUM_ON: AtomicBool = AtomicBool::new(false); + +fn enum_sink() -> &'static Option { + ENUM_SINK.get_or_init(|| { + let sink = sink_from_env("PERRY_ENUM_DIAG"); + ENUM_ON.store(sink.is_some(), Ordering::Relaxed); + sink + }) +} + +/// Is the enumeration/concat execution counter armed? +#[inline] +pub fn enum_on() -> bool { + if ENUM_SINK.get().is_none() { + enum_sink(); + } + ENUM_ON.load(Ordering::Relaxed) +} + +/// What actually runs at the two allocation sites the byte-share ranking put +/// at 7.8 % (`for-in` key arrays) and 6.9 % (string concat). +/// +/// The campaign's 19:30 correction is the reason this counts executions rather +/// than bytes: a category's byte share bounds the collection *schedule* it can +/// move, and nothing else. The cost that a small category can still carry is +/// whatever runs per allocation — here, for `for-in`, a heap `String` and a +/// SipHash insert for **every key at every prototype level**, allocated only to +/// be hashed for shadowing and dropped. Those `String`s are native-heap, so +/// they are not even in the 7.8 %. +#[derive(Default)] +pub struct EnumDiag { + started: Option, + last_dump: Option, + events: u32, + /// Entries to `js_for_in_keys_value`. + pub for_in_calls: u64, + /// `for-in` calls that took the non-pointer (primitive receiver) path. + pub for_in_primitive: u64, + /// Prototype levels walked, summed over all calls. + pub for_in_levels: u64, + /// Key arrays materialised by the walk: one `js_object_keys_value` plus one + /// `js_object_get_own_property_names` per level. + pub for_in_key_arrays: u64, + /// Keys seen at any level — each one costs a `String` and a hash. + pub for_in_keys_seen: u64, + /// `String` allocations made by `key_string`. + pub for_in_key_strings: u64, + /// Bytes in those `String`s. + pub for_in_key_string_bytes: u64, + /// `seen.insert` calls (SipHash of the whole key each time). + pub for_in_seen_inserts: u64, + /// Of those, inserts that found the name already present — pure waste, the + /// name was already shadowed. + pub for_in_seen_dupes: u64, + /// Keys actually emitted into the result array. + pub for_in_keys_emitted: u64, + /// Of those, keys emitted at prototype level >= 1 — the only ones for which + /// the shadow set is load-bearing. If this is ~0, every `String` and every + /// hash spent building that set was spent for nothing. + pub for_in_keys_emitted_deep: u64, + /// Times the deferred shadow set was actually materialised. + pub for_in_shadow_built: u64, + /// String concatenations, by entry point. + pub concat_calls: u64, + pub concat_site_calls: u64, + pub concat_chain_calls: u64, + /// Bytes produced by concatenation. + pub concat_out_bytes: u64, +} + +crate::perry_thread_local! { + static ENUM_DIAG: RefCell = RefCell::new(EnumDiag::default()); +} + +/// Run `f` against this thread's enumeration counters, then maybe dump. +#[inline] +pub fn enum_with(f: impl FnOnce(&mut EnumDiag)) { + ENUM_DIAG.with(|d| { + let mut d = d.borrow_mut(); + if d.started.is_none() { + d.started = Some(Instant::now()); + d.last_dump = d.started; + } + f(&mut d); + d.events = d.events.wrapping_add(1); + if d.events % TICK_EVERY == 0 { + let due = d + .last_dump + .is_some_and(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS); + if due { + d.last_dump = Some(Instant::now()); + if let Some(sink) = enum_sink() { + write_sink(sink, &d.render()); + } + } + } + }); +} + +impl EnumDiag { + fn render(&self) -> String { + use std::fmt::Write as _; + let mut out = String::with_capacity(1024); + let per = |n: u64, d: u64| if d == 0 { 0.0 } else { n as f64 / d as f64 }; + let _ = writeln!( + out, + "[enum-diag] for_in calls={} (primitive={}) levels={} ({:.2}/call)", + self.for_in_calls, + self.for_in_primitive, + self.for_in_levels, + per(self.for_in_levels, self.for_in_calls) + ); + let _ = writeln!( + out, + " key arrays materialised={} ({:.2}/call) keys seen={} ({:.1}/call) emitted={} ({:.1}/call)", + self.for_in_key_arrays, + per(self.for_in_key_arrays, self.for_in_calls), + self.for_in_keys_seen, + per(self.for_in_keys_seen, self.for_in_calls), + self.for_in_keys_emitted, + per(self.for_in_keys_emitted, self.for_in_calls) + ); + let _ = writeln!( + out, + " PER-KEY WORK: String allocs={} ({:.2} MB) seen.insert={} of which duplicate={} ({:.1} %)", + self.for_in_key_strings, + self.for_in_key_string_bytes as f64 / (1024.0 * 1024.0), + self.for_in_seen_inserts, + self.for_in_seen_dupes, + 100.0 * per(self.for_in_seen_dupes, self.for_in_seen_inserts) + ); + let _ = writeln!( + out, + " emitted/String ratio = {:.3} (1.0 would mean every String earned a key)", + per(self.for_in_keys_emitted, self.for_in_key_strings) + ); + let _ = writeln!( + out, + " LOAD-BEARING: keys emitted at proto level >=1 = {} ({:.2} % of emitted); shadow set built {} times ({:.2}/call)", + self.for_in_keys_emitted_deep, + 100.0 * per(self.for_in_keys_emitted_deep, self.for_in_keys_emitted), + self.for_in_shadow_built, + per(self.for_in_shadow_built, self.for_in_calls) + ); + let _ = writeln!( + out, + "[enum-diag] concat calls={} site={} chain={} out_bytes={:.2} MB ({:.1} B/call)", + self.concat_calls, + self.concat_site_calls, + self.concat_chain_calls, + self.concat_out_bytes as f64 / (1024.0 * 1024.0), + per( + self.concat_out_bytes, + self.concat_calls + self.concat_site_calls + self.concat_chain_calls + ) + ); + out + } +} diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 37247daf70..1af9e016ac 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -203,6 +203,7 @@ mod array_retargeted_proto; mod buffer_own_prop; mod class_object_props; mod crypto_key; +pub(crate) mod entries_shape; pub(crate) mod enumeration; mod field_ops; mod for_in_stable; diff --git a/crates/perry-runtime/src/object/field_get_set/entries_shape.rs b/crates/perry-runtime/src/object/field_get_set/entries_shape.rs new file mode 100644 index 0000000000..0113826716 --- /dev/null +++ b/crates/perry-runtime/src/object/field_get_set/entries_shape.rs @@ -0,0 +1,224 @@ +//! `Object.entries` over the shape alone. +//! +//! Split out of `enumeration.rs` to keep it under the 2000-line size gate. + +use super::enumeration::*; +use super::*; + +/// [`js_object_entries`] over the shape alone. +pub(super) fn js_object_entries_shape(obj: *const ObjectHeader) -> *mut ArrayHeader { + // #8149: a registered BUFFER receiver — node `Buffer`, `Uint8Array`, + // `ArrayBuffer`, `SharedArrayBuffer` or `DataView`. Asked FIRST, above the + // `is_valid_obj_ptr` guard: a `BufferHeader` is not an `ObjectHeader`, and + // the generic walk below reads its payload bytes as `keys_array` — `[]` + // when they are zero, SIGBUS in `js_array_length` when they are not. + // See `registered_buffer_own_keys`. + if let Some(result) = registered_buffer_enum(strip_nanbox_addr(obj), MapSetEnum::Entries) { + return result; + } + let stripped = { + let bits = obj as u64; + let top16 = bits >> 48; + if top16 == 0x7FFD || top16 >= 0x7FF8 { + (bits & 0x0000_FFFF_FFFF_FFFF) as *const ObjectHeader + } else { + obj + } + }; + // Map/Set receiver → no own enumerable properties; see the matching + // guard in `js_object_keys` for the rationale. + if crate::map::is_registered_map(stripped as usize) + || crate::set::is_registered_set(stripped as usize) + { + return map_set_exotic_enum(stripped, MapSetEnum::Entries); + } + if let Some(addr) = + crate::typedarray_props::typed_array_addr_from_value(f64::from_bits(obj as u64)) + { + return unsafe { + crate::typedarray_props::typed_array_own_enumerable_entries( + addr as *const crate::typedarray::TypedArrayHeader, + ) + }; + } + if crate::typedarray::lookup_typed_array_kind(stripped as usize).is_some() { + return unsafe { + crate::typedarray_props::typed_array_own_enumerable_entries( + stripped as *const crate::typedarray::TypedArrayHeader, + ) + }; + } + // Arrays: emit [index, value] pairs for present elements, then named props. + // `js_object_entries` has no `ArrayHeader` layout, so the generic object + // path below would read an array's body as object fields and crash; handle + // arrays explicitly (mirrors the `js_object_keys` / `js_object_values` + // array branches). + if !stripped.is_null() && (stripped as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 { + unsafe { + let gc_header = (stripped as *const u8).sub(crate::gc::GC_HEADER_SIZE) + as *const crate::gc::GcHeader; + if (*gc_header).obj_type == crate::gc::GC_TYPE_ARRAY { + let arr = crate::array::clean_arr_ptr(stripped as *const crate::array::ArrayHeader); + let length = (*arr).length; + if length > 100_000 { + return crate::array::js_array_alloc(0); + } + let elements = (arr as *const u8) + .add(std::mem::size_of::()) + as *const u64; + let result = crate::array::js_array_alloc(length); + for i in 0..length { + if std::ptr::read(elements.add(i as usize)) == crate::value::TAG_HOLE { + continue; + } + let pair = crate::array::js_array_alloc(2); + let s = i.to_string(); + let key_box = crate::string::js_string_new_sso(s.as_ptr(), s.len() as u32); + crate::array::js_array_push_f64(pair, key_box); + let v = crate::array::js_array_get(arr, i); + crate::array::js_array_push_f64(pair, f64::from_bits(v.bits())); + crate::array::js_array_push_f64( + result, + crate::value::js_nanbox_pointer(pair as i64), + ); + } + for name in crate::array::array_named_property_names(arr, true) { + if let Some(v) = crate::array::array_named_property_get_by_name(arr, &name) { + let pair = crate::array::js_array_alloc(2); + let key = + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + crate::array::js_array_push(pair, JSValue::string_ptr(key)); + crate::array::js_array_push_f64(pair, v); + crate::array::js_array_push_f64( + result, + crate::value::js_nanbox_pointer(pair as i64), + ); + } + } + return result; + } + } + } + if obj.is_null() || !is_valid_obj_ptr(obj as *const u8) { + // Issue #893 lineage: chalk's `Object.entries(ansiStyles)` passed a + // value whose unboxed low-48 bits weren't a real heap pointer + // (cross-module import where the default-export wrapper hasn't + // finished initializing). Pre-fix the `crate::object::object_keys_array(obj)` deref + // SIGSEGV'd at 0x14; now we return an empty array so the user's + // `for (const [k, v] of Object.entries(undefined)) {}` no-ops the + // way the spec's "abstract conversion to object" path would for + // an unrecognized receiver. Real JS throws TypeError here; we + // prefer the empty-array fallback because Perry doesn't have a + // clean "throw at codegen-call boundaries" path for these + // pointer-typed entry points and a segfault is strictly worse + // for the caller. + return crate::array::js_array_alloc(0); + } + unsafe { + if let Some(result) = super::super::string_wrapper::enumerate( + obj, + super::super::string_wrapper::Enumeration::Entries, + ) { + return result; + } + if (*obj).class_id == NATIVE_MODULE_CLASS_ID { + if let Some(result) = native_module_enum(obj, MapSetEnum::Entries) { + return result; + } + } + let keys = crate::object::object_keys_array(obj); + // Iterate up to keys_len (the logical property count), not + // field_count. Parser-built and dict-built objects with ≥9 + // fields cap field_count at the inline alloc_limit (8) and + // store overflow values in OVERFLOW_FIELDS — for those, + // field_count under-counts the actual property count by N-8. + // Without this fix, `Object.entries(obj)` on a 50-key dict + // returned only the first 8 entries (silent data loss). + // Mirrors the same fix in `js_object_keys` and the + // `actual_fields = keys_len` line in `json.rs::stringify_object`. + let count = if !keys.is_null() { + crate::array::js_array_length(keys) as usize + } else { + crate::object::object_live_slot_count(obj) as usize + }; + let result = crate::array::js_array_alloc(count as u32); + + // #2438: emit pairs in OrdinaryOwnPropertyKeys order (array-index keys + // first, ascending; then string keys in insertion order). + let order = ecma_own_key_order(keys); + let pos = |j: usize| -> u32 { + match &order { + Some(ord) => ord[j], + None => j as u32, + } + }; + // Spec (EnumerableOwnProperties): the own key list is determined ONCE up + // front, then `[[Get]]` is invoked per key. A getter that adds, removes, + // or hides a future key during enumeration must not change the set of + // entries reported (test262 entries/getter-adding-key, + // getter-removing-future-key, getter-making-future-key-nonenumerable). + // + // Snapshot the own key *bytes* (not NaN-boxed pointers): a getter fired + // by `js_object_get_field_by_name` can delete a future key and + // allocate/GC before we visit it, and a key kept only inside this + // Rust-heap `Vec` is not a stack-visible GC root — it could dangle. + // Owning the bytes and rematerializing the string at read time sidesteps + // that. Enumerability is likewise re-evaluated per key in the read phase + // (an earlier getter can create a descriptor or flip a future key's + // enumerability), so we deliberately do NOT filter it during the snapshot. + let mut snapshot_keys: Vec> = Vec::with_capacity(count); + let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + for j in 0..count { + let i = pos(j); + if keys.is_null() || i >= crate::array::js_array_length(keys) { + continue; + } + let key_val = crate::array::js_array_get(keys, i); + if instance_private_key_hidden(obj, key_val) { + continue; + } + if let Some(bytes) = crate::string::js_string_key_bytes(key_val, &mut key_buf) { + snapshot_keys.push(bytes.to_vec()); + } + } + + for key_bytes in snapshot_keys { + let key_str = + crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + if key_str.is_null() { + continue; + } + // Spec EnumerableOwnProperties re-reads `[[GetOwnProperty]]` per key + // and skips it when the descriptor is now undefined or no longer + // enumerable — a getter earlier in the loop may have deleted or + // hidden a key that was in the initial snapshot (test262 + // entries/getter-removing-future-key, getter-making-future-key- + // nonenumerable). + if !super::super::own_key_present(obj as *mut ObjectHeader, key_str) { + continue; + } + if descriptor_marks_non_enumerable(obj, JSValue::string_ptr(key_str)) { + continue; + } + // Create a pair array [key, value]. + let pair = crate::array::js_array_alloc(2); + crate::array::js_array_push_f64( + pair, + f64::from_bits(JSValue::string_ptr(key_str).bits()), + ); + + // Read the value through the name-keyed `[[Get]]`, which fires an + // own accessor's getter (the raw index-based field read returned the + // empty data slot for accessor-defined properties — test262 + // entries/getter-adding-key expected the getter's "B"). + let value = js_object_get_field_by_name(obj as *const ObjectHeader, key_str); + crate::array::js_array_push_f64(pair, f64::from_bits(value.bits())); + + // Push the pair to result (NaN-box the array pointer) + let pair_boxed = crate::value::js_nanbox_pointer(pair as i64); + crate::array::js_array_push_f64(result, pair_boxed); + } + + result + } +} diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index 30098dd73d..397973a8e1 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -7,13 +7,16 @@ use super::*; /// own enumerable properties — Node: `Object.keys(new Map([...])) === []`), /// but user EXPANDOS (`cache.custom = x`) live in the exotic side table /// (`ExoticKind::Map`/`Set`). Shared by the keys/values/entries guards. -enum MapSetEnum { +pub(super) enum MapSetEnum { Keys, Values, Entries, } -fn map_set_exotic_enum(stripped: *const ObjectHeader, what: MapSetEnum) -> *mut ArrayHeader { +pub(super) fn map_set_exotic_enum( + stripped: *const ObjectHeader, + what: MapSetEnum, +) -> *mut ArrayHeader { let addr = stripped as usize; let kind = if crate::map::is_registered_map(addr) { super::super::exotic_expando::ExoticKind::Map @@ -62,7 +65,7 @@ fn map_set_exotic_enum(stripped: *const ObjectHeader, what: MapSetEnum) -> *mut /// module export tables. The receiver, key list, output, and per-key values /// are rooted because resolving a callable export can allocate and trigger a /// moving collection. -unsafe fn native_module_enum( +pub(super) unsafe fn native_module_enum( obj: *const ObjectHeader, what: MapSetEnum, ) -> Option<*mut ArrayHeader> { @@ -276,7 +279,18 @@ pub extern "C" fn js_object_keys_value(value: f64) -> *mut ArrayHeader { /// non-enumerable) as "seen" after emitting that level's enumerable subset. #[no_mangle] pub extern "C" fn js_for_in_keys_value(value: f64) -> *mut ArrayHeader { + for_in_keys_with(value, lazy_shadow_enabled()) +} + +/// The walk itself, with the shadow-set strategy as a parameter so a test can +/// run BOTH and assert they agree. `js_for_in_keys_value` reads the env once +/// and delegates here. +pub(crate) fn for_in_keys_with(value: f64, lazy_shadow: bool) -> *mut ArrayHeader { let jv = JSValue::from_bits(value.to_bits()); + let diag = crate::hot_diag::enum_on(); + if diag { + crate::hot_diag::enum_with(|d| d.for_in_calls += 1); + } if jv.is_null() || jv.is_undefined() { return crate::array::js_array_alloc(0); } @@ -284,6 +298,9 @@ pub extern "C" fn js_for_in_keys_value(value: f64) -> *mut ArrayHeader { // Non-pointer primitives (number/boolean, boxed string) have only their own // enumerable keys; every prototype property they inherit is non-enumerable. if !jv.is_pointer() { + if diag { + crate::hot_diag::enum_with(|d| d.for_in_primitive += 1); + } let own = js_object_keys_value(value); let n = crate::array::js_array_length(own); for i in 0..n { @@ -293,12 +310,53 @@ pub extern "C" fn js_for_in_keys_value(value: f64) -> *mut ArrayHeader { return out; } let key_string = |kv: JSValue, scratch: &mut [u8; crate::value::SHORT_STRING_MAX_LEN]| { - unsafe { crate::string::js_string_key_bytes(kv, scratch) } - .and_then(|b| std::str::from_utf8(b).ok().map(|s| s.to_string())) + let made = unsafe { crate::string::js_string_key_bytes(kv, scratch) } + .and_then(|b| std::str::from_utf8(b).ok().map(|s| s.to_string())); + if diag { + if let Some(ref s) = made { + let n = s.len() as u64; + crate::hot_diag::enum_with(|d| { + d.for_in_key_strings += 1; + d.for_in_key_string_bytes += n; + }); + } + } + made }; let mut seen: std::collections::HashSet = std::collections::HashSet::new(); let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let mut current = value; + + // #9792 follow-up: the shadow set is DEFERRED. + // + // `seen` exists for one purpose — a name owned at a closer level hides the + // same name further along the chain (§14.7.5 / 12.6.4-2). That filter can + // only ever apply to a level >= 1, so nothing at level 0 needs it, and a + // level that contributes no enumerable keys of its own never consults it. + // + // The old shape paid for it unconditionally: at EVERY level it materialised + // the all-own-names array (a second key array, including non-enumerable + // names) and turned every name at every level into a heap `String` so it + // could be hashed into the set. Measured on the compiled claude-code TUI, + // one 400-character reply: 17,272 `for-in` calls, 4.00 key arrays per call, + // and **159,752 `String` allocations and SipHash inserts to emit 11,276 + // keys** — an emitted/String ratio of 0.071, for 1.90 MB of bytes. The + // bytes are why no allocation-share ranking could see this; the executions + // are the cost. + // + // So: remember the levels walked, and build the set only at the moment a + // level >= 1 actually has an enumerable key to filter. When it is built it + // is built from exactly the levels already visited, which is the same + // content the eager version would have had at that point, so the emitted + // key sequence is unchanged. + // The levels walked so far, for the rebuild that almost never happens. + // Inline: the measurement says 2.00 prototype levels per call, so a spill + // to the heap is the pathological case, not the common one — and a `Vec` + // here would just reintroduce one malloc per `for-in` in place of the + // 159,947 this change removes. + let mut visited = VisitedLevels::default(); + let mut shadow_live = !lazy_shadow; + let mut level: u32 = 0; // Depth cap guards against pathological / cyclic prototype graphs. for _ in 0..1000 { let cv = JSValue::from_bits(current.to_bits()); @@ -309,34 +367,204 @@ pub extern "C" fn js_for_in_keys_value(value: f64) -> *mut ArrayHeader { // skipping any name already shadowed by a closer level. let enum_arr = js_object_keys_value(current); let en = crate::array::js_array_length(enum_arr); - for i in 0..en { - let kv = crate::array::js_array_get(enum_arr, i); - let name = match key_string(kv, &mut scratch) { - Some(s) => s, - None => continue, - }; - if seen.insert(name) { + if diag { + let en64 = en as u64; + crate::hot_diag::enum_with(|d| { + d.for_in_levels += 1; + d.for_in_key_arrays += 1; + d.for_in_keys_seen += en64; + }); + } + // Level 0 can be shadowed by nothing, so its own enumerable names go + // straight out — own property names are unique within one object, which + // is the only thing the set was doing for this level. + if lazy_shadow && level == 0 && !shadow_live { + for i in 0..en { + let kv = crate::array::js_array_get(enum_arr, i); out = crate::array::js_array_push_f64(out, f64::from_bits(kv.bits())); } - } - // Mark ALL own names (incl non-enumerable) seen so they shadow the - // remainder of the chain. - let all_f64 = super::super::descriptors::js_object_get_own_property_names(current); - let all_arr = (all_f64.to_bits() & crate::value::POINTER_MASK) as *mut ArrayHeader; - if !all_arr.is_null() { - let an = crate::array::js_array_length(all_arr); - for i in 0..an { - let kv = crate::array::js_array_get(all_arr, i); - if let Some(name) = key_string(kv, &mut scratch) { - seen.insert(name); + if diag { + let en64 = en as u64; + crate::hot_diag::enum_with(|d| d.for_in_keys_emitted += en64); + } + } else { + if en > 0 && !shadow_live { + // First level >= 1 with something to filter: pay for the set + // now, over exactly the levels already walked. + build_shadow_set(visited.as_slice(), &mut seen, &mut scratch, diag); + shadow_live = true; + if diag { + crate::hot_diag::enum_with(|d| d.for_in_shadow_built += 1); + } + } + for i in 0..en { + let kv = crate::array::js_array_get(enum_arr, i); + let name = match key_string(kv, &mut scratch) { + Some(s) => s, + None => continue, + }; + let fresh = seen.insert(name); + if diag { + let deep = level > 0; + crate::hot_diag::enum_with(|d| { + d.for_in_seen_inserts += 1; + if !fresh { + d.for_in_seen_dupes += 1; + } else { + d.for_in_keys_emitted += 1; + if deep { + d.for_in_keys_emitted_deep += 1; + } + } + }); + } + if fresh { + out = crate::array::js_array_push_f64(out, f64::from_bits(kv.bits())); } } } + // Mark ALL own names (incl non-enumerable) so they shadow the remainder + // of the chain — but only once the set is live. Until then the level is + // recorded and the array is not materialised at all: this is the second + // of the four key arrays per call that the measurement found. + if shadow_live { + mark_own_names(current, &mut seen, &mut scratch, diag); + } else { + visited.push(current); + } current = super::super::object_ops::js_object_get_prototype_of(current); + level += 1; } out } +/// Prototype levels recorded for a possible shadow-set rebuild, inline for the +/// depths that actually occur. +/// +/// `INLINE` is 8 against a measured 2.00 levels per `for-in` call on the +/// compiled claude-code TUI, so the heap arm is for prototype chains an order +/// of magnitude deeper than anything the workload produces. It exists because +/// the depth cap is 1000, not because it is expected. +struct VisitedLevels { + inline: [f64; Self::INLINE], + len: usize, + spill: Vec, +} + +impl Default for VisitedLevels { + fn default() -> Self { + Self { + inline: [0.0; Self::INLINE], + len: 0, + spill: Vec::new(), + } + } +} + +impl VisitedLevels { + const INLINE: usize = 8; + + fn push(&mut self, v: f64) { + if self.len < Self::INLINE { + self.inline[self.len] = v; + self.len += 1; + } else { + self.spill.push(v); + } + } + + /// The recorded levels in walk order. Borrows rather than copies, and the + /// spill arm concatenates only when it is non-empty. + fn as_slice(&self) -> VisitedSlice<'_> { + VisitedSlice { + head: &self.inline[..self.len], + tail: &self.spill, + } + } +} + +struct VisitedSlice<'a> { + head: &'a [f64], + tail: &'a [f64], +} + +impl VisitedSlice<'_> { + fn iter(&self) -> impl Iterator { + self.head.iter().chain(self.tail.iter()) + } +} + +/// `PERRY_FORIN_LAZY_SHADOW=0` restores the eager shadow set, so one binary +/// carries both paths and an A/B is one environment variable. +fn lazy_shadow_enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + !matches!( + std::env::var("PERRY_FORIN_LAZY_SHADOW").ok().as_deref(), + Some("0") | Some("off") | Some("false") | Some("no") + ) + }) +} + +/// Add every own name of `recv` — enumerable or not — to the shadow set. +fn mark_own_names( + recv: f64, + seen: &mut std::collections::HashSet, + scratch: &mut [u8; crate::value::SHORT_STRING_MAX_LEN], + diag: bool, +) { + let all_f64 = super::super::descriptors::js_object_get_own_property_names(recv); + let all_arr = (all_f64.to_bits() & crate::value::POINTER_MASK) as *mut ArrayHeader; + if all_arr.is_null() { + return; + } + let an = crate::array::js_array_length(all_arr); + if diag { + let an64 = an as u64; + crate::hot_diag::enum_with(|d| { + d.for_in_key_arrays += 1; + d.for_in_keys_seen += an64; + }); + } + for i in 0..an { + let kv = crate::array::js_array_get(all_arr, i); + let name = unsafe { crate::string::js_string_key_bytes(kv, scratch) } + .and_then(|b| std::str::from_utf8(b).ok().map(|s| s.to_string())); + if let Some(name) = name { + if diag { + let n = name.len() as u64; + crate::hot_diag::enum_with(|d| { + d.for_in_key_strings += 1; + d.for_in_key_string_bytes += n; + }); + } + let fresh = seen.insert(name); + if diag { + crate::hot_diag::enum_with(|d| { + d.for_in_seen_inserts += 1; + if !fresh { + d.for_in_seen_dupes += 1; + } + }); + } + } + } +} + +/// Materialise the shadow set for the levels already walked, in order. Called +/// at most once per `for-in`, and only when a level >= 1 has an enumerable key +/// that something closer might hide. +fn build_shadow_set( + visited: VisitedSlice<'_>, + seen: &mut std::collections::HashSet, + scratch: &mut [u8; crate::value::SHORT_STRING_MAX_LEN], + diag: bool, +) { + for recv in visited.iter() { + mark_own_names(*recv, seen, scratch, diag); + } +} + fn closure_dynamic_enumerable_props(ptr: usize) -> Vec<(String, f64)> { let mut props: Vec<(String, f64)> = Vec::new(); @@ -851,7 +1079,7 @@ pub(crate) unsafe fn keys_contain_array_index(keys: *const ArrayHeader) -> bool /// The raw heap address behind a possibly still-NaN-boxed `ObjectHeader` /// pointer, as the enumeration entry points receive it. #[inline] -fn strip_nanbox_addr(obj: *const ObjectHeader) -> usize { +pub(super) fn strip_nanbox_addr(obj: *const ObjectHeader) -> usize { let bits = obj as u64; let top16 = bits >> 48; if top16 == 0x7FFD || top16 >= 0x7FF8 { @@ -930,7 +1158,7 @@ pub(crate) fn registered_buffer_own_value(addr: usize, key: &str) -> f64 { /// Build the `Object.keys` / `.values` / `.entries` answer for a registered /// buffer from [`registered_buffer_own_keys`]. -fn registered_buffer_enum(addr: usize, what: MapSetEnum) -> Option<*mut ArrayHeader> { +pub(super) fn registered_buffer_enum(addr: usize, what: MapSetEnum) -> Option<*mut ArrayHeader> { if addr == 0 || !crate::buffer::is_registered_buffer(addr) { return None; } @@ -1618,220 +1846,8 @@ pub extern "C" fn js_object_entries(obj: *const ObjectHeader) -> *mut ArrayHeade js_object_entries_shape(obj) } -/// [`js_object_entries`] over the shape alone. -fn js_object_entries_shape(obj: *const ObjectHeader) -> *mut ArrayHeader { - // #8149: a registered BUFFER receiver — node `Buffer`, `Uint8Array`, - // `ArrayBuffer`, `SharedArrayBuffer` or `DataView`. Asked FIRST, above the - // `is_valid_obj_ptr` guard: a `BufferHeader` is not an `ObjectHeader`, and - // the generic walk below reads its payload bytes as `keys_array` — `[]` - // when they are zero, SIGBUS in `js_array_length` when they are not. - // See `registered_buffer_own_keys`. - if let Some(result) = registered_buffer_enum(strip_nanbox_addr(obj), MapSetEnum::Entries) { - return result; - } - let stripped = { - let bits = obj as u64; - let top16 = bits >> 48; - if top16 == 0x7FFD || top16 >= 0x7FF8 { - (bits & 0x0000_FFFF_FFFF_FFFF) as *const ObjectHeader - } else { - obj - } - }; - // Map/Set receiver → no own enumerable properties; see the matching - // guard in `js_object_keys` for the rationale. - if crate::map::is_registered_map(stripped as usize) - || crate::set::is_registered_set(stripped as usize) - { - return map_set_exotic_enum(stripped, MapSetEnum::Entries); - } - if let Some(addr) = - crate::typedarray_props::typed_array_addr_from_value(f64::from_bits(obj as u64)) - { - return unsafe { - crate::typedarray_props::typed_array_own_enumerable_entries( - addr as *const crate::typedarray::TypedArrayHeader, - ) - }; - } - if crate::typedarray::lookup_typed_array_kind(stripped as usize).is_some() { - return unsafe { - crate::typedarray_props::typed_array_own_enumerable_entries( - stripped as *const crate::typedarray::TypedArrayHeader, - ) - }; - } - // Arrays: emit [index, value] pairs for present elements, then named props. - // `js_object_entries` has no `ArrayHeader` layout, so the generic object - // path below would read an array's body as object fields and crash; handle - // arrays explicitly (mirrors the `js_object_keys` / `js_object_values` - // array branches). - if !stripped.is_null() && (stripped as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 { - unsafe { - let gc_header = (stripped as *const u8).sub(crate::gc::GC_HEADER_SIZE) - as *const crate::gc::GcHeader; - if (*gc_header).obj_type == crate::gc::GC_TYPE_ARRAY { - let arr = crate::array::clean_arr_ptr(stripped as *const crate::array::ArrayHeader); - let length = (*arr).length; - if length > 100_000 { - return crate::array::js_array_alloc(0); - } - let elements = (arr as *const u8) - .add(std::mem::size_of::()) - as *const u64; - let result = crate::array::js_array_alloc(length); - for i in 0..length { - if std::ptr::read(elements.add(i as usize)) == crate::value::TAG_HOLE { - continue; - } - let pair = crate::array::js_array_alloc(2); - let s = i.to_string(); - let key_box = crate::string::js_string_new_sso(s.as_ptr(), s.len() as u32); - crate::array::js_array_push_f64(pair, key_box); - let v = crate::array::js_array_get(arr, i); - crate::array::js_array_push_f64(pair, f64::from_bits(v.bits())); - crate::array::js_array_push_f64( - result, - crate::value::js_nanbox_pointer(pair as i64), - ); - } - for name in crate::array::array_named_property_names(arr, true) { - if let Some(v) = crate::array::array_named_property_get_by_name(arr, &name) { - let pair = crate::array::js_array_alloc(2); - let key = - crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); - crate::array::js_array_push(pair, JSValue::string_ptr(key)); - crate::array::js_array_push_f64(pair, v); - crate::array::js_array_push_f64( - result, - crate::value::js_nanbox_pointer(pair as i64), - ); - } - } - return result; - } - } - } - if obj.is_null() || !is_valid_obj_ptr(obj as *const u8) { - // Issue #893 lineage: chalk's `Object.entries(ansiStyles)` passed a - // value whose unboxed low-48 bits weren't a real heap pointer - // (cross-module import where the default-export wrapper hasn't - // finished initializing). Pre-fix the `crate::object::object_keys_array(obj)` deref - // SIGSEGV'd at 0x14; now we return an empty array so the user's - // `for (const [k, v] of Object.entries(undefined)) {}` no-ops the - // way the spec's "abstract conversion to object" path would for - // an unrecognized receiver. Real JS throws TypeError here; we - // prefer the empty-array fallback because Perry doesn't have a - // clean "throw at codegen-call boundaries" path for these - // pointer-typed entry points and a segfault is strictly worse - // for the caller. - return crate::array::js_array_alloc(0); - } - unsafe { - if let Some(result) = super::super::string_wrapper::enumerate( - obj, - super::super::string_wrapper::Enumeration::Entries, - ) { - return result; - } - if (*obj).class_id == NATIVE_MODULE_CLASS_ID { - if let Some(result) = native_module_enum(obj, MapSetEnum::Entries) { - return result; - } - } - let keys = crate::object::object_keys_array(obj); - // Iterate up to keys_len (the logical property count), not - // field_count. Parser-built and dict-built objects with ≥9 - // fields cap field_count at the inline alloc_limit (8) and - // store overflow values in OVERFLOW_FIELDS — for those, - // field_count under-counts the actual property count by N-8. - // Without this fix, `Object.entries(obj)` on a 50-key dict - // returned only the first 8 entries (silent data loss). - // Mirrors the same fix in `js_object_keys` and the - // `actual_fields = keys_len` line in `json.rs::stringify_object`. - let count = if !keys.is_null() { - crate::array::js_array_length(keys) as usize - } else { - crate::object::object_live_slot_count(obj) as usize - }; - let result = crate::array::js_array_alloc(count as u32); - - // #2438: emit pairs in OrdinaryOwnPropertyKeys order (array-index keys - // first, ascending; then string keys in insertion order). - let order = ecma_own_key_order(keys); - let pos = |j: usize| -> u32 { - match &order { - Some(ord) => ord[j], - None => j as u32, - } - }; - // Spec (EnumerableOwnProperties): the own key list is determined ONCE up - // front, then `[[Get]]` is invoked per key. A getter that adds, removes, - // or hides a future key during enumeration must not change the set of - // entries reported (test262 entries/getter-adding-key, - // getter-removing-future-key, getter-making-future-key-nonenumerable). - // - // Snapshot the own key *bytes* (not NaN-boxed pointers): a getter fired - // by `js_object_get_field_by_name` can delete a future key and - // allocate/GC before we visit it, and a key kept only inside this - // Rust-heap `Vec` is not a stack-visible GC root — it could dangle. - // Owning the bytes and rematerializing the string at read time sidesteps - // that. Enumerability is likewise re-evaluated per key in the read phase - // (an earlier getter can create a descriptor or flip a future key's - // enumerability), so we deliberately do NOT filter it during the snapshot. - let mut snapshot_keys: Vec> = Vec::with_capacity(count); - let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; - for j in 0..count { - let i = pos(j); - if keys.is_null() || i >= crate::array::js_array_length(keys) { - continue; - } - let key_val = crate::array::js_array_get(keys, i); - if instance_private_key_hidden(obj, key_val) { - continue; - } - if let Some(bytes) = crate::string::js_string_key_bytes(key_val, &mut key_buf) { - snapshot_keys.push(bytes.to_vec()); - } - } - - for key_bytes in snapshot_keys { - let key_str = - crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); - if key_str.is_null() { - continue; - } - // Spec EnumerableOwnProperties re-reads `[[GetOwnProperty]]` per key - // and skips it when the descriptor is now undefined or no longer - // enumerable — a getter earlier in the loop may have deleted or - // hidden a key that was in the initial snapshot (test262 - // entries/getter-removing-future-key, getter-making-future-key- - // nonenumerable). - if !super::super::own_key_present(obj as *mut ObjectHeader, key_str) { - continue; - } - if descriptor_marks_non_enumerable(obj, JSValue::string_ptr(key_str)) { - continue; - } - // Create a pair array [key, value]. - let pair = crate::array::js_array_alloc(2); - crate::array::js_array_push_f64( - pair, - f64::from_bits(JSValue::string_ptr(key_str).bits()), - ); - - // Read the value through the name-keyed `[[Get]]`, which fires an - // own accessor's getter (the raw index-based field read returned the - // empty data slot for accessor-defined properties — test262 - // entries/getter-adding-key expected the getter's "B"). - let value = js_object_get_field_by_name(obj as *const ObjectHeader, key_str); - crate::array::js_array_push_f64(pair, f64::from_bits(value.bits())); - - // Push the pair to result (NaN-box the array pointer) - let pair_boxed = crate::value::js_nanbox_pointer(pair as i64); - crate::array::js_array_push_f64(result, pair_boxed); - } +use super::entries_shape::js_object_entries_shape; - result - } -} +#[cfg(test)] +#[path = "enumeration_tests.rs"] +mod enumeration_tests; diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration_tests.rs b/crates/perry-runtime/src/object/field_get_set/enumeration_tests.rs new file mode 100644 index 0000000000..1e463c06af --- /dev/null +++ b/crates/perry-runtime/src/object/field_get_set/enumeration_tests.rs @@ -0,0 +1,188 @@ +//! Tests for `enumeration.rs`, split out to keep it under the 2000-line gate. + +use super::enumeration::*; + +#[cfg(test)] +mod lazy_shadow_tests { + use super::*; + + fn s(bytes: &str) -> *mut crate::StringHeader { + crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + } + + fn obj_value(o: *mut ObjectHeader) -> f64 { + f64::from_bits(JSValue::object_ptr(o as *mut u8).bits()) + } + + /// Read a key array back as owned strings, in order. + fn keys_of(arr: *mut ArrayHeader) -> Vec { + let mut out = Vec::new(); + let n = crate::array::js_array_length(arr); + let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + for i in 0..n { + let kv = crate::array::js_array_get(arr, i); + if let Some(b) = unsafe { crate::string::js_string_key_bytes(kv, &mut scratch) } { + if let Ok(t) = std::str::from_utf8(b) { + out.push(t.to_string()); + } + } + } + out + } + + /// The deferred shadow set must produce the SAME key sequence as the eager + /// one, including the case the deferral exists to skip and the case it + /// cannot skip. + /// + /// This is the assertion the optimisation lives or dies on: `for_in_keys_with` + /// is run both ways over the same object graph and the two key sequences are + /// compared element by element. Deleting the `build_shadow_set` call, or + /// emitting level 0 through the set instead of directly, makes the + /// shadowing case below disagree and fails this test by name. + #[test] + fn deferring_the_shadow_set_does_not_change_the_key_sequence() { + // 1. Flat object, prototype contributes nothing enumerable — the case + // the deferral is FOR. Both paths must agree. + let flat = crate::object::js_object_alloc(0, 0); + crate::object::js_object_set_field_by_name(flat, s("alpha"), 1.0); + crate::object::js_object_set_field_by_name(flat, s("beta"), 2.0); + let flat_v = obj_value(flat); + let lazy = keys_of(for_in_keys_with(flat_v, true)); + let eager = keys_of(for_in_keys_with(flat_v, false)); + assert_eq!( + lazy, eager, + "a flat object's for-in keys must not depend on when the shadow set is built" + ); + assert_eq!(lazy, vec!["alpha".to_string(), "beta".to_string()]); + + // 2. Prototype WITH enumerable keys, one of them shadowed by an own + // property. This is the case the shadow set exists for, so the + // deferred build must fire and produce the same answer. + let proto = crate::object::js_object_alloc(0, 0); + crate::object::js_object_set_field_by_name(proto, s("beta"), 20.0); + crate::object::js_object_set_field_by_name(proto, s("gamma"), 30.0); + let child = crate::object::js_object_alloc(0, 0); + crate::object::js_object_set_field_by_name(child, s("alpha"), 1.0); + crate::object::js_object_set_field_by_name(child, s("beta"), 2.0); + let child_v = obj_value(child); + crate::object::object_ops::js_object_set_prototype_of(child_v, obj_value(proto)); + + let lazy = keys_of(for_in_keys_with(child_v, true)); + let eager = keys_of(for_in_keys_with(child_v, false)); + assert_eq!( + lazy, eager, + "an inherited enumerable key, and an own key shadowing one on the \ + prototype, must come out identically whether the shadow set was \ + built eagerly or on demand" + ); + // `beta` is owned by the child, so it appears once, at the child's + // position — never again from the prototype. + assert_eq!( + lazy, + vec!["alpha".to_string(), "beta".to_string(), "gamma".to_string()], + "own keys first in insertion order, then unshadowed inherited ones" + ); + } + + /// A prototype chain deeper than `VisitedLevels::INLINE` where the ONLY + /// level that shadows the name lives PAST the inline array. + /// + /// This arm never runs on the measured workload (the shadow set was built 0 + /// times in 17,266 `for-in` calls), so a test is its only coverage. + /// + /// # Why this test is shaped the way it is — do not "simplify" it + /// + /// The obvious way to write it is to give EVERY level the shadowing + /// property, which reads as a stronger test and is not one. The first + /// version of this test did exactly that: `marker` was owned + /// non-enumerably at every level *including the leaf*. Deleting + /// `VisitedLevels`' spill arm — so a rebuild cannot see any level past + /// `INLINE` — left that test still passing, because the LEAF's own + /// `marker` was already in the set and shadowed the root's copy on its + /// own. The assertion was true no matter what the spill did, so it could + /// not fail, and it certified nothing. + /// + /// The fix is not more levels or more assertions, it is making the + /// spilled level the *only* thing that can produce the expected answer: + /// levels `0..INLINE` own nothing at all, exactly one level past the + /// inline array (`INLINE + 2`) owns `marker` non-enumerably, and only the + /// root owns it enumerably. Now dropping the spill leaks `marker` into the + /// result and the test fails by name — verified by making that edit. + /// + /// The general rule this is an instance of: after writing a test for a + /// rarely-taken path, delete the code it covers and check the test + /// actually fails. A test whose expected value is reachable by a second + /// route is measuring the second route. + #[test] + fn only_a_spilled_level_shadows_the_root_and_the_rebuild_must_see_it() { + let shadow_level = VisitedLevels::INLINE + 2; + let depth = shadow_level + 2; + + // Root (deepest): the enumerable `marker` that must stay hidden. + let root = crate::object::js_object_alloc(0, 0); + crate::object::js_object_set_field_by_name(root, s("marker"), 1.0); + crate::object::js_object_set_field_by_name(root, s("deep_only"), 2.0); + + // Build downwards from the root; `chain[i]` is at prototype level + // `depth - 1 - i` when walked from the leaf. + let mut chain = vec![root]; + for _ in 1..depth { + let o = crate::object::js_object_alloc(0, 0); + let ov = obj_value(o); + crate::object::object_ops::js_object_set_prototype_of( + ov, + obj_value(*chain.last().unwrap()), + ); + chain.push(o); + } + // Exactly one level shadows `marker`, and it is past the inline array. + let shadower = chain[depth - 1 - shadow_level]; + crate::object::js_object_set_field_by_name_nonenum(shadower, s("marker"), 0.0); + + let leaf_v = obj_value(*chain.last().unwrap()); + let lazy = keys_of(for_in_keys_with(leaf_v, true)); + let eager = keys_of(for_in_keys_with(leaf_v, false)); + assert_eq!( + lazy, eager, + "a chain whose only shadowing level spilled past the inline array \ + must give the same keys eagerly and on demand" + ); + assert!( + !lazy.contains(&"marker".to_string()), + "the only level owning `marker` sits past VisitedLevels::INLINE, so \ + a rebuild that cannot see the spilled levels would leak the root's \ + enumerable `marker` — got {lazy:?}" + ); + assert_eq!(lazy, vec!["deep_only".to_string()]); + } + + /// A NON-enumerable own property still shadows the same name on the + /// prototype (12.6.4-2). The deferred set only marks all-own-names for a + /// level once it goes live, so this is exactly where a wrong deferral would + /// leak the prototype's copy through. + #[test] + fn a_non_enumerable_own_name_still_shadows_the_prototype_under_deferral() { + let proto = crate::object::js_object_alloc(0, 0); + crate::object::js_object_set_field_by_name(proto, s("hidden"), 9.0); + crate::object::js_object_set_field_by_name(proto, s("shown"), 8.0); + + let child = crate::object::js_object_alloc(0, 0); + // Own but NOT enumerable: must not be emitted, must still shadow. + crate::object::js_object_set_field_by_name_nonenum(child, s("hidden"), 1.0); + let child_v = obj_value(child); + crate::object::object_ops::js_object_set_prototype_of(child_v, obj_value(proto)); + + let lazy = keys_of(for_in_keys_with(child_v, true)); + let eager = keys_of(for_in_keys_with(child_v, false)); + assert_eq!( + lazy, eager, + "deferral must not change shadowing by a non-enumerable own name" + ); + assert!( + !lazy.contains(&"hidden".to_string()), + "a non-enumerable own `hidden` must hide the prototype's enumerable \ + `hidden` rather than letting it through — got {lazy:?}" + ); + assert_eq!(lazy, vec!["shown".to_string()]); + } +} diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index 242f726e75..55910e1fd6 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -928,25 +928,16 @@ pub extern "C" fn js_object_get_field_by_name( return JSValue::undefined(); } } - // A primitive string receiver inherits `.constructor` from String.prototype: - // `"x".constructor === String` (test262 language/types/string/S8.4_A9/A12). - // The common string members (`.length`, indices, methods) are served by the - // codegen fast paths and never reach this generic slow path, so only the - // inherited `constructor` read needs routing here; resolve it to the same - // global `String` value bare-`String` yields so identity holds. - { - let bits = obj as u64; - if !key.is_null() && crate::value::JSValue::from_bits(bits).is_any_string() { - unsafe { - let key_ptr = (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - if std::slice::from_raw_parts(key_ptr, key_len) == b"constructor" { - let ctor = - super::super::js_get_global_this_builtin_value(b"String".as_ptr(), 6); - return JSValue::from_bits(ctor.to_bits()); - } - } - } + // A named read on a primitive string uses the same own/prototype lookup + // as a computed read, including SSO receivers and function identity. + if crate::value::JSValue::from_bits(obj as u64).is_any_string() { + return JSValue::from_bits( + crate::string::js_string_index_get_boxed( + f64::from_bits(obj as u64), + crate::value::js_nanbox_string(key as i64), + ) + .to_bits(), + ); } // Native module registry handles can arrive here either as raw small // integers or as POINTER_TAG-boxed small integers. Route them before any @@ -1639,34 +1630,6 @@ pub extern "C" fn js_object_get_field_by_name( } } } - // SSO property access (v0.5.213 Step 1 gate). The codegen inline - // `.length` path routes SHORT_STRING_TAG receivers here because - // it doesn't yet know about the SSO tag. Handle `.length` by - // reading the length byte directly from the NaN-box payload. - // Other property accesses on an SSO string (e.g. `.charAt` via - // `[0]`, `.slice`) aren't yet routed here — handled by the - // string method dispatch in a future migration step; today they - // fall through to "undefined" which matches the behavior for - // string-valued property access on untyped locals in general. - { - let obj_bits = obj as u64; - if (obj_bits & crate::value::TAG_MASK) == crate::value::SHORT_STRING_TAG { - if !key.is_null() { - unsafe { - let key_ptr = - (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - let key_bytes = std::slice::from_raw_parts(key_ptr, key_len); - if key_bytes == b"length" { - let len = (obj_bits & crate::value::SHORT_STRING_LEN_MASK) - >> crate::value::SHORT_STRING_LEN_SHIFT; - return JSValue::number(len as f64); - } - } - } - return JSValue::undefined(); - } - } // #1670: Web Streams handles are returned as `id as f64` (a normal // float, NOT NaN-boxed) just above the pointer-tagged small-handle band, so // an inline `res.body.locked` reaches this generic field-get with `obj` diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 10ea9cdae4..dfa7a37746 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -1002,14 +1002,6 @@ pub(crate) fn get_field_by_name_object_tail( let s = obj as *const crate::StringHeader; return JSValue::number((*s).utf16_len as f64); } - // A primitive string inherits `.constructor` from String.prototype: - // `"x".constructor === String` (test262 language/types/string/ - // S8.4_A9/A12). Resolve to the same global `String` value bare- - // `String` yields so identity holds — mirrors the Array branch above. - if key_bytes == b"constructor" { - let v = js_get_global_this_builtin_value(b"String".as_ptr(), 6); - return JSValue::from_bits(v.to_bits()); - } if let Some((kind, asym_type)) = crate::buffer::asymmetric_key_meta(obj as usize) { if key_bytes == b"type" { let label = if kind == 1 { @@ -1069,7 +1061,13 @@ pub(crate) fn get_field_by_name_object_tail( } } } - return JSValue::undefined(); + return JSValue::from_bits( + crate::string::js_string_index_get_boxed( + crate::value::js_nanbox_string(obj as i64), + crate::value::js_nanbox_string(key as i64), + ) + .to_bits(), + ); } // Maps/Sets: `.size`, expando keys, and prototype member values — // see `map_set_receiver.rs` (extracted for the file-size gate). diff --git a/crates/perry-runtime/src/object/polymorphic_index.rs b/crates/perry-runtime/src/object/polymorphic_index.rs index accf224d09..0468240fe0 100644 --- a/crates/perry-runtime/src/object/polymorphic_index.rs +++ b/crates/perry-runtime/src/object/polymorphic_index.rs @@ -273,7 +273,10 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> }; if gc_type == crate::gc::GC_TYPE_STRING { - return crate::string::js_string_index_get(raw as *const crate::StringHeader, idx); + return crate::string::js_string_index_get_boxed( + crate::value::js_nanbox_string(raw as i64), + idx, + ); } if let Some(index) = numeric_key_u32_index(idx) { diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 3be652b229..683af8750a 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -875,6 +875,32 @@ pub extern "C" fn js_regexp_new( let canonical_flags = validate_and_canonicalize_flags(raw_flags_str); let flags_str = canonical_flags.as_str(); + // ★ Share the caller's flags string when it is ALREADY the canonical text. + // + // `flags_ptr` used to be a fresh `js_string_from_str` on every + // construction. A JS regex literal evaluates to a fresh RegExp object + // every time it is reached, so that is one 32-byte GC string per + // evaluation: `PERRY_REGEX_DIAG` counts 161,897 constructions per + // 400-character claude-code reply, ~5.2 MB of identical one- and two-byte + // strings, and ~44 MB on a 3300-character reply. + // + // JS strings are immutable and have no identity semantics, and a literal's + // flags text is written by the author in spec order (`/x/gi`, not + // `/x/ig`), so the caller's string usually IS the canonical text and can + // simply be shared. Nothing downstream depends on the pointer being fresh: + // `flags_ptr`-keyed lookups (`FANCY_CACHE`, `lookup_fancy_regex`) read it + // through `string_as_str` and compare CONTENT, and the header keeping a + // pointer to it is what keeps it alive. + // + // This comparison must happen HERE, before the validation block below, + // because `raw_flags_str` borrows the caller's GC string and that block + // can allocate. The root is taken here for the same reason: the raw + // `flags` argument may name from-space after any allocation, exactly as + // the ★ note on `pattern_root` says, and this one is stored into the + // header too. + let shared_flags_root = + (is_valid_ptr(flags) && raw_flags_str == flags_str).then(|| scope.root_string_ptr(flags)); + let case_insensitive = flags_str.contains('i'); let global = flags_str.contains('g'); let multiline = flags_str.contains('m'); @@ -1034,11 +1060,21 @@ pub extern "C" fn js_regexp_new( // leaked every header, which was a 64-byte-per-call leak on top of the // (now-fixed) regex object leak. let header_size = std::mem::size_of::(); - // Materialize the canonical flags into a fresh StringHeader so that - // `flags_ptr`-keyed lookups (FANCY_CACHE, lookup_fancy_regex) and the - // GC-survivable source table all agree on the canonical form, and the - // header never holds the caller's possibly-temporary input flags. - let canonical_flags_ptr = js_string_from_str(flags_str); + // `flags_ptr` must hold the CANONICAL form, so that `flags_ptr`-keyed + // lookups (FANCY_CACHE, lookup_fancy_regex) and the GC-survivable source + // table all agree. When the caller's string already is that text it is + // shared (rooted above); only a non-canonical spelling (`/x/ig` → `"gi"`, + // or a computed `new RegExp(p, f)`) still has to materialize one. The + // counter makes the removal provable rather than asserted. + let flags_root = match shared_flags_root { + Some(root) => root, + None => { + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.new_flags_allocated += 1); + } + scope.root_string_ptr(js_string_from_str(flags_str)) + } + }; // ★ #7341: root the canonical flags string too. The `gc_malloc` below is an // allocation and therefore a collection point, exactly as the comment above // `pattern_root` says — but only the PATTERN was rooted and re-read. The @@ -1051,7 +1087,7 @@ pub extern "C" fn js_regexp_new( // // The write barrier below already treated this as a real GC edge; what was // missing is that the value written had to survive the allocation first. - let flags_root = scope.root_string_ptr(canonical_flags_ptr); + unsafe { let raw = crate::gc::gc_malloc(header_size, crate::gc::GC_TYPE_REGEXP); if raw.is_null() { diff --git a/crates/perry-runtime/src/regex/flags.rs b/crates/perry-runtime/src/regex/flags.rs index 606e414d6a..67f5e9caff 100644 --- a/crates/perry-runtime/src/regex/flags.rs +++ b/crates/perry-runtime/src/regex/flags.rs @@ -14,7 +14,28 @@ use super::throw_regexp_syntax_error; /// its set-notation matching semantics are not implemented (the regex crate /// has no equivalent); it behaves like an ordinary unicode pattern. #[cfg(feature = "regex-engine")] -pub(super) fn validate_and_canonicalize_flags(flags: &str) -> String { +/// The canonical flag text, held inline. +/// +/// There are eight legal flags and each may appear once, so the canonical form +/// is at most eight ASCII bytes and never needs the heap. It used to be a +/// `String`, i.e. one heap allocation on **every** `RegExp` construction — and +/// a JS regex literal constructs a fresh object every time it is evaluated, so +/// on the claude-code TUI that was ~162,000 allocations per 400-character +/// reply for text that is almost always one or two bytes. +#[derive(Clone, Copy)] +pub(super) struct CanonicalFlags { + buf: [u8; 8], + len: u8, +} + +impl CanonicalFlags { + pub(super) fn as_str(&self) -> &str { + // Every byte written below comes from `FLAG_ORDER`, which is ASCII. + std::str::from_utf8(&self.buf[..self.len as usize]).unwrap_or("") + } +} + +pub(super) fn validate_and_canonicalize_flags(flags: &str) -> CanonicalFlags { // Spec order of the flag bits: d g i m s u v y. const FLAG_ORDER: &[char] = &['d', 'g', 'i', 'm', 's', 'u', 'v', 'y']; let mut seen = [false; 8]; @@ -37,10 +58,15 @@ pub(super) fn validate_and_canonicalize_flags(flags: &str) -> String { } } } - FLAG_ORDER - .iter() - .enumerate() - .filter(|(i, _)| seen[*i]) - .map(|(_, c)| *c) - .collect() + let mut out = CanonicalFlags { + buf: [0; 8], + len: 0, + }; + for (i, c) in FLAG_ORDER.iter().enumerate() { + if seen[i] { + out.buf[out.len as usize] = *c as u8; + out.len += 1; + } + } + out } diff --git a/crates/perry-runtime/src/string/char_ops.rs b/crates/perry-runtime/src/string/char_ops.rs index 6ef47c734e..cccd3eb705 100644 --- a/crates/perry-runtime/src/string/char_ops.rs +++ b/crates/perry-runtime/src/string/char_ops.rs @@ -3,6 +3,9 @@ use super::*; +#[cfg(test)] +mod computed_property_tests; + /// JS index coercion for the String character-access methods (#2787). /// Applies `ToIntegerOrInfinity`: a non-numeric argument is first run through /// the full `ToNumber` (`js_number_coerce`) so an object index with a custom @@ -139,19 +142,66 @@ pub extern "C" fn js_string_index_get_boxed(value: f64, key: f64) -> f64 { const UNDEFINED: f64 = f64::from_bits(crate::value::TAG_UNDEFINED); let jsval = crate::value::JSValue::from_bits(value.to_bits()); if jsval.is_short_string() { + let scope = crate::gc::RuntimeHandleScope::new(); + let key = scope.root_nanbox_f64(key); let hdr = crate::string::js_string_materialize_to_heap(value); if hdr.is_null() { return UNDEFINED; } - return js_string_index_get(hdr, key); + let own = js_string_index_get(hdr, key.get_nanbox_f64()); + return if own.to_bits() != crate::value::TAG_UNDEFINED { + own + } else { + string_property_get_miss(value, key.get_nanbox_f64()) + }; } // Heap strings and every non-string receiver keep the existing behavior: // `js_string_index_get` already guards invalid pointers and delegates // non-string heap objects to the polymorphic index path. - js_string_index_get( + let own = js_string_index_get( (value.to_bits() & crate::value::POINTER_MASK) as *const StringHeader, key, - ) + ); + if !jsval.is_string() || own.to_bits() != crate::value::TAG_UNDEFINED { + return own; + } + string_property_get_miss(value, key) +} + +/// Complete a primitive string Get after its own character lookup misses. +/// Keep the raw index helper own-only: boxed strings also use it while walking +/// their own properties, before consulting their potentially custom prototype. +fn string_property_get_miss(value: f64, key: f64) -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(value); + let key = scope.root_nanbox_f64(key); + let key = + scope.root_nanbox_f64(unsafe { crate::object::js_to_property_key(key.get_nanbox_f64()) }); + // Object / bigint keys can coerce to an own index or to `length`. + if let Some(name) = crate::builtins::jsvalue_string_content(key.get_nanbox_f64()) { + let value = receiver.get_nanbox_f64(); + if name == "length" { + let bits = value.to_bits(); + if crate::value::JSValue::from_bits(bits).is_short_string() { + return ((bits & crate::value::SHORT_STRING_LEN_MASK) + >> crate::value::SHORT_STRING_LEN_SHIFT) as f64; + } + let string = (bits & crate::value::POINTER_MASK) as *const StringHeader; + return unsafe { (*string).utf16_len as f64 }; + } + if canonical_string_index(&name).is_some() { + let string = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; + let own = js_string_index_get(string, key.get_nanbox_f64()); + if own.to_bits() != crate::value::TAG_UNDEFINED { + return own; + } + } + } + // Reflect.get preserves the original function value (no binding wrapper), + // and gives inherited accessors the primitive as their receiver. Both + // operands stay rooted across lazy prototype creation and user code. + let prototype = crate::object::builtin_prototype_value("String"); + crate::proxy::js_reflect_get(prototype, key.get_nanbox_f64(), receiver.get_nanbox_f64()) } /// `s[key]` indexed read with ECMAScript CanonicalNumericIndexString semantics diff --git a/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs b/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs new file mode 100644 index 0000000000..6743921dbf --- /dev/null +++ b/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs @@ -0,0 +1,70 @@ +use super::*; +use crate::value::{js_dyn_index_get, js_nanbox_string, JSValue}; + +fn string(value: &str) -> f64 { + js_nanbox_string(js_string_from_str(value) as i64) +} + +#[test] +fn computed_string_method_is_the_prototype_function() { + let scope = crate::gc::RuntimeHandleScope::new(); + let proto = scope.root_nanbox_f64(crate::object::builtin_prototype_value("String")); + for receiver in [ + string("abcdef"), + f64::from_bits(JSValue::try_short_string(b"abc").unwrap().bits()), + ] { + let receiver = scope.root_nanbox_f64(receiver); + for name in ["charAt", "trim", "toUpperCase", "toString", "constructor"] { + let key = scope.root_nanbox_f64(string(name)); + let expected = scope.root_nanbox_f64(crate::proxy::js_reflect_get( + proto.get_nanbox_f64(), + key.get_nanbox_f64(), + proto.get_nanbox_f64(), + )); + assert_ne!( + expected.get_nanbox_f64().to_bits(), + crate::value::TAG_UNDEFINED, + "prototype has {name}" + ); + for get in [js_string_index_get_boxed, js_dyn_index_get] { + let actual = get(receiver.get_nanbox_f64(), key.get_nanbox_f64()); + assert_eq!( + actual.to_bits(), + expected.get_nanbox_f64().to_bits(), + "{name} must preserve method identity" + ); + } + } + } +} + +#[test] +fn computed_string_own_properties_keep_index_semantics() { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(string("abcdef")); + for get in [js_string_index_get_boxed, js_dyn_index_get] { + assert_eq!(get(receiver.get_nanbox_f64(), string("length")), 6.0); + for key in [1.0, string("1")] { + let value = get(receiver.get_nanbox_f64(), key); + assert_eq!( + crate::builtins::jsvalue_string_content(value).as_deref(), + Some("b") + ); + } + for key in [ + -1.0, + 1.5, + 6.0, + f64::NAN, + f64::INFINITY, + string("01"), + string("1.0"), + string("missing"), + ] { + assert_eq!( + get(receiver.get_nanbox_f64(), key).to_bits(), + crate::value::TAG_UNDEFINED + ); + } + } +} diff --git a/crates/perry-runtime/src/string/concat.rs b/crates/perry-runtime/src/string/concat.rs index 77d58367b2..800d942489 100644 --- a/crates/perry-runtime/src/string/concat.rs +++ b/crates/perry-runtime/src/string/concat.rs @@ -624,6 +624,9 @@ pub extern "C" fn js_string_concat( a: *const StringHeader, b: *const StringHeader, ) -> *mut StringHeader { + if crate::hot_diag::enum_on() { + crate::hot_diag::enum_with(|d| d.concat_calls += 1); + } let scope = crate::gc::RuntimeHandleScope::new(); let a_handle = scope.root_string_ptr(a); let b_handle = scope.root_string_ptr(b); @@ -980,6 +983,9 @@ const CONCAT_CHAIN_MAX_PARTS: usize = 32; /// with STRING_TAG via the standard `nanbox_string_inline` helper. #[no_mangle] pub extern "C" fn js_string_concat_chain(parts: *const f64, n: i32) -> *mut StringHeader { + if crate::hot_diag::enum_on() { + crate::hot_diag::enum_with(|d| d.concat_chain_calls += 1); + } let n = (n as usize).min(CONCAT_CHAIN_MAX_PARTS); if n == 0 || parts.is_null() { return crate::string::js_string_from_bytes(b"".as_ptr(), 0); diff --git a/crates/perry-runtime/src/string/concat_site.rs b/crates/perry-runtime/src/string/concat_site.rs index 239eb923b2..1679d6b9a6 100644 --- a/crates/perry-runtime/src/string/concat_site.rs +++ b/crates/perry-runtime/src/string/concat_site.rs @@ -76,6 +76,9 @@ pub extern "C" fn js_string_concat_site_value( prefix: *const StringHeader, value: f64, ) -> f64 { + if crate::hot_diag::enum_on() { + crate::hot_diag::enum_with(|d| d.concat_site_calls += 1); + } let slot = concat_site_slot(value); if let Some(k) = slot { let cached = unsafe { *table.add(k) }; diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 9d94ab7329..440ef47bda 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -168,6 +168,9 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { return js_dyn_index_get(boxed, index.get_nanbox_f64()); } let jsval = JSValue::from_bits(bits); + if jsval.is_any_string() { + return crate::string::js_string_index_get_boxed(value, index); + } // #5525: a Symbol *index* (`obj[Symbol.iterator]`) must resolve through the // symbol side-table, never the integer-index / stringify paths below (which // would coerce the symbol's NaN-boxed bits to a garbage i32). The codegen @@ -191,20 +194,6 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { } } } - if jsval.is_string() || jsval.is_short_string() { - // Spec: string INDEXING `s[i]` returns `undefined` for a non-canonical - // or out-of-bounds index — unlike `s.charAt(i)`, which returns "". - // Route through the canonical-index helper (`js_string_index_get`, - // #3987) so an OOB read here is `undefined`. Calling `js_string_char_at` - // directly (charAt semantics) returned "" for OOB, which every - // generator/async LOCAL string read hit: the CPS box pass erases the - // local's static type, so `line[i]` reaches this dyn path instead of the - // `is_string_expr` static path — the `yaml` lexer's `parseDocument` - // `switch (line[n])` then never observed `undefined` at line-ends and - // its `*lex` state machine spun forever (#6067). - let s_ptr = js_get_string_pointer_unified(value) as *const crate::StringHeader; - return crate::string::js_string_index_get(s_ptr, index); - } // Class-ref value (INT32-tagged, top16 == 0x7FFE): `C[key]` where `C` is a // runtime class-ref value (e.g. a function parameter). Member-expression // access (`C.key`) already routes through `js_object_get_field_by_name_f64`, diff --git a/docs/examples/README.md b/docs/examples/README.md index 15029951be..daba8a2e72 100644 --- a/docs/examples/README.md +++ b/docs/examples/README.md @@ -1,8 +1,9 @@ # Perry Doc Examples Every `.ts` file under this directory is a real, compilable program that is -verified by `cargo run -p perry-doc-tests` on every PR. Documentation pages in -`docs/src/` pull these files in via mdBook's `{{#include}}` directive, so the +verified by `cargo run -p perry-doc-tests` in the full doc-tests CI job. +Documentation pages in `docs/src/` pull these files in via mdBook's +`{{#include}}` directive, so the code you see on the rendered docs site is the same code CI is checking. ## Adding an example @@ -20,6 +21,18 @@ Runtime examples (non-UI) should list all three platforms. UI examples list whichever platforms their widgets support. The harness skips an example whose banner doesn't include the current host platform. +Examples that need specialized runtime libraries, such as Fastify's request +pump, add `// requires: auto-optimize` to the opening banner (within its first +15 lines). The harness removes `PERRY_NO_AUTO_OPTIMIZE` from that example's +compiler process, so it builds the required libraries and participates in the +normal pass/fail report. Other examples keep using the host runner's prebuilt +libraries. Unknown requirements fail discovery rather than silently being +ignored. + +Use `// run: false` for examples that need external services or run indefinitely. +They still compile and link; combining it with `requires: auto-optimize` tests +the specialized build without starting a server or connecting to a database. + 3. Reference it from markdown: ```markdown diff --git a/docs/examples/getting-started/npm_packages.ts b/docs/examples/getting-started/npm_packages.ts index 71eb348999..ea0852ed33 100644 --- a/docs/examples/getting-started/npm_packages.ts +++ b/docs/examples/getting-started/npm_packages.ts @@ -1,6 +1,7 @@ // demonstrates: importing built-in stdlib npm packages (project-config.md) // docs: docs/src/getting-started/project-config.md // platforms: macos, linux, windows +// requires: auto-optimize // run: false // These four imports are Perry's most-used built-in stdlib shims: diff --git a/docs/examples/stdlib/http/fastify_json.ts b/docs/examples/stdlib/http/fastify_json.ts index 7342617ba0..f9c6fc0378 100644 --- a/docs/examples/stdlib/http/fastify_json.ts +++ b/docs/examples/stdlib/http/fastify_json.ts @@ -11,6 +11,7 @@ // // docs: docs/src/stdlib/http.md // platforms: macos, linux, windows +// requires: auto-optimize // run: false import Fastify from "fastify" diff --git a/docs/examples/stdlib/overview/snippets.ts b/docs/examples/stdlib/overview/snippets.ts index 26632fb2c5..e0268e2fc1 100644 --- a/docs/examples/stdlib/overview/snippets.ts +++ b/docs/examples/stdlib/overview/snippets.ts @@ -2,6 +2,7 @@ // in docs/src/stdlib/overview.md // docs: docs/src/stdlib/overview.md // platforms: macos, linux, windows +// requires: auto-optimize // run: false // The overview page exists to show "these imports compile". So we just diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index b85fd44cf8..335968a3cc 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -188,6 +188,7 @@ - [Explicit Memory Control](internals/explicit-memory.md) - [The GC rooting invariant (codegen)](internals/gc-rooting-invariant.md) - [Local binding type evidence](internals/local-binding-type-evidence.md) +- [Codegen mechanisms and workload evidence](internals/codegen-mechanisms.md) - [Incremental GC step bounds](internals/gc-step-bounds.md) - [RFC: rooting by construction](internals/rfc-rooting-by-construction.md) - [Node-API host design](internals/node-api-host.md) diff --git a/docs/src/internals/codegen-mechanisms.md b/docs/src/internals/codegen-mechanisms.md new file mode 100644 index 0000000000..bc957119da --- /dev/null +++ b/docs/src/internals/codegen-mechanisms.md @@ -0,0 +1,90 @@ +# Codegen mechanisms and workload evidence + +An optimization's benchmark result does not establish that another program +uses the same generated path. Record the admission rule, the workload and +compiler snapshot, and evidence of emitted code before attributing a result +to that mechanism. + +The [mechanism index](https://github.com/PerryTS/perry/blob/main/scripts/codegen_mechanisms.json) +starts with the per-site concat cache from +[#9514](https://github.com/PerryTS/perry/pull/9514), following the correction in +[#9824](https://github.com/PerryTS/perry/issues/9824#issuecomment-5554137476). +It is an evidence index, not a complete inventory or a new CI gate. Missing +entries mean unrecorded. Each entry names its lowering, runtime helper, +admission conditions, workload expectations, observations, and existing +regression tests. Refresh observations when the compiler, bundle, flags, or +proof changes; a historical negative is not a permanent property of an app. + +## Per-site concat cache + +The [lowering](https://github.com/PerryTS/perry/blob/main/crates/perry-codegen/src/concat_site_cache.rs) +requires a string literal on the left in HIR and one of these right operands: + +| Right operand | Admission proof | +| --- | --- | +| Compile-time integer | Value in `0..=255`, including supported constant arithmetic and integer-constant locals. | +| Loop-induction local | Proven interval with a nonnegative lower bound and upper bound at most 255. | +| `x % C` | Compile-time integer modulus `C` in `1..=256`; negative remainders fall back at runtime. | + +The admission limit is **255**, but each site has only **32 slots**. At runtime, +an integral numeric value in `0..31` can hit a filled slot. The fill arm calls +`js_string_concat_site_value`; the plain arm handles values outside the table. +Ordered comparisons reject NaN and boxed non-numbers. An unproven site keeps +the ordinary fused helper and process-wide memo without this per-site diamond. +`PERRY_CONCAT_SITE_CACHE=0` disables the lowering **when compiling the program**. + +The counted-loop shape in +[`bench_object_property.ts`](https://github.com/PerryTS/perry/blob/main/benchmarks/suite/bench_object_property.ts) +is admitted: `"field_" + j` has `j` in `0..19`. Fresh object compilation with +the codegen at `d36a1af0c` produced three tables and three fill call sites; +disabling the cache produced none. Both builds retained the ordinary helper. +This confirms applicability; it does not add a timing measurement to #9514. + +The #9824 report found zero fill-helper executions in three runs of the +compiled `cli_2.1.112.js` bundle and no fill-helper symbol in its inspected +binary. Its roughly 8,600 concatenations per reply do not by themselves meet +the admission proof. The record therefore expects no emitted path for that +**reported snapshot**, not for every version of Claude Code. This is expected +workload selectivity, not evidence that the cache is broken. The rule also +admits constants and bounded remainders; it is not restricted to counted loops. + +## Recheck a workload + +From the repository root, using the compiler whose behavior you want to audit: + +```sh +PERRY_NO_CACHE=1 PERRY_NO_AUTO_OPTIMIZE=1 PERRY_LLVM_KEEP_IR=1 PERRY_CONCAT_SITE_CACHE=1 \ + perry compile benchmarks/suite/bench_object_property.ts \ + --no-link --keep-intermediates -o /tmp/concat-on.o 2>/tmp/concat-on.log +PERRY_NO_CACHE=1 PERRY_NO_AUTO_OPTIMIZE=1 PERRY_LLVM_KEEP_IR=1 \ + PERRY_CONCAT_SITE_CACHE=0 \ + perry compile benchmarks/suite/bench_object_property.ts \ + --no-link --keep-intermediates -o /tmp/concat-off.o 2>/tmp/concat-off.log +nm -u /tmp/concat-on.o | rg 'js_string_concat_site_value' +nm -u /tmp/concat-off.o | rg 'js_string_concat_site_value' +``` + +The last command should have no match (exit 1). `PERRY_NO_CACHE` forces fresh +code generation; `--no-link` makes this an object-level check. For a module +graph, inspect every generated object and every retained IR path in the log. + +Open the file named by each `kept LLVM IR:` log line. Count **call/invoke +instructions** to `@js_string_concat_site_value(` and definitions of +`@perry_concat_site_*` globals. A `declare` line is not a call site; searching +for the helper name alone gives a false positive. Confirm the disabled arm +still calls `js_string_concat_value_box` so the negative control is meaningful. + +Keep these evidence levels separate: + +- Retained pre-optimization IR shows whether codegen emitted the lowering. +- An object reference shows that a call survived compilation. Its absence + alone cannot distinguish non-emission from later dead-code elimination. +- Linked-binary symbol inspection must account for stripping and linkage. +- A counter at the helper entry measures fill-helper executions, not inline + cache hits. Zero executions alone does not prove the lowering was absent. + +The [existing compiler tests](https://github.com/PerryTS/perry/blob/main/crates/perry/tests/concat_site_cache.rs) +pin positive and negative admission, the disable switch, and Node parity under +evacuation. The [runtime lifecycle test](https://github.com/PerryTS/perry/blob/main/crates/perry-runtime/src/gc/tests/concat_site.rs) +checks that collection rewrites a filled slot. These cover the public admitted +shape even when the application bundle legitimately has no eligible sites. diff --git a/scripts/addr_class_ratchet_baseline.txt b/scripts/addr_class_ratchet_baseline.txt index 136e25806a..22ca960d27 100644 --- a/scripts/addr_class_ratchet_baseline.txt +++ b/scripts/addr_class_ratchet_baseline.txt @@ -113,7 +113,8 @@ handle-floor | crates/perry-runtime/src/object/delete_rest.rs | 1 handle-floor | crates/perry-runtime/src/object/descriptor_state.rs | 1 handle-floor | crates/perry-runtime/src/object/descriptors.rs | 1 handle-floor | crates/perry-runtime/src/object/field_get_set/accessors.rs | 2 -handle-floor | crates/perry-runtime/src/object/field_get_set/enumeration.rs | 4 +handle-floor | crates/perry-runtime/src/object/field_get_set/enumeration.rs | 3 +handle-floor | crates/perry-runtime/src/object/field_get_set/entries_shape.rs | 1 handle-floor | crates/perry-runtime/src/object/field_get_set/field_ops.rs | 3 handle-floor | crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs | 3 handle-floor | crates/perry-runtime/src/object/field_get_set/has_property.rs | 2 @@ -231,7 +232,8 @@ lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/construct.rs lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/prototype_objects.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/descriptors.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/accessors.rs | 2 -lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/enumeration.rs | 3 +lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/enumeration.rs | 2 +lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/entries_shape.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/field_ops.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/ic_miss.rs | 1 diff --git a/scripts/codegen_mechanisms.json b/scripts/codegen_mechanisms.json new file mode 100644 index 0000000000..2b1981e23b --- /dev/null +++ b/scripts/codegen_mechanisms.json @@ -0,0 +1,71 @@ +{ + "schema_version": 1, + "_comment": [ + "Workload applicability records for codegen mechanisms, starting with #9824.", + "This is an evidence index, not an exhaustive census or an automated admission gate.", + "An absent mechanism/workload is unrecorded, not evidence that a lowering never fires.", + "Update snapshot observations when the compiler, bundle, flags, or admission proof changes." + ], + "mechanisms": [ + { + "id": "concat_site_cache", + "introduced_by": "https://github.com/PerryTS/perry/pull/9514", + "codegen": "crates/perry-codegen/src/concat_site_cache.rs::try_lower_concat_site_cached", + "caller": "crates/perry-codegen/src/lower_string_concat.rs::coerce_concat_body", + "runtime": "crates/perry-runtime/src/string/concat_site.rs", + "fill_symbol": "js_string_concat_site_value", + "global_prefix": "perry_concat_site_", + "compile_time_disable": "PERRY_CONCAT_SITE_CACHE=0", + "admission": { + "left": "HIR Expr::String literal", + "right_alternatives": [ + "Compile-time integer in 0..=255, including supported constant arithmetic and integer-constant locals", + "LocalGet with a loop-induction interval whose lower bound is nonnegative and upper bound is at most 255", + "Remainder x % C with a compile-time integer C in 1..=256; negative remainders use the runtime plain arm" + ], + "maximum_proven_value": 255, + "cache_slots": 32, + "runtime_hit": "Integral numeric value in 0..31 with a filled slot; ordered comparisons reject NaN and boxed non-numbers" + }, + "workloads": [ + { + "id": "bench_object_property", + "source": "benchmarks/suite/bench_object_property.ts", + "expected_lowering": "emitted", + "why": "The field_ literal is concatenated with j bounded by FIELDS=20; the constant FIELDS-1 is admitted too.", + "observation": { + "date": "2026-09-05", + "codegen_revision": "d36a1af0c205ebdc7cf7f75b351ea34b5bc0fc0b", + "target": "aarch64-apple-darwin", + "method": "Fresh no-link compilation, retained pre-LLVM-optimization IR, and nm -u on the generated object", + "enabled": { "site_tables": 3, "fill_call_sites": 3, "object_fill_reference": true }, + "disabled": { "site_tables": 0, "fill_call_sites": 0, "object_fill_reference": false }, + "control": "Both arms retain calls to js_string_concat_value_box; these are code-presence observations, not execution counts or timing results." + } + }, + { + "id": "cc_parity_bundle", + "expected_lowering": "not_emitted_on_reported_snapshot", + "why": "The reported bundle has no retained sites meeting this admission proof; its concat volume alone does not imply eligibility.", + "observation": { + "date": "2026-09-05", + "provenance": "reported in issue #9824; not a fresh measurement made by this record", + "source": "https://github.com/PerryTS/perry/issues/9824#issuecomment-5554137476", + "bundle": "cli_2.1.112.js", + "compiler_snapshot": "c7361c87c plus perf/for-in-deferred-shadow-set (#9823), as reported", + "workload": "offline mock API, one 400-character streamed reply, chunk 100", + "runtime_fill_calls": [0, 0, 0], + "linked_binary_fill_symbol_present": false, + "limit": "No artifact hash or retained IR is supplied here. Symbol absence proves no surviving reference in the inspected artifact; inspect pre-optimization IR to distinguish non-emission from later elimination. Re-audit another compiler or bundle." + } + } + ], + "regression_tests": [ + "crates/perry/tests/concat_site_cache.rs::site_cache_fires_and_matches_node_under_evacuation", + "crates/perry/tests/concat_site_cache.rs::admission_follows_the_proven_bound", + "crates/perry/tests/concat_site_cache.rs::kill_switch_restores_the_plain_helper_and_stays_correct", + "crates/perry-runtime/src/gc/tests/concat_site.rs::test_filled_slot_is_rewritten_by_a_copied_minor" + ] + } + ] +} diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index d2dc1990d0..0100899021 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -276,7 +276,7 @@ "file": "crates/perry-runtime/src/gc/census.rs", "name": "PASS1_MARKED", "verdict": "non_moving_snapshot", - "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-pinned 2026-09-05 for the #9740 hot-TLS conversion of this file: the sole change is `thread_local!` \u2192 `crate::perry_thread_local!`, a macro-name swap with identical declaration syntax and `.with()` call sites. No control flow, no phase boundary, and no storage semantics change. Re-audited 2026-09-06 (train128) after #9794's GC diagnostics touched `gc/mod.rs` and `gc/policy.rs`: both gain diagnostic module declarations and counters only \u2014 no mark/sweep control flow, and the census bracketing in `step_mark_propagation` / `step_sweep` is unchanged. Re-audited for #9794's GC diagnostics: `gc/mod.rs` gains `mod diag_sites;` / `mod survival_diag;`, a re-export, a `diag_sites::full_started(...)` call at TRIGGER time (before mark propagation begins), and exit-time reporting. Nothing executes between mark-complete and sweep-entry, so the window is unchanged.", + "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-audited 2026-09-05 for the retained array-growth verifier fix: the only cycle.rs change passes the existing non-copying evacuation verifier an explicit all-forwarded policy. That call remains in minor finalization, outside the synchronous full-cycle census window; its root/heap reads do not allocate GC objects, move objects, or invoke JS callbacks. The mark-complete and sweep-entry boundaries are unchanged. (A union merge briefly restored an older branch's pins for these files; they are recomputed from the tree here. #9822 does not modify any window file \u2014 its diff against them is empty \u2014 so the audits already recorded above still stand.)", "window": { "start": { "file": "crates/perry-runtime/src/gc/census.rs", @@ -292,7 +292,7 @@ }, "sources": { "crates/perry-runtime/src/gc/census.rs": "388414f9629f196e84673e91bebd04bdcdcabdaa180252d2dfe4b82d1b49ca5a", - "crates/perry-runtime/src/gc/cycle.rs": "763d552271b8e983a796b4e9648cd8ee984a0602b2b56aeefdb8713c0049c31f", + "crates/perry-runtime/src/gc/cycle.rs": "2e2f5adca2229f74409e01a1cb571e2147cd8a33f58d0976711fce98d4777309", "crates/perry-runtime/src/gc/mod.rs": "7dd42b9506a97e6844fd3225dc53dfd59512631784750f58ff72208d68595481", "crates/perry-runtime/src/gc/policy.rs": "fa8e9fa188d50bd92c3fbe23950a195906baf387f3a208497791bdd23f1c72db", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" @@ -365,6 +365,12 @@ "verdict": "not_a_gc_pointer", "why": "#9717: monotonic count of array-growth forwarding stubs a budgeted full cycle admitted through `classifier_valid_object_start`, reported as `forwarded_stub_recoveries=` on the PERRY_GC_DIAG `[gc-incremental]` line. A `Cell` holding a tally, never an address \u2014 the stubs it counts are reached through the worklist, not retained here. Nothing for the collector." }, + { + "file": "crates/perry-runtime/src/hot_diag.rs", + "name": "ENUM_DIAG", + "verdict": "not_a_gc_pointer", + "why": "#9823 for-in diagnostics (`PERRY_ENUM_DIAG`), off unless armed. Two `Instant`s and a set of u64 counters (`for_in_calls`, `for_in_primitive`, `for_in_levels`, `for_in_key_arrays`, \u2026). No field stores an address." + }, { "file": "crates/perry-runtime/src/hot_diag.rs", "name": "IC_DIAG", diff --git a/scripts/string_payload_access_baseline.txt b/scripts/string_payload_access_baseline.txt index b9817beaa0..4622eaf4b0 100644 --- a/scripts/string_payload_access_baseline.txt +++ b/scripts/string_payload_access_baseline.txt @@ -12,7 +12,7 @@ inline-offset | perry-ext-nodemailer | 1 inline-offset | perry-ext-pg | 2 inline-offset | perry-ext-zlib | 3 inline-offset | perry-ffi | 3 -inline-offset | perry-runtime | 354 +inline-offset | perry-runtime | 352 inline-offset | perry-stdlib | 40 inline-offset | perry-updater | 5 reader-helper | perry-ext-ethers | 1 diff --git a/test-files/test_gap_9815_primitive_computed_properties.ts b/test-files/test_gap_9815_primitive_computed_properties.ts new file mode 100644 index 0000000000..28258f802b --- /dev/null +++ b/test-files/test_gap_9815_primitive_computed_properties.ts @@ -0,0 +1,67 @@ +// Dynamic value reads must return the original prototype method (#9815). +function typed(s: string, key: any): any { return s[key]; } +function unknown(s: any, key: any): any { return s[key]; } +function named(s: any, key: string): any { return s[key]; } + +for (const s of ["abcde", " abcdef ", "a" + "bc"]) { + for (const key of ["charAt", "trim", "toUpperCase", "toString", "constructor"]) { + const expected = String.prototype[key]; + console.log(key, typeof typed(s, key), typed(s, key) === expected, + unknown(s, key) === expected, named(s, key) === expected); + } + const charAt = typed(s, "charAt"); + const trim = unknown(s, "trim"); + const upper = named(s, "toUpperCase"); + console.log("borrowed", charAt.call(s, 1), trim.apply(s, []), upper.bind(s)()); + console.log("length", typed(s, "length"), unknown(s, "length"), named(s, "length")); + for (const key of [0, -0, 1, "1", -1, 1.5, 99, NaN, Infinity, "01", "1.0", "missing"]) { + console.log("index", String(key), typed(s, key), unknown(s, key)); + } +} + +const proto: any = String.prototype; +const sym = Symbol("computed"); +proto[sym] = 73; +proto.custom9815 = function () { "use strict"; return this; }; +proto["01"] = 81; +proto["99"] = 99; +proto["1"] = "shadowed"; +Object.defineProperty(proto, "get9815", { + configurable: true, + get: function () { "use strict"; return typeof this + ":" + this; } +}); +Object.defineProperty(Object.prototype, "inherited9815", { + configurable: true, + get: function () { "use strict"; return typeof this + ":" + this; } +}); + +for (const key of ["custom9815", "get9815", "inherited9815", "01", "99", "1", sym]) { + const value = typed("abc", key); + console.log("custom", typeof value, value === unknown("abc", key)); + if (typeof value === "function") console.log("receiver", value.call("xyz")); + else console.log("value", value); +} +let coercions = 0; +const indexKey = { toString() { coercions++; return "1"; } }; +const nameKey = { [Symbol.toPrimitive](hint) { coercions++; return "get9815"; } }; +const symbolKey = { [Symbol.toPrimitive](hint) { coercions++; return sym; } }; +console.log("coercion", typed("abc", indexKey), unknown("abc", nameKey), typed("abc", symbolKey), coercions); +console.log("bigint index", typed("abc", 1n)); +console.log("symbol identity", typed("abc", Symbol.iterator) === String.prototype[Symbol.iterator]); +const boxed: any = new String("abc"); +Object.setPrototypeOf(boxed, { custom9815: 42, "1": "shadowed" }); +console.log("boxed", unknown(boxed, "custom9815"), unknown(boxed, "1"), unknown(boxed, "length")); +const savedConstructor = proto.constructor; +proto.constructor = 91; +console.log("constructor override", typed("abcdef", "constructor"), named("abcdef", "constructor")); +proto.constructor = savedConstructor; +const callKey = "charAt"; +console.log("direct call", "abc"[callKey](1), "abc"["charAt"](1)); +delete proto[sym]; +delete proto.custom9815; +delete proto["01"]; +delete proto["99"]; +delete proto["1"]; +delete proto.get9815; +delete Object.prototype.inherited9815; +console.log("deleted", typed("abc", "custom9815"), typed("abc", "99"));