Skip to content

fix(gates): restore addr-class lint, broken on main by #7579 - #7585

Merged
proggeramlug merged 2 commits into
mainfrom
fix/addr-class-iter-result
Aug 7, 2026
Merged

fix(gates): restore addr-class lint, broken on main by #7579#7585
proggeramlug merged 2 commits into
mainfrom
fix/addr-class-iter-result

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

lint has been red on main — and therefore on every open PR — since #7579 merged. My fault: I audited that PR against raw_handle_debt, check_file_size and cargo fmt, but not addr_class_inventory.py, which CLAUDE.md lists as a gate. Found by an agent working an unrelated issue in the same file.

crates/perry-runtime/src/iter_result.rs:139: [gcheader-cast]
  let gc_header = (keys as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader;

The fix, and why not an allowlist entry

The obvious repair is a 127th entry in scripts/addr_class_allowlist.txt. I didn't take it. Every one of the 126 grandfathered entries carries the same sentence"migrate to addr_class::try_read_gc_header in a follow-up" — so adding another is borrowing against a debt nobody is paying down.

The reason none of them has migrated is structural, not neglect: try_read_gc_header returns &'static GcHeader, a shared reference. That is deliberate — it is the safe probe for an address that might be a handle, so it must not be able to write. These call sites all want to set a flag. No amount of follow-up work makes a shared reference serve a *mut write; they needed a second, narrower entry point that never existed.

So this adds it: gc::mark_shape_shared, in gc/ where the cast is permitted, with the precondition stated — the pointer must come from allocation this thread just performed, never from a NaN-box payload, which is the same discipline the arena walkers are already allowlisted under. iter_result.rs calls it and the cast is gone. The allowlist does not grow, and the other sites now have a real migration target.

Also

Drops one stale ratchet entry the tool itself reported as over-counted (field_get_set.rs lone-valid-obj-ptr: baseline says 1, found 0). Same family as #7582 — a suppression that outlived its subject.

Validation

  • addr_class_inventory.py passes (885 files, 267 allowlisted, 542 sites held by the ratchet).
  • cargo test -p perry-runtime --lib: 1818 passed, 0 failed, including shared_iter_result_keys_are_marked_copy_on_write, which is the test that guards this exact flag write.
  • raw_handle_debt 998 (baseline 998), check_file_size.sh clean, cargo fmt --check clean.

CI has a deep backlog and may not report; this is local validation.

Summary by CodeRabbit

  • Bug Fixes

    • Improved runtime handling of shared iterator-result data to support safer memory management and reduce the risk of related issues.
  • Documentation

    • Added release documentation for the shared-data memory-management update.
  • Chores

    • Updated the application version from 0.5.1328 to 0.5.1329.

#7579 added a bare `as *mut GcHeader` cast in iter_result.rs to stamp the
shared keys array copy-on-write. addr_class_inventory.py refuses that outside
gc/ and value/addr_class.rs, so `lint` has been red on main and on every open
PR since it merged.

Rather than add a 127th allowlist entry, move the cast into gc/ behind
`mark_shape_shared`. Every grandfathered entry in the allowlist carries the
same promise -- 'migrate to a helper in a follow-up' -- and this is that helper
for the one thing those sites actually do: set a flag.

addr_class::try_read_gc_header cannot serve them, and that is by design rather
than an oversight: it returns `&'static GcHeader`, a SHARED reference,
precisely so a probe of an untrusted address can never write through it. A flag
write needs `*mut`, hence a narrower entry point with a stronger precondition.

Also drops one stale addr-class ratchet entry the tool reported as over-counted
(field_get_set.rs lone-valid-obj-ptr: baseline 1, found 0).
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e69639ec-fb5e-4fa4-a0d5-ca8f00435552

📥 Commits

Reviewing files that changed from the base of the PR and between 9688bbe and ebf1d0c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • CLAUDE.md
  • Cargo.toml
  • changelog.d/7585-addr-class-mark-shape-shared.md
  • crates/perry-runtime/src/gc/mod.rs
  • crates/perry-runtime/src/iter_result.rs
  • scripts/addr_class_ratchet_baseline.txt

📝 Walkthrough

Walkthrough

The runtime adds gc::mark_shape_shared for shared-object header marking and updates iterator-result handling to use it. The address-class baseline and changelog are updated. The workspace and documented versions advance to 0.5.1329.

Changes

Shared-shape GC metadata

Layer / File(s) Summary
GC header marking helper
crates/perry-runtime/src/gc/mod.rs, crates/perry-runtime/src/iter_result.rs, scripts/addr_class_ratchet_baseline.txt, changelog.d/7585-addr-class-mark-shape-shared.md
Adds mark_shape_shared, uses it for shared iterator-result keys, removes the stale address-class allowance, and documents the change.
Release metadata update
Cargo.toml, CLAUDE.md
Updates the workspace and documented versions from 0.5.1328 to 0.5.1329.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • PerryTS/perry#6810: Introduced the shared-shape GC marking logic that this PR extracts into gc::mark_shape_shared.
  • PerryTS/perry#6636: Modified related runtime GC pointer handling and address-class ratchet validation.
  • PerryTS/perry#7525: Modified GC metadata handling for shared key arrays.

Suggested reviewers: jdalton, thehypnoo

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/addr-class-iter-result

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant