fix(runtime): guard boxed-mixed array get/set against the null-container sentinel#591
Conversation
…ner sentinel A missed indexed read (`$rows[5]`) forwarded as one arm of a ternary whole-boxes into a `mixed` cell whose payload pointer is the in-band null-container sentinel (`NULL_SENTINEL = 0x7ffffffffffffffe`) rather than a real array pointer. `__rt_mixed_array_get` dispatched on the cell's tag (indexed/assoc/object), loaded the container pointer from `[cell+8]`, and guarded it only with a zero check (`cbz`/`test`), which the non-zero sentinel slips past. The next load dereferenced the sentinel and segfaulted; the sibling writer `__rt_mixed_array_set` had the identical hole on `$r[0] = …`. The ternary's nullable-union path bypasses `wider_type_for_merge` (the `match` merge path does not), so the sentinel reaches the helpers in-band. Both helpers now route a null OR sentinel container payload to their existing absent-container paths via the shared `emit_branch_if_null_container`, on aarch64 and x86_64: the read returns `Mixed(null)` (with PHP's undefined-key warning) and the write is dropped, matching PHP's quiet access and elephc's existing null-target behavior. Regression tests cover the exact issue repro, the present-arm control, the indexed write, and heap-clean checks for both the read and the dropped write. Fixes illegalstudio#585 Claude-Session: https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L
github-actions
Bot
added
area:runtime
|
While validating this fix I found an adjacent pre-existing crash on the append path ( |
…rray-get-sentinel # Conflicts: # src/codegen_support/runtime/objects/mixed_array_set.rs # tests/codegen/regressions/arrays.rs
…d cell A missed indexed read forwarded through a ternary/branch merge boxes the null-container sentinel (0x7ffffffffffffffe) into a `mixed` cell tagged as an indexed array. `$r[] = v` on that cell (`Op::MixedArrayAppend`) unboxed the receiver, accepted the tag-4 payload, and read the array header inline (`ldur x1, [x0, #-8]`) before `__rt_array_to_mixed`. The only pre-deref guard was a plain-zero check, so the sentinel passed through and the header read faulted at `sentinel - 8` (issue illegalstudio#592, exit 139). PHP instead auto-vivifies the null into a fresh `["z"]` array and continues. `lower_mixed_array_append` (aarch64 + x86_64) now guards the unboxed payload for both the null pointer and the in-band null-container sentinel before the header dereference (issue illegalstudio#533 convention). For a container-shaped tag with such a payload — indexed (4), associative (5), or null (8) — it auto-vivifies: it allocates a fresh empty indexed array (`__rt_array_new(0, 8)`, as `__rt_mixed_new_empty_array_cell` does), transfers that reference into the Mixed cell, retags the cell as an indexed array, and appends at index 0 through the shared `__rt_mixed_array_set` tail. Scalars whose payload happens to be zero or the sentinel are excluded by the tag check and keep the existing drop behavior. Ownership is balanced (the cell owns the fresh array; the array owns the appended value), so the autovivify path stays heap-clean. Adds regression coverage in tests/codegen/arrays/mixed_append_autovivify.rs: the exact issue repro, the `["z"]` read-back, a heap-clean sentinel variant, an associative missed-read sibling, a multi-append grow case, and a real-array Mixed-cell control that guards against a double-conversion regression. Fixtures make the taken arm `$argc`-dependent to defeat constant folding. Claude-Session: https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L
github-actions
Bot
added
area:codegen
|
Picking this up. CI is red on the current head The failures cluster in the ArrayAccess family: One concrete data point: All of the above is read off the CI logs; we have not reproduced it locally yet and are not claiming a cause. |
…flag operand The warn-on-missing operand added to boxed-Mixed subscript reads gave those reads a third operand, which two consumers still read as a write. ArrayAccess dispatch classified by operand count alone, so `$obj[$k]` matched the `offsetSet` arm and called the setter with the flag as its value. Reads are now identified structurally by having a result value (subscript writes lower through `emit_void`), and the flag operand is stripped before dispatch so `offsetGet` keeps its single-argument PHP signature. `lower_list_unpack` still emitted two operands for boxed-Mixed sources, so `[$a, $b] = $narrowed` failed codegen with "runtime_call missing operand 2". It now supplies the flag through the same helper as the array-access path. Claude-Session: https://claude.ai/code/session_01124pGFSEbzYQcGbNykWN7V
|
Pushed a fix for the CI failures on Both share the same cause: the new warn-on-missing operand gave boxed-Mixed subscript reads a third operand, and two consumers still interpreted three operands as a write. 1. ArrayAccess reads dispatched Reads are now identified structurally by having a result value — subscript writes lower through 2. One thing worth your judgement. The dispatch into Side effect, deliberate: the structural discriminator also makes No CHANGELOG entry and no new tests: both regressions were introduced and fixed inside this unmerged PR so nothing user-facing changed, and the tests that caught them are the pre-existing ones that turned red. Verification, all on macOS aarch64 and all against the exact pushed commit:
Two caveats I'd rather state than have you find: the full Unrelated pre-existing bug found while validating, reproduces byte-identical on the last green head so it predates this branch: |
…rray-get-sentinel # Conflicts: # CHANGELOG.md
2 participants
Summary
A missed indexed read (
$rows[5]) forwarded as one arm of a ternary whole-boxes into amixedcell whose payload pointer is the in-band null-container sentinel(
NULL_SENTINEL = 0x7ffffffffffffffe) rather than a real array pointer.$r[0] ?? "none"then crashed in
__rt_mixed_array_get, which dispatched on the cell tag, loaded thecontainer pointer from
[cell + 8], and guarded it only with a zero check (cbz/test) —which the non-zero sentinel slips past. The next load dereferenced the sentinel and
segfaulted (exit 139). The sibling writer
__rt_mixed_array_sethad the identical hole on$r[0] = ….The ternary's nullable-union path bypasses
wider_type_for_merge(thematchmerge pathdoes not), so the sentinel reaches the helpers in-band. PHP 8.4 warns and yields
null/drops the write; elephc crashed.
Root cause
Both helpers load the container payload from
[cell + 8]in each of their indexed / assoc /object paths and guarded it against a zero pointer only. A missed container read boxed into a
mixedstores the sentinel (not zero, not a valid pointer) in that slot, so the guard isbypassed and the following dereference faults.
Fix
__rt_mixed_array_getand__rt_mixed_array_setnow route a null or null-containersentinel payload to their existing absent-container paths via the shared
emit_branch_if_null_containerhelper (issue #533 convention: guards live inside the runtimehelpers), on both aarch64 and x86_64. The read returns
Mixed(null)with PHP's undefined-keywarning; the write is dropped, matching elephc's existing null-target behavior.
Test plan
New regressions in
tests/codegen/regressions/arrays.rs:Red→green: the four sentinel-path tests crash before the fix and pass after. Focused slices
(
ternary,mixed_array,coalesce,null_container,missed,regressions::arrays,null_sentinel,json,stdclass,spl_,assoc) pass: 958 tests, 0 failures.Fixes #585
https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L