fix(gc): classify fetch handle ids by band, not magnitude — SIGSEGV on instanceof (#7526)

[proggeramlug]

Fixes #7526.

The crash: a handle id dereferenced as a heap pointer

fetch_subclass_handle_id classified its receiver by magnitude, not by band:

if obj < GC_HEADER_SIZE + 0x1000 || !is_valid_obj_ptr(obj as *const u8) { return None; }
let gc_header = (obj as *const u8).sub(GC_HEADER_SIZE) as *const GcHeader;   // <-- 0x3fff8

is_valid_obj_ptr's own HEAP_MIN is 0x1000, so that pair is two magnitude tests and no band test. Every handle band sits above the floor and below HANDLE_BAND_MAX (0x100000), so none of them was excluded — Web Fetch ids live in [0x40000, 0xE0000).

new Response(...) returns id 0x40000 exactly, and r instanceof Request reaches this probe, so the first fetch handle a program allocates segfaulted. Under lldb:

EXC_BAD_ACCESS (code=1, address=0x3fff8)
frame #0: perry_runtime::object::field_get_set::fetch_subclass_handle_id + 44
->  ldurb  w8, [x0, #-0x8]      ; the GcHeader sniff, x0 = 0x40000
    cmp    w8, #0x2             ; GC_TYPE_OBJECT

Deterministic, reproduced on two hosts, and not profile-dependent (fails under perry-dev and --release alike). It is also not the GC-rooting family that dominated today: nothing here is stale, the address was never a pointer.

Fix

addr_class::is_plausible_heap_addr — the canonical is_above_handle_band && is_valid_obj_ptr pairing — plus addr_class::try_read_gc_header, which magnitude-classifies before touching memory. This is the predicate the module docs already nominate for exactly this job.

The regression test walks the band boundaries (COMMON_HANDLE_BAND_END, FETCH_HANDLE_BAND_{START,END}, PROXY_ID_BAND_START, HANDLE_BAND_MAX - 1, 1) rather than one value, so a band added to addr_class without a matching guard here fails it.

Sabotage-verified, and my first attempt was insufficient — worth recording. Reverting only the band check left try_read_gc_header in place, which classifies safely on its own, so the test still passed. Restoring the original guard and the raw deref makes the test binary die with signal: 11, SIGSEGV. A partial revert is not a sabotage test.

test_gap_fetch_instanceof_5433 is now byte-identical to node 26.5.1, exit 0.

The reason nobody saw it: the snapshot laundered it

The crash was accepted in gap_snapshot.json:

"status": "crash", "issue": "5433", "category": "bug-open",
"reason": "fetch Response/Request instanceof (#5433); standing per #5917."

Both cited issues are CLOSED. The entry claimed bug-open while nothing tracked it, and it was the only status: "crash" entry in the file — an exception, not a convention.

run_gap_tests.sh already printed crashes every run, deliberately, with a comment explaining that crashes "are hard defects, never cosmetic gaps". But printing is not gating, so it sat there for a month. This adds the enforcement: the harness now refuses any status: "crash" snapshot entry and exits 2.

It starts green — this change removes the only such entry — so it can only go red on a new attempt to park a crash. Planting a probe entry makes it refuse, verified.

Validation

cargo test -p perry-runtime --no-fail-fast 1762 passed / 0 failed. raw_handle_debt.py 999 (baseline 999), check_file_size.sh, addr_class_inventory.py, cargo fmt --all --check all clean.

Sibling sites — named, not fixed here

The same magnitude-floor idiom (< GC_HEADER_SIZE + 0x1000 with no band test) appears in at least eight other places: util_promisify.rs:752, node_v8.rs:153, native_handle.rs:149, timer.rs:944, dgram.rs:314, proxy.rs:{534,869,1149}. Whether each is reachable with a banded value needs checking per site, so they are not swept in blind here. I will file that as its own issue rather than widen a crash fix.

Summary by CodeRabbit

• Bug Fixes

  • Fixed a crash that could occur when handling invalid Web Fetch handle addresses.
  • Improved safety when validating and reading runtime object metadata.

• Testing

  • Added boundary-focused coverage to ensure invalid handle addresses are rejected safely.
  • Gap testing now fails when crash results are recorded instead of treating them as expected differences.

• Documentation

  • Added changelog coverage for the crash fix and updated gap-test behavior.

• Release

  • Updated the application version to 0.5.1300.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 950ed37a-f5ac-4e35-b167-0f219548e990

📥 Commits

Reviewing files that changed from the base of the PR and between 3195e61 and 5540310.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• CLAUDE.md
• Cargo.toml
• changelog.d/7530-fetch-handle-band-deref.md
• crates/perry-runtime/src/object/field_get_set.rs
• scripts/run_gap_tests.sh
• test-parity/gap_snapshot.json

---

📝 Walkthrough

Walkthrough

This PR hardens fetch handle address checks in the runtime, adds regression coverage for handle-band addresses, rejects status: "crash" entries in gap snapshots, updates related changelog text and snapshot encoding, and bumps the project version to 0.5.1300.

Changes

Fetch handle safety and gap enforcement

Layer / File(s)	Summary
Runtime handle-band guard crates/perry-runtime/src/object/field_get_set.rs, changelog.d/7530-fetch-handle-band-deref.md	fetch_subclass_handle_id now uses is_plausible_heap_addr and try_read_gc_header before object-type checks. Tests cover handle-band boundary and interior addresses and assert a None result without a fault. The changelog describes the same runtime fix and regression coverage.
Gap snapshot crash enforcement scripts/run_gap_tests.sh, test-parity/gap_snapshot.json, changelog.d/7530-fetch-handle-band-deref.md	run_gap_tests.sh now validates the configured snapshot before update or verification, reports parked crash entries, and exits with status 2 if any exist. Snapshot text now uses \u2014 escapes, and the changelog documents the new crash-entry rejection rule.
Version metadata update Cargo.toml, CLAUDE.md	The workspace package version and documented current version changed from 0.5.1299 to 0.5.1300.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• PerryTS/perry#6599: Both PRs harden fetch_subclass_handle_id against invalid handle values and unsafe dereference paths.
• PerryTS/perry#6755: Both PRs change gap-test snapshot handling in scripts/run_gap_tests.sh and test-parity/gap_snapshot.json.
• PerryTS/perry#6859: Both PRs reject handle-band values before unsafe runtime object interpretation.

Suggested labels: bug, parity

Suggested reviewers: thehypnoo, andrewtdiz, jdalton

✨ 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/7526-fetch-handle-band-deref

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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