Skip to content

addr-class: eight sites still classify heap addresses by magnitude floor, not by band (the #7526 idiom) #7531

Description

@proggeramlug

The idiom

Eight-plus sites classify a candidate heap address with a magnitude floor
and no band test:

if addr < crate::gc::GC_HEADER_SIZE + 0x1000 { ... }

0x1008 is below every handle band, and is_valid_obj_ptr (often paired
with it) is also magnitude-only — its HEAP_MIN is 0x1000. So the pair
excludes null and small garbage but admits every registry handle:

band range source
common handles [1, 0x40000) perry-stdlib common/handle.rs
Web Fetch [0x40000, 0xE0000) fetch/mod.rs
proxy ids [0xF0000, 0x100000) proxy.rs

#7526 was exactly this: fetch_subclass_handle_id dereferenced 0x40000 at
addr - 8 and SIGSEGV'd on the first new Response(...) a program made.
Fixed in #7530 by switching to addr_class::is_plausible_heap_addr +
try_read_gc_header.

Sites to audit

Each needs a per-site answer to "can a banded value reach here?" — a blind
sweep would be wrong, since some of these are already unreachable with handles
and a needless band test costs a branch on a hot path:

  • crates/perry-runtime/src/util_promisify.rs:752
  • crates/perry-runtime/src/node_v8.rs:153
  • crates/perry-runtime/src/native_handle.rs:149
  • crates/perry-runtime/src/timer.rs:944
  • crates/perry-runtime/src/dgram.rs:314
  • crates/perry-runtime/src/proxy.rs:534, :869, :1149

native_handle.rs and proxy.rs are the ones I would look at first: both sit
directly on paths where a handle is the expected representation, so a
banded value arriving is routine rather than exotic.

Why this is worth a ticket rather than a sweep

macOS masks it. Its heap floor is ~2 TB, so a handle-band deref lands in
unmapped low memory and faults loudly; on platforms with a low heap base the
same read can silently hit mapped memory and return garbage that then flows
into an obj_type comparison. run_gap_tests.sh's own crash banner says this:
"Reproduce on LINUX — several crash classes (e.g. handle-band derefs, #6271)
are masked on macOS."
A silent wrong answer is worse than #7526's crash.

Suggested acceptance

Every listed site either uses addr_class::is_plausible_heap_addr /
try_read_gc_header, or carries a one-line comment stating why a banded value
cannot reach it. Plus a band-boundary test per converted site, in the style of
fetch_subclass_probe_rejects_every_handle_band_without_dereferencing (#7530)
— walk the boundaries, so a band added later without a guard fails the test.

scripts/addr_class_inventory.py already ratchets new bare-address sites;
this is the pre-existing residue it grandfathered.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions