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

[mirchaemanuel]

Fixes #647. Based on current main 9f74f5300.

Sibling of #646 (same file, same sentinel family) but a different function and different PHP semantics — see below. The two are independent; see the conflict note at the end.

Root cause

emit_print_r_array (src/codegen/lower_inst/builtins/debug.rs) had no null branch at all — not even the zero-pointer check that emit_var_dump_array carried. So a missed element read wrote Array\n and passed the null-container sentinel straight to the walker as a live container.

Pre-fix assembly for the repro:

    ; print_r()
    ldur x0, [x29, #-96]          <- loads the sentinel
    str  x0, [sp, #-16]!
    ...                           <- writes "Array\n"
    bl   __rt_print_r_indexed     <- sentinel handed to the walker

No cbz, no comparison — so unlike #581 the crash happens inside the walker rather than at an inline header load.

Fix, and why it is not #646's fix

The guard uses the shared sentinels::emit_branch_if_null_container (zero or sentinel) and jumps to a label placed after the walker call, skipping both the Array\n write and the walk.

It deliberately does not reuse the branch target added in #646, because PHP's semantics differ and I checked them before implementing:

php -r 'echo "["; print_r(null); echo "]";'                          -> []
php -r '$s = print_r(null, true); echo strlen($s);'                  -> 0
php -r 'var_dump(print_r(null));'                                    -> bool(true)

print_r(null) prints nothing, where var_dump(null) prints NULL. Skipping every write is also what makes all three call modes correct with one branch: echo mode prints nothing and still returns true (the immediate is loaded later, in lower_print_r), and the capture modes leave the buffer empty so __rt_pr_finish yields "".

Tests

Eight regressions in tests/codegen/io/printing.rs — the natural home, next to the 20+ existing test_print_r_* cases. Six are behavioural and fail on the pre-fix build with program crashed; one guards the shapes that already worked; one asserts the emitted guard directly.

The assembly test was checked with an explicit negative control: reverting the source makes it fail on the host and under ELEPHC_TEST_TARGET=linux-x86_64 and linux-aarch64, with missing print_r null-container skip branch. So x86_64 is genuinely verified, not assumed.

Sibling shapes, all matching PHP after the fix: hash receiver with an array value, hash receiver with a hash value (the __rt_print_r_hash walker — it crashed too), a miss through ?? null, the return mode (print_r($x, true)), and the runtime-flag mode (print_r($x, $flag)). Return mode was the least obvious: it crashed without printing Array, because the text went into the capture buffer first, so its test asserts len=0 rather than merely the absence of a crash.

A miss inside a boxed mixed was already correct before this change — the boxing-boundary normalization from #585/#591 covers it — and is left alone rather than claimed.

Verification (macOS ARM64, 78a6a6590)

filter	result
print_r (incl. the 8 new)	42 passed, 0 failed
same, ELEPHC_IR_OPT=off	37 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
output_buffering (print_r writes through ob)	44 passed, 0 failed
var_dump	28 passed, 0 failed
sentinel	41 passed, 0 failed, 3 ignored
array_miss (#526)	20 passed, 0 failed
null_container (#556/#592)	2 passed, 0 failed
autovivify (#600/#592)	26 passed, 0 failed
all three call modes under --heap-debug	leak summary: clean, exit 0
cargo build / git diff --check	clean, zero warnings

Narrow filters only, per the project's test policy; CI owns the full matrix. Linux not executed locally (no Docker), but the x86_64 lowering is covered by the assembly test above.

Conflict note with #646

Checked with git merge-tree rather than assumed:

• debug.rs merges cleanly — fix(codegen): guard var_dump's container walk against the null-container sentinel (#581) #646 touches emit_var_dump_array (~line 535), this touches emit_print_r_array (~line 296), roughly 230 lines apart.
• tests do not collide: these live in tests/codegen/io/printing.rs, fix(codegen): guard var_dump's container walk against the null-container sentinel (#581) #646's in tests/codegen/regressions/arrays.rs.
• CHANGELOG.md conflicts, since both add a bullet under ## [Unreleased]. Routine rebase resolution: keep both lines. Merge order does not matter.

[greptile-apps]

Greptile Summary

This PR fixes a crash (SIGSEGV) in print_r() when the array- or hash-typed local carries the in-band null-container sentinel produced by a missed element read. The previous lowering had no null branch at all — unlike its var_dump sibling — so the sentinel was passed directly to the runtime walker as a live container pointer.

• Adds emit_branch_if_null_container at the top of emit_print_r_array, jumping past both the Array\n header write and the walker call, which correctly produces empty output for all three call modes (echo, return, and runtime-flag), matching PHP's print_r(null) semantics.
• Includes eight new regression tests covering indexed miss, hash-of-array miss, hash-of-hash miss, ?? null forwarding, return mode, runtime-flag mode, a guard for live containers, and an assembly-level ordering assertion verified on both linux-x86_64 and linux-aarch64.

Confidence Score: 5/5

• Safe to merge. The guard is a small, targeted insertion before any stack or I/O work in emit_print_r_array, it reuses a well-exercised sentinel helper, and the change is independently verified by an assembly-ordering test on both Linux targets in addition to six behavioural regression tests.
• The fix is minimal — four lines inserted at the top of a single function — and follows an established pattern used across dozens of other container-handling sites in the codebase. The test suite directly validates the crash cases that motivated the fix, the guard-for-live-containers test confirms no regression for the happy path, and the assembly test proves the guard ordering holds on x86_64 and AArch64, not just the host architecture.
• No files require special attention.

Important Files Changed

Filename	Overview
src/codegen/lower_inst/builtins/debug.rs	Adds emit_branch_if_null_container guard at the top of emit_print_r_array, branching past both the Array\n header write and the walker call when the container is null or carries the in-band sentinel. Correct placement, correct label, uses the established sentinel helper pattern consistently with emit_var_dump_array.
tests/codegen/io/printing.rs	Adds eight regression tests: six behavioral (indexed miss, hash miss of array value, hash miss of hash value, miss through ?? null, return mode, runtime-flag mode), one guard for previously-working shapes, and one assembly-level ordering test verifying the sentinel comparison precedes the walker call on both supported architectures.
CHANGELOG.md	Adds a changelog bullet under [Unreleased] describing the fix. Entry is detailed and matches the project's existing changelog style.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["emit_print_r_array(ctx, walker)"] --> B["emit_branch_if_null_container\n(result_reg, scratch_reg, skip_label)"]
    B --> C{null or sentinel?}
    C -- yes --> G["skip_label: (no output)"]
    C -- no --> D["emit_push_reg(result_reg)\nwrite 'Array\\n'\nemit_pop_reg(result_reg)"]
    D --> E["set indent = 0\n(AArch64: mov x1, #0\nx86_64: mov edi, 0)"]
    E --> F["emit_call_label(walker)\n__rt_print_r_indexed or\n__rt_print_r_hash"]
    F --> G
    G --> H["Ok(())"]

Loading

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

[mirchaemanuel]

CI note — the three Non-Codegen Tests failures are not caused by this change.

All three targets fail on the same unrelated test:

test eval_can_name_enum_cases_matches_php_case_numbering ... FAILED
  panicked at tests/eval_object_handle_tests.rs:112:5
  elephc compile failed:
  native project error: regex support requires managed native package pcre2

It is an eval/enum fixture failing on a missing managed native package — nothing to do with print_r or the sentinel guard. This branch is based on 9f74f5300, which predates the commits that introduced and then fixed that dependency:

a69cd5c89 feat: add managed native dependencies
eec7496f7 ci: initialize native cache at step runtime
ac0cbd507 test: seed managed pcre2 for eval CLI fixtures

None of those is in 9f74f5300 (checked), and all three are in current main. So rebasing onto current main should clear it. I'm leaving the rebase to you rather than force-pushing.

For what it is worth locally, on this branch: print_r_return_mode_heap_tests 6/6, builtin_parity_tests 8/8, var_export_and_strstr_result_tests 20/20, plus the filters listed in the PR body.

[mirchaemanuel]

Rebased onto current main (61aeafe73); head is a1274a4db. I said earlier that I would leave this rebase to you — it stayed conflicted for a week, so I did it rather than let it keep aging. Nothing but the base changed; the guard and its tests are untouched.

The only conflict was CHANGELOG.md, and only over placement: var_dump's entry for the same sentinel guard (issue #581) landed meanwhile through #646, and this branch adds print_r's. Both are kept, the print_r one on top. The diff against main is that single line.

This also clears the CI failure I reported here on July 29th. The three Non-Codegen Tests targets were failing on eval_can_name_enum_cases_matches_php_case_numbering for a missing managed pcre2 package — a consequence of the old base 9f74f5300, which predates a69cd5c89, eec7496f7 and ac0cbd507. The new base contains all three.

Locally on the new head, filtered to what this touches: print_r 42 passed / 0 failed, var_dump 34 passed / 0 failed — the second because both sentinel guards now live in builtins/debug.rs and I wanted it on record that they coexist rather than assume it from a clean merge. CI on this exact head is the terminal check.