Skip to content

JIT: Fix OP_DATA read from an unwritten slot in zend_jit_assign_dim_op() - #24451

Open
waroad wants to merge 1 commit into
php:PHP-8.4from
waroad:traj-s-20261017-181136-06-tracing-undef-key-compound-assign-miscompile-pr2
Open

waroad wants to merge 1 commit into
php:PHP-8.4from
waroad:traj-s-20261017-181136-06-tracing-undef-key-compound-assign-miscompile-pr2

Conversation

@waroad

@waroad waroad commented Oct 20, 2026

Copy link
Copy Markdown

In zend_jit_assign_dim_op(), when the array element may be a reference, the cold branch for typed references spills the OP_DATA register to its stack slot and then sets op3_addr = real_addr;. That reassignment also applies to the code emitted after the branch, so the normal +=/%=/... path then reads the right-hand operand from a stack slot that is only written on the typed-reference path. With the tracing JIT, $a[0] += $i + 1 on an array parameter adds a stale value (prints int(3) instead of int(6)), and the %= variant from the issue can divide by a stale zero and dies with SIGFPE. A CV on the right ($a[0] += $i) is affected too, also with the default JIT counters.

The fix passes real_addr to the helper call only, the same way zend_jit_assign_op() already does for its typed reference branch, and leaves op3_addr alone for the rest of the function.

The new test ext/opcache/tests/jit/gh24437.phpt covers the undefined-key case, an element that is a reference, an element that is a typed reference, and the %= case. Without the fix the first two cases print 3 and 13 instead of 6 and 16, and the %= case crashes with an FPE; with it, the test passes. The reproducers from the issue now match opcache.jit=off.

Here's the PR, as suggested. It is close to #24387: there a register wasn't loaded on all paths around the typed reference check; here the stack slot is written only on the typed-reference path but read on the normal one. zend_jit_assign_dim_op() already force-loads OP_DATA before the reference check, so the change from #24387 isn't needed here.

Targets PHP-8.4, where I built and tested it (debug + ASan/UBSan build); ext/opcache/tests/jit passes in tracing and function mode.

Fixes #24437.

Found with an LLM-based testing tool; I wrote this fix with help from Claude and reviewed it myself.

The typed-reference branch spilled OP_DATA to its stack slot and then set
op3_addr to that slot, so the code emitted after the branch also read
OP_DATA from the slot, which is only written on the typed-reference path.
Pass the slot to zend_jit_assign_op_to_typed_ref() only, as
zend_jit_assign_op() does.

Assisted-by: Claude

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
It's interesting because it's one of those cases where the code looks right, but due to a later code path it actually isn't right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JIT: tracing JIT gives wrong sum for $a[0] += $i + 1 on an array parameter in a loop

2 participants