chore(lint): allowlist guarded GcHeader probe in collection_proto_thunks (#6659) - #6722
Conversation
…_thunks (PerryTS#6659) PerryTS#6659 (fac2bbe) added a `(ptr as *const u8).sub(GC_HEADER_SIZE) as *const GcHeader` obj_type probe in collection_proto_thunks.rs without an addr-class allowlist entry, so the address-classification audit (the `lint` gate) went red on main and on every open PR branched from it. The probe is correctly magnitude-classified — it runs only inside `if is_above_handle_band(ptr) && is_valid_obj_ptr(ptr as *const u8)`, so the pointer is never a handle-band small integer at the deref. Add the standard allowlist entry (same shape as the sibling object/* probes) with a follow-up note to migrate it to `addr_class::try_read_gc_header`. Restores lint to green; no functional change.
📝 WalkthroughWalkthroughAdds one address-classification allowlist entry documenting a guarded ChangesAddress classification
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/addr_class_allowlist.txt`:
- Line 139: Update the allowlist entry for collection_proto_thunks.rs to replace
the broad “*” match with the exact GcHeader cast/probe substring being
grandfathered. Preserve the existing guard context and comment, while ensuring
future GcHeader casts in that file are not automatically allowlisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95fd6449-ccac-4704-be3e-dcd68000f2bc
📒 Files selected for processing (1)
scripts/addr_class_allowlist.txt
| crates/perry-runtime/src/object/class_registry/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of class_registry.rs) | ||
| crates/perry-runtime/src/object/descriptor_state.rs | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of object/mod.rs) | ||
| crates/perry-runtime/src/object/to_string_tag.rs | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of object/mod.rs) | ||
| crates/perry-runtime/src/object/collection_proto_thunks.rs | * | GcHeader obj_type probe guarded by is_above_handle_band + is_valid_obj_ptr (magnitude-classified before the deref); migrate to addr_class::try_read_gc_header in a follow-up (added in #6659) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Narrow the allowlist match to the specific probe.
Using * suppresses every future GcHeader-cast finding in collection_proto_thunks.rs, not just the guarded probe being grandfathered. Replace the wildcard with the exact cast substring so newly added unsafe probes cannot bypass the audit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/addr_class_allowlist.txt` at line 139, Update the allowlist entry for
collection_proto_thunks.rs to replace the broad “*” match with the exact
GcHeader cast/probe substring being grandfathered. Preserve the existing guard
context and comment, while ensuring future GcHeader casts in that file are not
automatically allowlisted.
Problem
The
lintgate is red onmainand on every open PR (verified: #6721, #6720, #6717, #6712, #6711 all faillintidentically). The failing step is the address-classification audit (scripts/addr_class_inventory.py):This
GcHeaderobj_type probe was added by #6659 (fac2bbead, merged 2026-07-19) without an allowlist entry, so the ratchet has been failing lint for every PR branched from that point.Why an allowlist entry is the right fix
The cast is already correctly magnitude-classified — it runs only inside:
So
ptris never a handle-band small integer at the deref — exactly the invariant the ratchet exists to enforce. This is the same shape as the siblingobject/*probes already in the allowlist, so I added one line with a follow-up note to migrate it toaddr_class::try_read_gc_header.Change
One line in
scripts/addr_class_allowlist.txt. No functional change.Verification
All five
lintsteps pass locally on this branch:cargo fmt --all -- --check, benchmark-freshness,check_file_size.sh, GC store-site inventory, and the address-classification audit (self-test+ full run, exit 0). Restoreslintto green, which unblocks all currently-open PRs.No version bump / changelog — maintainer folds metadata at merge.
🤖 Generated with Claude Code
Summary by CodeRabbit