Skip to content

fix(codegen): guard container consumers against the null-container sentinel (chained-read miss segfault)#533

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/526-array-get-miss-null-fallback
Open

fix(codegen): guard container consumers against the null-container sentinel (chained-read miss segfault)#533
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/526-array-get-miss-null-fallback

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Fixes #526 — and the crash class turned out wider than the report: on main, all of these segfault (exit 139), not just the chained read: isset($a[7][1]), $a[7][1] ?? …, $m['nope']['x'], $a[9][0][0], $a[0][9][0], foreach ($a[7] ?? [] as $v), foreach ($a[7] as $v).

Root cause

emit_array_get_null_fallback (src/codegen/lower_inst/arrays.rs) materializes the in-band NULL_SENTINEL (0x7fff_ffff_ffff_fffe) for missed reads of refcounted element types, and every downstream container consumer dereferenced it unguarded: lower_array_get_{aarch64,x86_64} loaded the length header straight from it; the same held for __rt_hash_get, __rt_array_get_mixed_key, __rt_hash_iter_next (null-guarded 0 but not the sentinel), the isset array probe, and the foreach length snapshot. Compounding it, IsNull on statically-container-typed slots was hard-coded false, so $a[7] ?? [] evaluated the miss as non-null and propagated the sentinel into foreach.

Design

Consumer-side guards, keeping the sentinel as the established null-container representation — it is already what ??/isset compare against for scalars, what Throwable::getPrevious() uses for null objects, and what incref/decref already skip via heap-range checks, so it is GC-safe by construction. (Changing the fallback to a different representation was considered and rejected: the sentinel-as-null convention is load-bearing across comparisons, so a representation change would ripple much wider.)

  • New shared emitter helper emit_branch_if_null_container (src/codegen_support/sentinels.rs): branch on reg == 0 || reg == NULL_SENTINEL, symmetric ARM64/x86_64 arms.
  • Guarded: lower_array_get_* (jumps to the fallback past the warning, so the Silent variants stay silent automatically), the isset array probes, IsNull for Array/AssocArray/Iterable/Object, the foreach indexed-length snapshot; plus 3-instruction sentinel extensions of the existing null guards in __rt_hash_get, __rt_array_get_mixed_key, __rt_hash_iter_next (both arches each).
  • One ir_lower change so isset/?? silence propagates through the whole subscript chain (isset($a[7][1]) must not warn at any level — PHP semantics).

Before → after (base v0.26.1 93f576168, macOS arm64; all post-fix runs exit 0 and are PHP-compared)

Case before after
$a[7][1] (issue repro) warn + SIGSEGV warn, x=, done, heap clean
$a[1][7] (contrast) OK byte-identical to baseline
isset($a[7][1]) SIGSEGV false, no warning
$a[7][1] ?? 'dflt' SIGSEGV silent, no crash (yields '' — pre-existing gap, see notes)
$m['nope']['x'] string-keyed SIGSEGV NULL, mirrors the working direction
3-level miss at each level 2 of 3 SIGSEGV NULL ×3, one warning per real miss
foreach ($a[7] ?? [] as $v) / foreach ($a[7] as $v) SIGSEGV iterates empty / warns and skips
Mixed-element miss OK unchanged

--heap-debug: the repro and every previously-crashing variant end clean; the leaks remaining on some second-index-miss paths are byte-identical on unmodified main (pre-existing; they belong to the leak family being fixed in #524/#531/#532 — verified composed: on a local merge of those PRs the same programs end clean).

Cross-arch verification

Both arch arms edited symmetrically in every touched emitter (diff-reviewed). Beyond that, I captured the actual x86_64 assembly on this host (user asm via a runtime-cache pre-seed, runtime asm via an as shim — the toolchain otherwise insists on assembling with the host as) and verified the emitted guards verbatim (test/jz + movabs sentinel + cmp/je) in array_get, the isset probe, is_null_container, the iterator length snapshot, __rt_hash_get, __rt_array_get_mixed_key, __rt_hash_iter_next. Executing x86_64 locally is not possible on this host — relying on CI for the run matrix.

Tests

10 new regression tests in tests/codegen/regressions/arrays.rs (crash repro asserting stdout+warning+exit, second-index guard, heap-clean assertion, isset/?? silence, string-key chain, 3-level chain, foreach direct + coalesce, Mixed-element guard); the exact fixtures exit 139 on main. Focused suites, 0 failures: arrays 347 (incl. the 10 new), array_basics 81, isset 21, nested 119, coalesce 54, foreach 105, is_null 18, previous 3, runtime_gc 122. Zero build warnings; scripts/check_asm_comments.py clean on all touched codegen files.

Notes for the maintainer

…llegalstudio#526)

A chained subscript read whose FIRST index misses materialized the
in-band scalar null sentinel (0x7fff_ffff_ffff_fffe) for refcounted
element types, and the consuming outer read then loaded the array
length header from that sentinel address and crashed with SIGSEGV.

Treat a zero pointer or the null sentinel as the in-band PHP-null
container representation on every read surface, on both ARM64 and
x86_64:

- indexed array_get (warn + silent variants) branches to the null
  fallback, skipping the undefined-key warning, when the receiver is
  null/sentinel
- the isset() array-offset probe reports such receivers as missing and
  recognizes ArrayGetSilent producers
- __rt_hash_get, __rt_array_get_mixed_key, and __rt_hash_iter_next
  extend their existing null-table guards with the sentinel pattern
- IsNull on statically typed containers (Array/AssocArray/Iterable/
  Object) now detects the null/sentinel representation so `?? []`
  yields the default instead of propagating the sentinel into foreach
- foreach's indexed length snapshot treats a null/sentinel source as
  length zero so the loop body is skipped instead of crashing
- isset()/`??` suppress undefined-offset warnings across the whole
  subscript chain, matching PHP's silence for every level

Fixes illegalstudio#526

Claude-Session: https://claude.ai/code/session_01Jfn7uqEH5Dog1nBzTM5DCW
@github-actions github-actions Bot added area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:s Small pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chained subscript read where the first index misses segfaults (null-fallback sentinel dereferenced as an array pointer)

1 participant