From 79a07ffc5ef4f5a7cb1100cf25179c5900bd45b9 Mon Sep 17 00:00:00 2001 From: mirchaemanuel Date: Sat, 11 Jul 2026 09:22:23 +0200 Subject: [PATCH] fix(codegen): guard container reads against null/sentinel receivers (#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 #526 Claude-Session: https://claude.ai/code/session_01Jfn7uqEH5Dog1nBzTM5DCW --- src/codegen/lower_inst/arrays.rs | 18 ++ src/codegen/lower_inst/builtins/isset.rs | 8 +- src/codegen/lower_inst/iterators.rs | 15 ++ src/codegen/lower_inst/predicates.rs | 26 +++ .../runtime/arrays/array_get_mixed_key.rs | 14 ++ .../runtime/arrays/hash_get.rs | 14 ++ .../runtime/arrays/hash_iter.rs | 17 ++ src/codegen_support/sentinels.rs | 27 +++ src/ir_lower/expr/mod.rs | 32 +++- tests/codegen/regressions/arrays.rs | 170 ++++++++++++++++++ 10 files changed, 336 insertions(+), 5 deletions(-) diff --git a/src/codegen/lower_inst/arrays.rs b/src/codegen/lower_inst/arrays.rs index 4afaee788d..0045a4c4a1 100644 --- a/src/codegen/lower_inst/arrays.rs +++ b/src/codegen/lower_inst/arrays.rs @@ -614,8 +614,16 @@ fn lower_array_get_aarch64( ctx.load_value_to_reg(index, result_reg)?; ctx.load_value_to_reg(array, array_reg)?; let null_label = ctx.next_label("array_get_null"); + let null_receiver_label = ctx.next_label("array_get_null_recv"); let done_label = ctx.next_label("array_get_done"); + // -- guard the receiver: a missed outer read carries a null/sentinel container -- + crate::codegen::sentinels::emit_branch_if_null_container( + ctx.emitter, + array_reg, + len_reg, + &null_receiver_label, + ); ctx.emitter.instruction(&format!("cmp {}, #0", result_reg)); // check whether the indexed-array offset is negative ctx.emitter.instruction(&format!("b.lt {}", null_label)); // negative indexed-array offsets read as null abi::emit_load_from_address(ctx.emitter, len_reg, array_reg, 0); @@ -627,6 +635,7 @@ fn lower_array_get_aarch64( if warn_on_missing { emit_undefined_array_key_warning(ctx); } + ctx.emitter.label(&null_receiver_label); emit_array_get_null_fallback(ctx, result_ty); ctx.emitter.label(&done_label); store_if_result(ctx, inst) @@ -693,8 +702,16 @@ fn lower_array_get_x86_64( ctx.load_value_to_reg(array, array_reg)?; ctx.load_value_to_reg(index, result_reg)?; let null_label = ctx.next_label("array_get_null"); + let null_receiver_label = ctx.next_label("array_get_null_recv"); let done_label = ctx.next_label("array_get_done"); + // -- guard the receiver: a missed outer read carries a null/sentinel container -- + crate::codegen::sentinels::emit_branch_if_null_container( + ctx.emitter, + array_reg, + len_reg, + &null_receiver_label, + ); ctx.emitter.instruction(&format!("cmp {}, 0", result_reg)); // check whether the indexed-array offset is negative ctx.emitter.instruction(&format!("jl {}", null_label)); // negative indexed-array offsets read as null abi::emit_load_from_address(ctx.emitter, len_reg, array_reg, 0); @@ -706,6 +723,7 @@ fn lower_array_get_x86_64( if warn_on_missing { emit_undefined_array_key_warning(ctx); } + ctx.emitter.label(&null_receiver_label); emit_array_get_null_fallback(ctx, result_ty); ctx.emitter.label(&done_label); store_if_result(ctx, inst) diff --git a/src/codegen/lower_inst/builtins/isset.rs b/src/codegen/lower_inst/builtins/isset.rs index c5a3b4acc7..2306d7e871 100644 --- a/src/codegen/lower_inst/builtins/isset.rs +++ b/src/codegen/lower_inst/builtins/isset.rs @@ -74,7 +74,9 @@ fn emit_isset_missing_result(ctx: &mut FunctionContext<'_>, value: ValueId) -> R if let Some(inst) = source_instruction(ctx, value)? { match inst.op { Op::StrCharAt => return emit_isset_string_offset_missing_result(ctx, &inst), - Op::ArrayGet => return emit_isset_array_offset_missing_result(ctx, &inst), + Op::ArrayGet | Op::ArrayGetSilent => { + return emit_isset_array_offset_missing_result(ctx, &inst) + } Op::HashGet => return emit_isset_hash_offset_missing_result(ctx, &inst), _ => {} } @@ -317,6 +319,8 @@ fn emit_isset_array_offset_missing_aarch64( let result_reg = abi::int_result_reg(ctx.emitter); ctx.load_value_to_reg(index, result_reg)?; ctx.load_value_to_reg(array, array_reg)?; + // -- guard the receiver: a missed outer read carries a null/sentinel container -- + crate::codegen::sentinels::emit_branch_if_null_container(ctx.emitter, array_reg, len_reg, missing); ctx.emitter.instruction(&format!("cmp {}, #0", result_reg)); // reject negative indexes as missing array elements ctx.emitter.instruction(&format!("b.lt {}", missing)); // missing indexes make isset return false abi::emit_load_from_address(ctx.emitter, len_reg, array_reg, 0); @@ -344,6 +348,8 @@ fn emit_isset_array_offset_missing_x86_64( let result_reg = abi::int_result_reg(ctx.emitter); ctx.load_value_to_reg(array, array_reg)?; ctx.load_value_to_reg(index, result_reg)?; + // -- guard the receiver: a missed outer read carries a null/sentinel container -- + crate::codegen::sentinels::emit_branch_if_null_container(ctx.emitter, array_reg, len_reg, missing); ctx.emitter.instruction(&format!("cmp {}, 0", result_reg)); // reject negative indexes as missing array elements ctx.emitter.instruction(&format!("jl {}", missing)); // missing indexes make isset return false abi::emit_load_from_address(ctx.emitter, len_reg, array_reg, 0); diff --git a/src/codegen/lower_inst/iterators.rs b/src/codegen/lower_inst/iterators.rs index 3b4473d8e0..225082e9f1 100644 --- a/src/codegen/lower_inst/iterators.rs +++ b/src/codegen/lower_inst/iterators.rs @@ -833,11 +833,26 @@ fn store_iterator_cursor(ctx: &mut FunctionContext<'_>, offset: usize, cursor: i /// Snapshots the indexed-array length into the iterator state so `IterNext` compares /// against the original length rather than re-reading it each iteration. PHP snapshots /// the array length at loop entry, so elements appended during iteration are not visited. +/// A null/sentinel source (a missed outer array read) snapshots length zero so the loop +/// body never runs instead of dereferencing the sentinel as an array header. fn snapshot_indexed_array_length(ctx: &mut FunctionContext<'_>, offset: usize) { let array_reg = abi::int_result_reg(ctx.emitter); let len_reg = abi::secondary_scratch_reg(ctx.emitter); + let null_label = ctx.next_label("iter_len_null_source"); + let done_label = ctx.next_label("iter_len_done"); abi::load_at_offset(ctx.emitter, array_reg, offset - ITER_SOURCE_OFFSET_DELTA); + // -- guard the source: a missed outer read carries a null/sentinel container -- + crate::codegen::sentinels::emit_branch_if_null_container( + ctx.emitter, + array_reg, + len_reg, + &null_label, + ); abi::emit_load_from_address(ctx.emitter, len_reg, array_reg, 0); + abi::emit_jump(ctx.emitter, &done_label); + ctx.emitter.label(&null_label); + abi::emit_load_int_immediate(ctx.emitter, len_reg, 0); + ctx.emitter.label(&done_label); abi::store_at_offset(ctx.emitter, len_reg, offset - ITER_SNAPSHOT_LEN_OFFSET_DELTA); } diff --git a/src/codegen/lower_inst/predicates.rs b/src/codegen/lower_inst/predicates.rs index 5c5f170512..27cf23c919 100644 --- a/src/codegen/lower_inst/predicates.rs +++ b/src/codegen/lower_inst/predicates.rs @@ -107,6 +107,11 @@ pub(super) fn emit_is_null_result(ctx: &mut FunctionContext<'_>, value: ValueId) emit_int_result_null_sentinel_bool(ctx); Ok(()) } + PhpType::Array(_) | PhpType::AssocArray { .. } | PhpType::Iterable | PhpType::Object(_) => { + ctx.load_value_to_result(value)?; + emit_int_result_null_container_bool(ctx); + Ok(()) + } _ => { abi::emit_load_int_immediate(ctx.emitter, abi::int_result_reg(ctx.emitter), 0); Ok(()) @@ -114,6 +119,27 @@ pub(super) fn emit_is_null_result(ctx: &mut FunctionContext<'_>, value: ValueId) } } +/// Compares the loaded container pointer against PHP's null representations — a zero +/// pointer or the in-band null-container sentinel produced by missed reads of refcounted +/// slots — and materializes the boolean result. +fn emit_int_result_null_container_bool(ctx: &mut FunctionContext<'_>) { + let null_label = ctx.next_label("is_null_container"); + let done_label = ctx.next_label("is_null_container_done"); + let result_reg = abi::int_result_reg(ctx.emitter); + let scratch_reg = abi::secondary_scratch_reg(ctx.emitter); + crate::codegen::sentinels::emit_branch_if_null_container( + ctx.emitter, + result_reg, + scratch_reg, + &null_label, + ); + abi::emit_load_int_immediate(ctx.emitter, result_reg, 0); + abi::emit_jump(ctx.emitter, &done_label); + ctx.emitter.label(&null_label); + abi::emit_load_int_immediate(ctx.emitter, result_reg, 1); + ctx.emitter.label(&done_label); +} + /// Compares the loaded tagged-scalar tag register against PHP's null tag. fn emit_tagged_scalar_null_bool(ctx: &mut FunctionContext<'_>) { match ctx.emitter.target.arch { diff --git a/src/codegen_support/runtime/arrays/array_get_mixed_key.rs b/src/codegen_support/runtime/arrays/array_get_mixed_key.rs index 70f928a85d..8038cd3161 100644 --- a/src/codegen_support/runtime/arrays/array_get_mixed_key.rs +++ b/src/codegen_support/runtime/arrays/array_get_mixed_key.rs @@ -45,6 +45,13 @@ pub fn emit_array_get_mixed_key(emitter: &mut Emitter) { emitter.instruction("str x3, [sp, #24]"); // save whether missing keys should emit PHP warnings emitter.instruction("cbz x0, __rt_array_get_mixed_key_null"); // null array → Mixed(null) + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "x9", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp x0, x9"); // does the receiver carry the in-band null-container sentinel? + emitter.instruction("b.eq __rt_array_get_mixed_key_null"); // sentinel-null receivers from missed reads → Mixed(null) // -- dispatch on array storage kind -- emitter.instruction("ldr x9, [x0, #-8]"); // load packed kind metadata from the array header @@ -197,6 +204,13 @@ fn emit_array_get_mixed_key_linux_x86_64(emitter: &mut Emitter) { emitter.instruction("test rdi, rdi"); // null array check emitter.instruction("je __rt_array_get_mixed_key_null"); // null array → Mixed(null) + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "r9", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp rdi, r9"); // does the receiver carry the in-band null-container sentinel? + emitter.instruction("je __rt_array_get_mixed_key_null"); // sentinel-null receivers from missed reads → Mixed(null) // -- dispatch on array storage kind -- emitter.instruction("mov r9, QWORD PTR [rdi - 8]"); // load packed kind metadata from the array header diff --git a/src/codegen_support/runtime/arrays/hash_get.rs b/src/codegen_support/runtime/arrays/hash_get.rs index 60b3ecb8ab..83c9d45c75 100644 --- a/src/codegen_support/runtime/arrays/hash_get.rs +++ b/src/codegen_support/runtime/arrays/hash_get.rs @@ -42,6 +42,13 @@ pub fn emit_hash_get(emitter: &mut Emitter) { emitter.instruction("str x1, [sp, #8]"); // save key_ptr emitter.instruction("str x2, [sp, #16]"); // save key_len emitter.instruction("cbz x0, __rt_hash_get_not_found"); // null tables cannot contain the requested key + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "x5", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp x0, x5"); // does the receiver carry the in-band null-container sentinel? + emitter.instruction("b.eq __rt_hash_get_not_found"); // sentinel-null receivers from missed reads cannot contain the key emitter.instruction("ldr x5, [x0, #8]"); // load capacity before hashing to avoid division by zero on empty tables emitter.instruction("cbz x5, __rt_hash_get_not_found"); // zero-capacity tables cannot contain the requested key @@ -145,6 +152,13 @@ fn emit_hash_get_linux_x86_64(emitter: &mut Emitter) { emitter.instruction("mov QWORD PTR [rbp - 24], rdx"); // save the key length across helper calls and probe iterations emitter.instruction("test rdi, rdi"); // null tables cannot contain the requested key emitter.instruction("jz __rt_hash_get_not_found"); // return a miss before reading a null table header + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "r11", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp rdi, r11"); // does the receiver carry the in-band null-container sentinel? + emitter.instruction("je __rt_hash_get_not_found"); // sentinel-null receivers from missed reads cannot contain the key emitter.instruction("mov r11, QWORD PTR [rdi + 8]"); // load capacity before hashing to avoid division by zero on empty tables emitter.instruction("test r11, r11"); // zero capacity means there are no live entries to probe emitter.instruction("jz __rt_hash_get_not_found"); // return a miss for empty hash tables diff --git a/src/codegen_support/runtime/arrays/hash_iter.rs b/src/codegen_support/runtime/arrays/hash_iter.rs index 3d97cc017f..7430cbdec1 100644 --- a/src/codegen_support/runtime/arrays/hash_iter.rs +++ b/src/codegen_support/runtime/arrays/hash_iter.rs @@ -48,6 +48,14 @@ pub fn emit_hash_iter(emitter: &mut Emitter) { // >0 = slot index + 1 of the next entry to return // -2 = post-last cursor returned with the final yielded entry // -1 = no more entries + emitter.instruction("cbz x0, __rt_hash_iter_end"); // null tables from missed reads have nothing to iterate + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "x7", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp x0, x7"); // does the table carry the in-band null-container sentinel? + emitter.instruction("b.eq __rt_hash_iter_end"); // sentinel-null tables from missed reads have nothing to iterate emitter.instruction("cmp x1, #-1"); // has the caller already consumed the end sentinel? emitter.instruction("b.eq __rt_hash_iter_end"); // repeated end probes stay at done emitter.instruction("cmp x1, #-2"); // was the previous yielded entry the tail? @@ -109,6 +117,15 @@ fn emit_hash_iter_linux_x86_64(emitter: &mut Emitter) { emitter.comment("--- runtime: hash_iter_next ---"); emitter.label_global("__rt_hash_iter_next"); + emitter.instruction("test rdi, rdi"); // null tables from missed reads have nothing to iterate + emitter.instruction("jz __rt_hash_iter_end"); // return the done sentinel before reading a null table header + crate::codegen_support::abi::emit_load_int_immediate( + emitter, + "r11", + crate::codegen_support::sentinels::NULL_SENTINEL, + ); + emitter.instruction("cmp rdi, r11"); // does the table carry the in-band null-container sentinel? + emitter.instruction("je __rt_hash_iter_end"); // sentinel-null tables from missed reads have nothing to iterate emitter.instruction("cmp rsi, -1"); // has the caller already consumed the terminal done sentinel? emitter.instruction("je __rt_hash_iter_end"); // repeated end probes keep returning the done sentinel emitter.instruction("cmp rsi, -2"); // did the previous yielded entry already encode the post-last cursor? diff --git a/src/codegen_support/sentinels.rs b/src/codegen_support/sentinels.rs index 5685085262..453740d8b4 100644 --- a/src/codegen_support/sentinels.rs +++ b/src/codegen_support/sentinels.rs @@ -116,6 +116,33 @@ pub(crate) fn emit_tagged_scalar_from_int_result(emitter: &mut Emitter) { } } +/// Branches to `label` when the container pointer in `value_reg` represents PHP null: +/// either a zero pointer or the in-band `NULL_SENTINEL` that missed reads of refcounted +/// slots materialize. Clobbers `scratch_reg` with the sentinel bit pattern. Used to keep +/// container reads from dereferencing a null/sentinel receiver (issue #526). +pub(crate) fn emit_branch_if_null_container( + emitter: &mut Emitter, + value_reg: &str, + scratch_reg: &str, + label: &str, +) { + match emitter.target.arch { + Arch::AArch64 => { + emitter.instruction(&format!("cbz {}, {}", value_reg, label)); // zero container pointers take the caller's null path + super::abi::emit_load_int_immediate(emitter, scratch_reg, NULL_SENTINEL); + emitter.instruction(&format!("cmp {}, {}", value_reg, scratch_reg)); // does the container carry the in-band null sentinel? + emitter.instruction(&format!("b.eq {}", label)); // sentinel-null containers take the caller's null path + } + Arch::X86_64 => { + emitter.instruction(&format!("test {}, {}", value_reg, value_reg)); // is the container pointer zero (PHP null)? + emitter.instruction(&format!("jz {}", label)); // zero container pointers take the caller's null path + super::abi::emit_load_int_immediate(emitter, scratch_reg, NULL_SENTINEL); + emitter.instruction(&format!("cmp {}, {}", value_reg, scratch_reg)); // does the container carry the in-band null sentinel? + emitter.instruction(&format!("je {}", label)); // sentinel-null containers take the caller's null path + } + } +} + /// Branches to `label` when the tagged scalar in the result registers is PHP null /// (tag register == null tag). pub(crate) fn emit_branch_if_tagged_scalar_null(emitter: &mut Emitter, label: &str) { diff --git a/src/ir_lower/expr/mod.rs b/src/ir_lower/expr/mod.rs index 71a2f70ede..4ac12599a8 100644 --- a/src/ir_lower/expr/mod.rs +++ b/src/ir_lower/expr/mod.rs @@ -1877,7 +1877,13 @@ fn lower_lazy_isset( let merge = ctx.builder.create_named_block("isset.lazy_merge", Vec::new()); for (idx, arg) in args.iter().enumerate() { let checked = lower_lazy_isset_operand(ctx, arg).unwrap_or_else(|| { - let value = lower_expr(ctx, arg); + // `isset()` never emits undefined-offset warnings, so eager array + // operands must be lowered with the silent read variants. + let value = if let ExprKind::ArrayAccess { array, index } = &arg.kind { + lower_array_access_with_missing_warning(ctx, array, index, arg, false) + } else { + lower_expr(ctx, arg) + }; emit_builtin_call_value(ctx, name, vec![value.value], PhpType::Int, arg.span) }); let then_target = if idx + 1 == args.len() { @@ -2108,7 +2114,7 @@ fn lower_native_isset_offset_probe( index: &Expr, expr: &Expr, ) -> LoweredValue { - let array_value = lower_expr(ctx, array); + let array_value = lower_subscript_receiver_silently(ctx, array); if value_is_nullable(ctx, array_value.value) { return lower_nullable_native_isset_offset_probe(ctx, array_value, index, expr); } @@ -7215,7 +7221,9 @@ fn lower_array_access( } /// Lowers array, hash, string, or ArrayAccess indexing with configurable -/// undefined-offset warning behavior for native indexed-array reads. +/// undefined-offset warning behavior for native indexed-array reads. Suppressed +/// warnings propagate through the whole subscript chain: PHP's `isset()` and `??` +/// are silent for every level of `$a[1][2][3]`, not just the outermost read. fn lower_array_access_with_missing_warning( ctx: &mut LoweringContext<'_, '_>, array: &Expr, @@ -7223,13 +7231,29 @@ fn lower_array_access_with_missing_warning( expr: &Expr, warn_on_missing: bool, ) -> LoweredValue { - let array_value = lower_expr(ctx, array); + let array_value = if warn_on_missing { + lower_expr(ctx, array) + } else { + lower_subscript_receiver_silently(ctx, array) + }; if value_is_nullable(ctx, array_value.value) { return lower_nullable_array_access(ctx, array_value, index, expr, warn_on_missing); } lower_array_access_from_value(ctx, array_value, index, expr, warn_on_missing) } +/// Lowers a subscript-chain receiver with undefined-offset warnings suppressed on +/// nested array reads, so `isset()`/`??` stay silent across chained subscripts. +fn lower_subscript_receiver_silently( + ctx: &mut LoweringContext<'_, '_>, + array: &Expr, +) -> LoweredValue { + if let ExprKind::ArrayAccess { array: inner_array, index: inner_index } = &array.kind { + return lower_array_access_with_missing_warning(ctx, inner_array, inner_index, array, false); + } + lower_expr(ctx, array) +} + /// Lowers array access once the receiver is already evaluated. fn lower_array_access_from_value( ctx: &mut LoweringContext<'_, '_>, diff --git a/tests/codegen/regressions/arrays.rs b/tests/codegen/regressions/arrays.rs index 97e673763a..fbb74568a8 100644 --- a/tests/codegen/regressions/arrays.rs +++ b/tests/codegen/regressions/arrays.rs @@ -965,3 +965,173 @@ echo $a[0], ":", $b[0]; ); assert_eq!(out, "5:6"); } + +// --- Issue #526: chained subscript read with a miss on the FIRST index --- + +/// Regression for issue #526: a chained subscript read whose FIRST index misses +/// must warn and propagate null instead of dereferencing the scalar null +/// sentinel as an array pointer (historical SIGSEGV in the outer length load). +#[test] +fn test_chained_read_first_index_miss_warns_and_yields_null() { + let out = compile_and_run_capture( + r#" ['x' => 1, 'y' => 2], 'b' => ['x' => 3]]; +$v = $m['nope']['x']; +echo is_null($v) ? 'null' : 'notnull'; +"#, + ); + assert!(out.success, "program crashed: {}", out.stderr); + assert_eq!(out.stdout, "null"); +} + +/// Regression for issue #526: a three-level chain with a miss at each level +/// yields null at every level without crashing. +#[test] +fn test_three_level_chain_miss_at_each_level_yields_null() { + let out = compile_and_run_capture( + r#"