Add typed feedback runtime foundation - #1752
Merged
proggeramlug merged 5 commits intoMay 25, 2026
Merged
Conversation
proggeramlug
added a commit
that referenced
this pull request
May 25, 2026
…e LTO (#1764) (#1765) The typed-feedback instrumentation helpers (#1752) are #[no_mangle] but had no #[used] retention anchors. Nothing in the Rust crate graph references them — only codegen-emitted .ts object files do — so the auto-optimize build (whole- program thin-LTO + strip=true) internalized and dead-stripped them, dangling the codegen call at final link (`Undefined symbols: _js_typed_feedback_*`). This blocked linking for every instrumented program under the default build, including all node:async_hooks / AsyncLocalStorage tests. Add #[used] typed fn-pointer statics for all 23 js_typed_feedback_* exports — same retention mechanism as value/dyn_index.rs (KEEP_JS_DYN_INDEX_GET) / process.rs (KEEP_JS_SETENV, #1344). (A [usize;N] ptrtoint array or [*const ();N] pointer array do NOT survive thin-LTO; only individual typed fn-pointer statics keep the symbol external — verified via llvm-nm + the four async-context tests now linking and passing under auto-optimize.)
This was referenced May 25, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Jul 17, 2026
…pread calls (#6518) The #6486 family: js_array_grow moves a push-grown array and leaves a GC_FLAG_FORWARDED stub at the old address (#233), so a caller-held pre-grow pointer raw-dereferenced as (*arr).length reads the forwarding pointer's bytes as the element count. #6517 fixed the from-array construction paths; this closes out the remaining readers from the #6518 audit, each verified individually. Confirmed bugs (negative-control verified — reverting these two files crashes the new grown_array_crossing case): - thread.rs parallel_map_impl / parallel_filter_impl: parallelMap and parallelFilter on a grown array read raw (*arr).length after the NaN-box strip. - thread.rs serialize_array: a stale array pointer crossing the thread boundary (as an element of a crossed array, or as a worker's return value) serialized with a garbage length — the old GcHeader still reads GC_TYPE_ARRAY, so serialize_nanbox_for_thread dispatched straight into the raw read. Hardened (raw deref real, stale caller not currently reachable in-tree): value_call.rs js_closure_call_apply_with_spread — in-tree codegen pre-resolves the spread source via js_array_like_to_array (whose real-Array arm runs clean_arr_ptr), but the helper is no_mangle + stdlib-FFI-declared, so its contract accepts a raw handle; it now re-cleans rather than leaning on upstream cleaning for memory safety. Audited, no behavior change: typed_feedback.rs guards (plain_array_index_guard, numeric_array_push_guard) already reject GC_FLAG_FORWARDED headers before their raw length/capacity reads (since #1752), so stale grown pointers deterministically fall back to the chain-following slow path. Documented with an audit note marking the arm load-bearing. Tests: test_gap_6518_spread_call_grown_array.ts (node-parity, byte-identical) pins end-to-end spread-of-grown-array behavior; new grown_array_crossing case in run_thread_tests.sh covers the perry/thread paths node cannot mirror (fails pre-fix, passes post-fix). An unrelated pre-existing bug found while writing the repros (spawn capture of an async-fn local array crosses as empty) is filed as #6520.
proggeramlug
pushed a commit
that referenced
this pull request
Jul 17, 2026
…pread calls (#6518) The #6486 family: js_array_grow moves a push-grown array and leaves a GC_FLAG_FORWARDED stub at the old address (#233), so a caller-held pre-grow pointer raw-dereferenced as (*arr).length reads the forwarding pointer's bytes as the element count. #6517 fixed the from-array construction paths; this closes out the remaining readers from the #6518 audit, each verified individually. Confirmed bugs (negative-control verified — reverting these two files crashes the new grown_array_crossing case): - thread.rs parallel_map_impl / parallel_filter_impl: parallelMap and parallelFilter on a grown array read raw (*arr).length after the NaN-box strip. - thread.rs serialize_array: a stale array pointer crossing the thread boundary (as an element of a crossed array, or as a worker's return value) serialized with a garbage length — the old GcHeader still reads GC_TYPE_ARRAY, so serialize_nanbox_for_thread dispatched straight into the raw read. Hardened (raw deref real, stale caller not currently reachable in-tree): value_call.rs js_closure_call_apply_with_spread — in-tree codegen pre-resolves the spread source via js_array_like_to_array (whose real-Array arm runs clean_arr_ptr), but the helper is no_mangle + stdlib-FFI-declared, so its contract accepts a raw handle; it now re-cleans rather than leaning on upstream cleaning for memory safety. Element reads at all touched sites (serialize_array, the map/filter serialize loops, single_thread_map/filter, the spread arg copy) go through js_array_get_f64 instead of raw pointer walks, following the rule established in #6517 review: a sparse array (length > capacity, far slots in ARRAY_NAMED_PROPS) legally passes clean_arr_ptr, so walking length raw slots reads out of bounds. The accessor also normalizes the TAG_HOLE sentinel to undefined, which the raw copies leaked into arg buffers and cross-thread payloads. Audited, no behavior change: typed_feedback.rs guards (plain_array_index_guard, numeric_array_push_guard) already reject GC_FLAG_FORWARDED headers before their raw length/capacity reads (since #1752), so stale grown pointers deterministically fall back to the chain-following slow path. Documented with an audit note marking the arm load-bearing. Tests: test_gap_6518_spread_call_grown_array.ts (node-parity, byte-identical, incl. holes-spread-as-undefined) pins end-to-end spread-of-grown-array behavior; new grown_array_crossing case in run_thread_tests.sh covers the perry/thread paths node cannot mirror (fails pre-fix, passes post-fix), incl. holes crossing as undefined. An unrelated pre-existing bug found while writing the repros (spawn capture of an async-fn local array crosses as empty) is filed as #6520.
proggeramlug
added a commit
that referenced
this pull request
Jul 17, 2026
…pread calls (#6518) (#6521) * fix(runtime): resolve array forwarding stubs in thread crossing and spread calls (#6518) The #6486 family: js_array_grow moves a push-grown array and leaves a GC_FLAG_FORWARDED stub at the old address (#233), so a caller-held pre-grow pointer raw-dereferenced as (*arr).length reads the forwarding pointer's bytes as the element count. #6517 fixed the from-array construction paths; this closes out the remaining readers from the #6518 audit, each verified individually. Confirmed bugs (negative-control verified — reverting these two files crashes the new grown_array_crossing case): - thread.rs parallel_map_impl / parallel_filter_impl: parallelMap and parallelFilter on a grown array read raw (*arr).length after the NaN-box strip. - thread.rs serialize_array: a stale array pointer crossing the thread boundary (as an element of a crossed array, or as a worker's return value) serialized with a garbage length — the old GcHeader still reads GC_TYPE_ARRAY, so serialize_nanbox_for_thread dispatched straight into the raw read. Hardened (raw deref real, stale caller not currently reachable in-tree): value_call.rs js_closure_call_apply_with_spread — in-tree codegen pre-resolves the spread source via js_array_like_to_array (whose real-Array arm runs clean_arr_ptr), but the helper is no_mangle + stdlib-FFI-declared, so its contract accepts a raw handle; it now re-cleans rather than leaning on upstream cleaning for memory safety. Element reads at all touched sites (serialize_array, the map/filter serialize loops, single_thread_map/filter, the spread arg copy) go through js_array_get_f64 instead of raw pointer walks, following the rule established in #6517 review: a sparse array (length > capacity, far slots in ARRAY_NAMED_PROPS) legally passes clean_arr_ptr, so walking length raw slots reads out of bounds. The accessor also normalizes the TAG_HOLE sentinel to undefined, which the raw copies leaked into arg buffers and cross-thread payloads. Audited, no behavior change: typed_feedback.rs guards (plain_array_index_guard, numeric_array_push_guard) already reject GC_FLAG_FORWARDED headers before their raw length/capacity reads (since #1752), so stale grown pointers deterministically fall back to the chain-following slow path. Documented with an audit note marking the arm load-bearing. Tests: test_gap_6518_spread_call_grown_array.ts (node-parity, byte-identical, incl. holes-spread-as-undefined) pins end-to-end spread-of-grown-array behavior; new grown_array_crossing case in run_thread_tests.sh covers the perry/thread paths node cannot mirror (fails pre-fix, passes post-fix), incl. holes crossing as undefined. An unrelated pre-existing bug found while writing the repros (spawn capture of an async-fn local array crosses as empty) is filed as #6520. * fix(runtime): root the closure across parallelMap/parallelFilter GC points (#6521 review) CodeRabbit flagged that single_thread_map/single_thread_filter root the input array and re-derive it every iteration (the callback can trigger a moving minor GC) but passed the closure as a raw unrooted pointer — a moved closure left later iterations calling through a dangling capture block. Root it alongside the array (before the result allocation, which is itself a GC point) and re-derive it per iteration, mirroring util_promisify's closure-rooting discipline. Also hardened the impl-level ordering the review didn't flag: in parallel_map_impl/parallel_filter_impl the closure func_ptr deref and downstream capture-serialization derefs sat AFTER the clean_arr_ptr call this PR added, and clean_arr_ptr can force-materialize a lazy array — a GC point. Validate + root the closure first, resolve the array, then re-derive the closure from its handle. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 1 of the typed-runtime follow-up stack. This adds the runtime and codegen foundation for typed feedback sites without enabling the larger guarded numeric layout work yet.
Stack
codex/guarded-dispatch-specializations)codex/gc-1096-numeric-layouts)This PR is an enabling slice for #1096, but does not close it by itself.
Verification
cargo check -p perry-runtime -p perry-codegenon this slicecargo check -p perry-runtime -p perry-codegenExisting warning noise remains.