fix(gc): re-read imported-ctor args, root the key-add receiver, refuse ConstFn finalize on a keys-identity miss - #11783
Merged
Merged
Conversation
added 2 commits
October 3, 2026 08:04
…e ConstFn finalize on a keys-identity miss Three findings from the #11680 audit. H1: an imported constructor's positional arguments and the values pushed into its rest/arguments arrays were register snapshots taken before js_array_alloc, the pushes and the class-value lookup. ImportedCtorArg now names a group operand, every push re-reads its element from its root, and the dispatch re-reads every positional argument. The root_reload pass was masking this in the emitted IR, so the test now also runs with that pass off (test-only seam) and checks every pushed value and the positional operand against each collecting call. M2: publish_key_add_edge wrote the ConstFn prewrite, the SPECIAL stamp and the convergence through the pre-mint receiver. The GC call-effects graph cannot prove a mint non-collecting, so the receiver is rooted and re-read after each publication. The finalizer comment that called the mint non-collecting is corrected. The test arms a cfg(test) collection after each publication under forced evacuation and the evacuation verifier. M1: the static ConstFn finalizer aborted the process when a receiver had the same key names as the record seeded under the requested id but a different keys array. It now compares the complete record, keys identity included, before minting: a match is stamped and anything else is a refusal that leaves the receiver untouched.
proggeramlug
added
the
run-extended-tests
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (9)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
GC-safety and crash fixes from the audit of #11680.
H1: imported-constructor arguments. Positional arguments and the values pushed into rest/
argumentsarrays were read beforejs_array_allocandjs_class_value, both of which can move objects. The whole-functionroot_reloadpass happened to re-read them in the emitted IR, which is why nothing broke so far and why the old test couldn't fail. Each positional argument is now named by its index in the root group and re-read at dispatch, and each pushed value is re-read before its push. The test checks every use against every collecting call, compiling withroot_reloaddisabled (a#[cfg(test)]switch) so it exercises the lowering itself.M2:
publish_key_add_edge. The gc_call_effects graph shows the key-add mint can collect: for exampleset_object_keys_with_live_rep→js_array_length→js_number_coerce. The receiver was still written through a raw pointer after the mint. It is now rooted and re-read after each publish. The false "cannot collect" comment in the finalizer is corrected. The test uses a#[cfg(test)]hook that collects after each publish, runs under forced evacuation with the verifier, and checks the moved receiver.M1: ConstFn finalizer abort. A receiver with the same key names as the seeded record but a different keys array aborted the process, despite the doc saying refusals leave the receiver untouched. The finalizer now compares the whole record, including keys-array identity, before minting under the requested id, and otherwise refuses. Covered by a child-process test.
Sabotage: reverting each fix turns its own test red.
Verification
RUST_TEST_THREADS=1, perry-dev)manifest_consistency, which fails on main since #11757new/ctor/classM3, unifying the ConstFn admission rules, is left for a follow-up: it needs a design decision about the closure-value exclusions.
Summary by CodeRabbit
argumentsarrays.