fix(codegen): guard var_dump's container walk against the null-container sentinel (#581)

[mirchaemanuel]

Fixes #581. Based on current main 9f74f5300.

Root cause

Not in the __rt_var_dump_* runtime walkers — in the lowering. emit_var_dump_array (src/codegen/lower_inst/builtins/debug.rs) guarded its header walk with a zero-pointer check only:

abi::emit_branch_if_int_result_zero(ctx.emitter, &null_label);

That comment described the one case it was written for: an untyped null-defaulted property rebound to array storage. A missed element read produces something else — the in-band null-container sentinel, which is non-zero. So the branch never fired, array( was written, and the next instruction dereferenced the sentinel as a live array header.

Visible in the emitted assembly for the repro:

    ldur x0, [x29, #-96]
    cbz  x0, _eir_main_var_dump_array_null_4   <- zero check only
    str  x0, [sp, #-16]!
    ...                                        <- writes "array("
    ldr  x0, [x0]                              <- SIGSEGV on the sentinel

which matches the reported output byte for byte: array( with no newline, then the crash.

Fix

Swap the zero-only branch for the shared sentinels::emit_branch_if_null_container, which recognizes both null shapes and is already used by the guards from #526, #556 and #585. Scratch register from abi::secondary_scratch_reg, the same way lower_array_len takes one.

One call site covers the Array, AssocArray and Iterable arms plus both entries from emit_var_dump_mixed, because they share this function. The helper is target-aware, so both supported architectures are handled with no ARM64-only path.

Before / after on the report's repro:

before: Warning: Undefined array key 7 / array(     [exit 139, "done" never reached]
after:  Warning: Undefined array key 7 / NULL / done [exit 0]
php:    Warning: Undefined array key 7 / NULL / done [exit 0]

Tests

Six regressions in tests/codegen/regressions/arrays.rs. Four are behavioural and fail on the pre-fix build with program crashed; one is a guard for the shapes that already worked; one asserts the emitted guard instruction directly.

The assembly test was checked with an explicit negative control — reverting the source makes it fail on both architectures with missing sentinel comparison 'cmp rax, r10' and 'cmp x0, x10' respectively, so x86_64 is genuinely verified rather than assumed. It runs through compile_source_to_asm_with_options with ELEPHC_TEST_TARGET, the same way #556's test does, because the CLI builds a runtime object even under --emit-asm and so cannot cross-emit locally.

Two sibling shapes were fixed by the same one-line change and are covered: a hash source whose value is an array, and one whose value is a hash (the AssocArray arm) — the latter crashed too and is not mentioned in the issue. Also covered: a missed read through ?? null, and a genuine null local as a non-regression guard.

Verification (macOS ARM64, beea5b280)

filter	result
the 6 new tests	6 passed, 0 failed
same, ELEPHC_IR_OPT=off	6 passed, 0 failed
asm test, ELEPHC_TEST_TARGET=linux-x86_64	1 passed, 0 failed
asm test, ELEPHC_TEST_TARGET=linux-aarch64	1 passed, 0 failed
var_dump (existing)	34 passed, 0 failed
sentinel	41 passed, 0 failed, 3 ignored
array_miss (#526 guards)	20 passed, 0 failed
null_container (#556/#592)	2 passed, 0 failed
autovivify (#600/#592)	26 passed, 0 failed
repro under --heap-debug	leak summary: clean
cargo build / git diff --check	clean, zero warnings

Narrow filters only, per the project's test policy; CI owns the full matrix. Linux targets not executed locally (no Docker), but the x86_64 lowering is verified through the asm test above rather than by assumption.

Out of scope, reported separately

• print_r() has the same crash and no guard at all — not even the zero check var_dump had. print_r($a[7]) still segfaults on current main and on this branch. Different lowering and different PHP semantics (print_r(null) prints nothing, it does not print NULL), so it wants its own change. Filed separately.
• var_dump($a[7][2]) where the element type is a scalar prints string(0) "" instead of NULL. Both warnings are correct; the static type of the expression is Str, so the dump takes the string arm and the chained miss materializes an empty string rather than the sentinel. That is a value-materialization defect on the read path, not a walker defect, and predates this change.

[greptile-apps]

Greptile Summary

Guards emit_var_dump_array in the debug lowering against the in-band null-container sentinel by swapping the old zero-only branch for sentinels::emit_branch_if_null_container, which recognizes both null shapes. This fixes the crash (#581) where a missed array element read materializes the non-zero sentinel, which the previous cbz-only guard let through, causing array( to be written before the header pointer was dereferenced.

• src/codegen/lower_inst/builtins/debug.rs: One call site change in emit_var_dump_array covers Array, AssocArray, and Iterable arms plus the two Mixed-dispatch entries in emit_var_dump_mixed. A secondary scratch register is taken via abi::secondary_scratch_reg, consistent with other null-container guard sites.
• tests/codegen/regressions/arrays.rs: Six new tests — four behavioral regressions (indexed miss, hash-value miss, assoc-value miss, coalesce-null miss), one non-regression guard for genuine null locals and live arrays/hashes, and one assembly-ordering test that verifies the sentinel comparison precedes the header load on both supported architectures through ELEPHC_TEST_TARGET.

Confidence Score: 5/5

Safe to merge. The change is a single, targeted call-site swap using a helper already established by earlier guard fixes (#526, #556, #585), all tested paths remain covered, and the fix does not touch ownership, ABI layout, or register allocation.

The fix is a one-line substitution at a single call site, replacing a partial guard with the shared complete guard. The helper being adopted is already exercised by 41 passing sentinel tests and multiple prior guard sites, so there is no novel logic to go wrong. Six new regression tests cover all the crash-path variants plus a non-regression guard and an assembly ordering assertion on both supported architectures. The scratch register usage matches the pattern already established in lower_array_len and peers. No ownership paths, ABI contracts, or cross-function interfaces are modified.

Files Needing Attention: No files require special attention. The print_r sibling gap acknowledged in the PR description is filed separately and is out of scope here.

Important Files Changed

Filename	Overview
src/codegen/lower_inst/builtins/debug.rs	Replaces emit_branch_if_int_result_zero with emit_branch_if_null_container in emit_var_dump_array, correctly guarding against both the zero-pointer and in-band sentinel null shapes before the array-header dereference.
tests/codegen/regressions/arrays.rs	Adds six regression tests: four behavioral (crash → NULL on miss), one non-regression guard for live containers, and one assembly-level ordering test confirming the sentinel comparison precedes the header load on both architectures.
CHANGELOG.md	Adds a detailed [Unreleased] bullet describing the var_dump sentinel guard fix, following the project changelog policy.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["emit_var_dump_array(ctx, ty)"] --> B["emit_branch_if_null_container\n(result_reg, scratch_reg, null_label)"]
    B --> C{"value == 0?"}
    C -- yes --> N["null_label:\nemit_var_dump_null()"]
    C -- no --> D{"value == NULL_SENTINEL\n(0x7fff_ffff_ffff_fffe)?"}
    D -- yes --> N
    D -- no --> E["push result_reg\nemit_write 'array('"]
    E --> F["pop, push result_reg\nldr x0, [x0]  ← header load\n(safe: not null/sentinel)"]
    F --> G["emit element count + body walk"]
    G --> H["done_label"]
    N --> H

Loading

_{Reviews (1): Last reviewed commit: "fix(codegen): guard var_dump's container..." | Re-trigger Greptile}

[nahime0]

Fine for me. I'll merge as is, without merging main here. I don't see potential semantic conflicts