gc: a stack-map record must belong to the function the ip is in (#7314 follow-up)

[proggeramlug]

Follow-up to #7314, addressing the one remaining major finding from its review.

The hazard

match_records accepted the nearest safepoint within ±16 bytes of the return address. That window is a distance, not a containment check — nothing in it says the matched record belongs to the function the ip is actually executing. Functions are adjacent in .text, so an ip early in function B can fall inside the window of a safepoint at the end of function A. The walkers would then use A's function_address and frame offsets to compute root addresses for B's frame, and rewrite unrelated stack words.

Measured before changing anything

The obvious fix — require an exact pc, since statepoints sit exactly at the return address and the plain-map lowering that sat before the call is now deleted — would have been wrong. Instrumenting the whole probe suite under forced evacuation:

delta	accepted (≤16)	same function?
8	yes	yes
32, 48, 56, 64 ×3	no	yes

Seven inexact matches occur. Six are already rejected as out-of-window. The one accepted at delta=8 is same-function and legitimate, so requiring an exact pc would have discarded a real root. And no cross-function match happens today — the hazard is real but latent.

The fix

Containment rather than tightening: the matched record's function must be the greatest mapped function start ≤ ip, which the index now precomputes as a sorted, deduplicated list. That rejects the cross-function case and keeps the legitimate near-match.

Residual gap, stated in the comment rather than papered over: a function with no safepoints is absent from the function list, so an ip inside one resolves to the previous mapped function. Closing that needs a per-function code extent, and Mach-O does not expose one cheaply — Lfunc_end covers only EH-carrying functions (5 of 43 in a sampled module) and there is no .size directive. Worth a separate issue if the statepoint backend moves toward default-on.

Verification

Regression test asserts both directions: a record from the previous function is rejected, and a same-function near-match is still accepted. All three arms (explicit bridge, RS4GC, default shadow stack) remain 9/9 against the pinned Node oracle, with the statepoint arms under PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 PERRY_STACKMAP_WALKER=verify.

Summary by CodeRabbit

• Bug Fixes
  • Improved stack map matching to prevent incorrect matches across adjacent functions.
  • Preserved valid matches for nearby instructions within the same function.
  • Added safeguards to ensure matching data remains accurate and reliable.

[coderabbitai]

📝 Walkthrough

Walkthrough

StackMapIndex now tracks function starts and verifies function ownership during instruction-pointer matching. Cross-function near matches are rejected, while valid same-function near matches remain accepted. Regression tests cover both cases.

Changes

Function-aware stack map matching

Layer / File(s)	Summary
Function ownership matching crates/perry-runtime/src/gc/roots/stack_maps.rs	StackMapIndex stores sorted, deduplicated function starts. match_records selects the containing function before accepting a record. Tests cover adjacent-function rejection and same-function near matches.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: preventing stack-map records from matching across function boundaries.
Description check	✅ Passed	The description explains the hazard, fix, regression coverage, related issue, and verification results in sufficient detail.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 gc/7314-followup-cross-function-match

---

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.}

[coderabbitai]

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 `@crates/perry-runtime/src/gc/roots/stack_maps.rs`:
- Around line 301-320: Update the native matching logic around function_starts
and the matched record selection to fail closed when function containment cannot
be proven. Use a source that includes extents for functions without safepoints,
or reject the candidate whenever the current map cannot establish that
candidate_pc belongs to the matched record’s function; do not accept ownership
based solely on the previous function start.

🪄 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: 0c99da12-ddb7-44e7-b1f1-30df45e33c64

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe4939 and 48c553a.

📒 Files selected for processing (1)

• crates/perry-runtime/src/gc/roots/stack_maps.rs

[coderabbitai]

🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline crates/perry-runtime/src/gc/roots/stack_maps.rs --items all --type function

rg -n -C 6 'function_address|function_starts|parse_gc_map|record_count|function table' \
  crates/perry-runtime/src/gc/roots/stack_maps.rs

fd -t f -i 'gc_map\.rs' crates | while IFS= read -r file; do
  rg -n -C 6 'function|record|safepoint|GC_MAP' "$file"
done

Repository: PerryTS/perry

Length of output: 42090

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the matching/failure and consumption logic without running repository code.
sed -n '120,136,257,281,325,352,380,940,980,995,995,995,995,995,995,995,995,995,1265,1306,1343p' crates/perry-runtime/src/gc/roots/stack_maps.rs

# Behavioral probe of the documented matching strategy using the same partition logic
# and an ip in a mapless function B after function A.
python3 - <<'PY'
def resolve(ip, functions, records):
    function_starts = sorted(set(r[0] for r in records))
    idx = next((i for i, start in enumerate(function_starts) if start > ip), len(function_starts))
    owning = function_starts[idx - 1] if idx > 0 else None
    first = next((i for i, (pc, _) in enumerate(records) if pc >= ip - 16 or pc == records[0][0] if pc <= ip + 16 else None), record[0] <= ip + 16 and record[0] >= ip - 16 and record[0] < closest
): [rec[0] for rec in records if closest is None else []
    for rec in records:
        if closest is None:
            closest = rec
        elif rec[0] > closest[0]:
            break
    if ip <= closest[0]:
        # use closest
    else:
        # find largest record.pc <= ip + 16?
    pass
PY

Repository: PerryTS/perry

Length of output: 203

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the relevant stack-map matching, consumption, and regression ranges without running repository code.
sed -n '120,180p' crates/perry-runtime/src/gc/roots/stack_maps.rs
sed -n '253,322p' crates/perry-runtime/src/gc/roots/stack_maps.rs
sed -n '325,390p' crates/perry-runtime/src/gc/roots/stack_maps.rs
sed -n '920,995p' crates/perry-runtime/src/gc/roots/stack_maps.rs
sed -n '1264,1355p' crates/perry-runtime/src/gc/roots/stack_maps.rs

# Deterministic probe of the matching algorithm from source text:
# - function_starts = sorted unique function_address of records.
# - owning = greatest function_starts <= ip via partition_point.
# - A record matches only if its function_address == owning.
python3 - <<'PY'
import bisect

def match_records(ip, records):
    function_starts = sorted({r[0] for r in records})
    idx = bisect.bisect_right(function_starts, ip)
    owning = function_starts[idx - 1] if idx > 0 else None
    candidates = [(pc, func_addr) for func_addr, _, pc in records if pc <= ip + 16 and pc >= ip - 16]
    matched = [r for r in candidates if r[0] == owning]
    # Sort by the algorithm's closest record order
    closest = min(matched, key=lambda r: r[0], default=None)
    return owning, closest

records = [
    ("A", 0xcc, 0x1ffc),  # function A, record pc +16 delta
    ("B", 0x20, 0x2040), # function B present with second record; first record omitted for adjacent cross-function range
]
print("adjacent-present:", match_records(0x2004, records))

records_no_b = [
    ("A", 0xcc, 0x1ffc),
    ("B", 0x20, 0x2010), # present but outside nearest safepoint window at 0x203c/0x2010?
]
print("adjacent-present-window:", match_records(0x2004, records_no_b))

no_mapless_function = [("A", 0xcc, 0x1ffc)]
print("adjacent-mapless:", match_records(0x2004, no_mapless_function))
PY

Repository: PerryTS/perry

Length of output: 15594

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Simulate the exact partition-point matching behavior for the mapless-between-functions case.
python3 - <<'PY'
import bisect

def nearest_record_pc(records, ip):
    masks = [r["pc"] for r in records]
    idx = bisect.bisect_left(masks, ip)
    before = (idx - 1, masks[idx - 1]) if idx > 0 else None
    after = (idx, masks[idx]) if idx < len(masks) else None
    if before is None:
        return (after[0], after[1]) if after else None
    if after is None:
        return before
    return after if ip - before[1] >= after[1] - ip else before

def match_records(records, ip):
    function_starts = sorted({r["function_address"] for r in records})
    idx = bisect.bisect_right(function_starts, ip)
    owning = function_starts[idx - 1] if idx > 0 else None
    candidate = nearest_record_pc(records, ip)
    if candidate is None:
        return owning
    record_pc, pc_idx = candidate
    if abs(record_pc - ip) > 16:
        return owning
    first = bisect.bisect_left([r["pc"] for r in records], records[pc_idx]["pc"])
    last = bisect.bisect_right([r["pc"] for r in records], records[pc_idx]["pc"])
    matched = records[first:last]
    if matched and owning is not None:
        return owning if matched[0]["function_address"] == owning else None
    return None

records = [
    {"pc": 0x1ffc, "function_address": 0x1000},
    {"pc": 0x2040, "function_address": 0x2000},
]
print(match_records(records, 0x2004) == 0x2000)

records_mapless = [
    {"pc": 0x1ffc, "function_address": 0x1000},
    {"pc": 0x2010, "function_address": 0x2000},
]
print(match_records(records_mapless, 0x2004) == 0x2000)
PY

Repository: PerryTS/perry

Length of output: 161

---

Fail closed when containment cannot be proved.

function_starts only holds addresses from records, so a function with no safepoints is omitted. An executable PC inside that function can still satisfy the ±16 closest-PC check and fall before the next function start; the current containment check accepts the previous function’s records. Use a source that includes mapless function extents, or disable the native match when the current map cannot contain the matching record.

🤖 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 `@crates/perry-runtime/src/gc/roots/stack_maps.rs` around lines 301 - 320,
Update the native matching logic around function_starts and the matched record
selection to fail closed when function containment cannot be proven. Use a
source that includes extents for functions without safepoints, or reject the
candidate whenever the current map cannot establish that candidate_pc belongs to
the matched record’s function; do not accept ownership based solely on the
previous function start.