Repository navigation
gc tests: root the key tables in the residual prototype tests (#12137) - #12138
Merged
Merged
Conversation
The copying nursery guard starts with no root scanners. These tests call Object.setPrototypeOf and Object.getPrototypeOf on builtin owners, which build intrinsics inside the window and mint key atoms in the string atom table. A copying minor then freed those atoms while the table still named them. The next intrinsic build used a freed address as a key, and a key list read a garbage length past the end of its 2 MiB block (#12137). Register the atom and intern table, shape table and canonical keys scanners, as production does.
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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
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.
Closes #12137.
This was a test isolation bug, not a region-manager bug.
CopyingNurseryTestGuardstarts with an empty root-scanner set. Inside that window the residual-prototype tests callObject.setPrototypeOf/getPrototypeOfon builtin owners, which lazily builds intrinsics and creates key atoms in the thread's atom table (ATOMS, string/intern.rs).scan_intern_table_roots_mut(intern.rs:589), but the window never registered it. A copying minor freed the atoms while the table still named them.populate_global_this_builtins→canonicalize) then published a canonical key list with freed addresses as keys.key_bytes_hashread a garbage length of about 2 MB and ran off the end of its block into the gap after the 2 MiB region. Since gc: Perry-owned memory for GC arena blocks (2 MiB OS regions, huge pages by complete units) #12132 that gap faults; before, the same read was probably a silent garbage read.The fix registers the atom/intern, shape-table rekey and canonical-keys scanners in these tests, the same set
runtime_roots/function_attrs_old_minor.rsuses.Verification:
gc::testssequence ran 10 times with no SIGSEGV.z8_churnwas skipped because it hits debug builds: z8 stream churn test panics with 'attempt to subtract with overflow' at array/storage.rs:32 #12136 first.perex_lifecycle.rs:247andregexp_last_index.rs:211.Separate note for the shape owner, not fixed here and not shown to cause anything in production: the canonical-keys trie is weak, and young canonical key lists are only pruned at a full collection (
young_prune: Noneat gc/dead_owner.rs:434). A list that dies in a minor stays in the trie until the next full collection.