Fix GH-21267: JIT infinite loop on FETCH_OBJ_R with IS_UNDEF property#21368
Merged
dstogov merged 1 commit intophp:PHP-8.4from Mar 16, 2026
Merged
Conversation
Member
|
I see the problem, but the fix looks too pessimistic. It adds a guard check that is going to be almost always useless. For now I simplified the test case . The first one fails on PHP-8.4 and above the second also fails on PHP-8.3 --TEST--
GH-21267 (JIT infinite loop on FETCH_OBJ_R with IS_UNDEF property in polymorphic context)
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit=tracing
opcache.jit_buffer_size=64M
opcache.jit_hot_loop=0
opcache.jit_hot_func=2
opcache.jit_hot_return=0
opcache.jit_hot_side_exit=1
--FILE--
class C {
public $x = true;
public function __get($name) { return null; }
public function getX() { return $this->x; }
}
$o1 = new C;
$o2 = new C;
$o2->x = false;
$o3 = new C;
unset($o3->x);
$a = [$o1, $o2, $o3];
for ($i = 0; $i < 8; $i++) {
$m = $a[$i % 3];
$m->getX();
$m->getX();
}
?>
OK
--EXPECT--
OK--TEST--
GH-21267 (JIT infinite loop on FETCH_OBJ_R with IS_UNDEF property in polymorphic context)
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit=tracing
opcache.jit_buffer_size=64M
opcache.jit_hot_loop=0
opcache.jit_hot_func=2
opcache.jit_hot_return=0
opcache.jit_hot_side_exit=1
--FILE--
class C {
public $x = true;
public function __get($name) { return null; }
public function getX() { return $this->x; }
}
$o1 = new C;
unset($o1->x);
$o2 = new C;
$a = [$o1, $o2, $o2];
for ($i = 0; $i < 8; $i++) {
$m = $a[$i % 3];
$m->getX();
$m->getX();
}
?>
OK
--EXPECT--
OK |
Member
|
Following is a better fix: diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index 19b3ce12573..899188d5423 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -7972,7 +7972,19 @@ static int zend_jit_escape_if_undef(zend_jit_ctx *jit, int var, uint32_t flags,
}
jit_LOAD_IP_ADDR(jit, opline - 1);
- ir_IJMP(jit_STUB_ADDR(jit, jit_stub_trace_escape));
+
+ /* We can't use trace_escape() because opcode handler may be overriden by JIT */
+ zend_jit_op_array_trace_extension *jit_extension =
+ (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(jit->current_op_array);
+ size_t offset = jit_extension->offset;
+ zend_vm_opcode_handler_func_t orig_handler =
+ ZEND_OP_TRACE_INFO((opline - 1), offset)->orig_handler;
+ ir_ref ref = ir_CONST_ADDR(orig_handler);
+ if (GCC_GLOBAL_REGS || ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL) {
+ zend_jit_tailcall_handler(jit, ref);
+ } else {
+ zend_jit_vm_enter(jit, ref);
+ }
ir_IF_TRUE(if_def);
This should be applied to PHP-8.4 and above. |
iliaal
added a commit
to iliaal/php-src
that referenced
this pull request
Mar 16, 2026
When the JIT defers the IS_UNDEF check for FETCH_OBJ_R to the result type guard, the deoptimization escape path dispatches to opline->handler via the trace_escape stub. If opline->handler has been overwritten with JIT code (e.g. a function entry trace), this creates an infinite loop. Fix by dispatching to the original VM handler (orig_handler from the trace extension) instead of going through the trace_escape stub. This avoids the extra IS_UNDEF guard on every property read while correctly handling the rare IS_UNDEF case during deoptimization. Also set current_op_array in zend_jit_trace_exit_to_vm so that the blacklisted exit deoptimizer can resolve orig_handler, covering the case where side trace compilation is exhausted. Closes phpGH-21368.
28e99da to
1d9aba6
Compare
Contributor
Author
|
@dstogov Thanks for the review and the better fix. Updated the PR based on your approach — One addition: I also set |
dstogov
reviewed
Mar 16, 2026
Comment on lines
+8097
to
+8107
| ir_ref ref = ir_CONST_ADDR(ZEND_OP_TRACE_INFO((opline - 1), offset)->orig_handler); | ||
| if (GCC_GLOBAL_REGS) { | ||
| ir_TAILCALL(IR_VOID, ref); | ||
| } else { | ||
| #if defined(IR_TARGET_X86) | ||
| ref = ir_CAST_FC_FUNC(ref); | ||
| #endif | ||
| ref = ir_CALL_1(IR_I32, ref, jit_FP(jit)); | ||
| ir_GUARD(ir_GE(ref, ir_CONST_I32(0)), jit_STUB_ADDR(jit, jit_stub_trace_halt)); | ||
| ir_RETURN(ir_CONST_I32(1)); // ZEND_VM_ENTER | ||
| } |
Member
There was a problem hiding this comment.
I see, my patch didn't handle all VM kinds right.
Anyway, I'm not completely sure if the "else" part of your patch is going to work for all VM kinds.
Contributor
Author
There was a problem hiding this comment.
The else branch matches the dispatch pattern used by zend_jit_trace_exit_stub (line 2527) and zend_jit_trace_return (line 17148) on PHP-8.4: ir_CALL_1(IR_I32, ...) + ir_GUARD + ir_RETURN(ZEND_VM_ENTER). On 8.4, JIT only runs with HYBRID VM kind — GCC_GLOBAL_REGS vs CALL convention covers both paths.
For a master forward-port this would need zend_jit_tailcall_handler/zend_jit_vm_enter to also handle ZEND_VM_KIND_TAILCALL.
Member
There was a problem hiding this comment.
It seems we can use TAILCALL for the else part.
ext/opcache/jit/zend_jit_trace.c
Outdated
Comment on lines
+7371
to
+7375
| /* Set current_op_array so that zend_jit_escape_if_undef() can | ||
| * resolve orig_handler to avoid dispatching to JIT-overwritten | ||
| * opline->handler (which would cause an infinite loop). */ | ||
| ctx.current_op_array = zend_jit_traces[zend_jit_traces[trace_num].root].op_array; | ||
|
|
Member
There was a problem hiding this comment.
if ctx.current_op_array may be uninitialized, better pass op_array into zend_jit_escape_if_undef() as a parameter.
Contributor
Author
There was a problem hiding this comment.
Done — zend_jit_escape_if_undef now takes op_array as an explicit parameter. The call site passes jit->current_op_array, which is set by zend_jit_trace_start in the side-trace path and by the new initialization in zend_jit_trace_exit_to_vm for the blacklist path.
Member
There was a problem hiding this comment.
I meant not to set ctx.current_op_array, but pass the value directly into zend_jit_escape_if_undef() completely removing this chunk.
iliaal
added a commit
to iliaal/php-src
that referenced
this pull request
Mar 16, 2026
When the JIT defers the IS_UNDEF check for FETCH_OBJ_R to the result type guard, the deoptimization escape path dispatches to opline->handler via the trace_escape stub. If opline->handler has been overwritten with JIT code (e.g. a function entry trace), this creates an infinite loop. Fix by dispatching to the original VM handler (orig_handler from the trace extension) instead of going through the trace_escape stub. This avoids the extra IS_UNDEF guard on every property read while correctly handling the rare IS_UNDEF case during deoptimization. Also set current_op_array in zend_jit_trace_exit_to_vm so that the blacklisted exit deoptimizer can resolve orig_handler, covering the case where side trace compilation is exhausted. Closes phpGH-21368.
1d9aba6 to
c95f761
Compare
dstogov
reviewed
Mar 16, 2026
Member
dstogov
left a comment
There was a problem hiding this comment.
I reduced the patch to
diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index 3ddaa027088..f5e821c2c66 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -8066,7 +8066,7 @@ static int zend_jit_defined(zend_jit_ctx *jit, const zend_op *opline, uint8_t sm
return 1;
}
-static int zend_jit_escape_if_undef(zend_jit_ctx *jit, int var, uint32_t flags, const zend_op *opline, int8_t reg)
+static int zend_jit_escape_if_undef(zend_jit_ctx *jit, int var, uint32_t flags, const zend_op *opline, const zend_op_array *op_array, int8_t reg)
{
zend_jit_addr reg_addr = ZEND_ADDR_REF_ZVAL(zend_jit_deopt_rload(jit, IR_ADDR, reg));
ir_ref if_def = ir_IF(jit_Z_TYPE(jit, reg_addr));
@@ -8089,7 +8089,21 @@ static int zend_jit_escape_if_undef(zend_jit_ctx *jit, int var, uint32_t flags,
}
jit_LOAD_IP_ADDR(jit, opline - 1);
- ir_IJMP(jit_STUB_ADDR(jit, jit_stub_trace_escape));
+
+ /* We can't use trace_escape() because opcode handler may be overridden by JIT */
+ zend_jit_op_array_trace_extension *jit_extension =
+ (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(op_array);
+ size_t offset = jit_extension->offset;
+ if (GCC_GLOBAL_REGS) {
+ ir_ref ref = ir_CONST_ADDR(ZEND_OP_TRACE_INFO((opline - 1), offset)->orig_handler);
+ ir_TAILCALL(IR_VOID, ref);
+ } else {
+ ir_ref ref = ir_CONST_ADDR(ZEND_OP_TRACE_INFO((opline - 1), offset)->call_handler);
+#if defined(IR_TARGET_X86)
+ ref = ir_CAST_FC_FUNC(ref);
+#endif
+ ir_TAILCALL_1(IR_I32, ref, jit_FP(jit));
+ }
ir_IF_TRUE(if_def);
diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c
index 03b59eea615..b5d980ca5af 100644
--- a/ext/opcache/jit/zend_jit_trace.c
+++ b/ext/opcache/jit/zend_jit_trace.c
@@ -3603,7 +3603,7 @@ static int zend_jit_trace_deoptimization(
ZEND_ASSERT(STACK_FLAGS(parent_stack, check2) == ZREG_ZVAL_COPY);
ZEND_ASSERT(reg != ZREG_NONE);
- if (!zend_jit_escape_if_undef(jit, check2, flags, opline, reg)) {
+ if (!zend_jit_escape_if_undef(jit, check2, flags, opline, exit_info->op_array, reg)) {
return 0;
}
if (!zend_jit_restore_zval(jit, EX_NUM_TO_VAR(check2), reg)) {Do you see any problems?
Merge to master is going to be not trivial because of tail-call VM...
ext/opcache/jit/zend_jit_trace.c
Outdated
Comment on lines
+7371
to
+7375
| /* Set current_op_array so that zend_jit_escape_if_undef() can | ||
| * resolve orig_handler to avoid dispatching to JIT-overwritten | ||
| * opline->handler (which would cause an infinite loop). */ | ||
| ctx.current_op_array = zend_jit_traces[zend_jit_traces[trace_num].root].op_array; | ||
|
|
Member
There was a problem hiding this comment.
I meant not to set ctx.current_op_array, but pass the value directly into zend_jit_escape_if_undef() completely removing this chunk.
Comment on lines
+8097
to
+8107
| ir_ref ref = ir_CONST_ADDR(ZEND_OP_TRACE_INFO((opline - 1), offset)->orig_handler); | ||
| if (GCC_GLOBAL_REGS) { | ||
| ir_TAILCALL(IR_VOID, ref); | ||
| } else { | ||
| #if defined(IR_TARGET_X86) | ||
| ref = ir_CAST_FC_FUNC(ref); | ||
| #endif | ||
| ref = ir_CALL_1(IR_I32, ref, jit_FP(jit)); | ||
| ir_GUARD(ir_GE(ref, ir_CONST_I32(0)), jit_STUB_ADDR(jit, jit_stub_trace_halt)); | ||
| ir_RETURN(ir_CONST_I32(1)); // ZEND_VM_ENTER | ||
| } |
Member
There was a problem hiding this comment.
It seems we can use TAILCALL for the else part.
Member
It seems we may always use |
When the JIT defers the IS_UNDEF check for FETCH_OBJ_R to the result type guard, the deoptimization escape path dispatches to opline->handler via the trace_escape stub. If opline->handler has been overwritten with JIT code (e.g. a function entry trace), this creates an infinite loop. Fix by dispatching to the original VM handler (orig_handler from the trace extension) instead of going through the trace_escape stub. This avoids the extra IS_UNDEF guard on every property read while correctly handling the rare IS_UNDEF case during deoptimization. Also set current_op_array in zend_jit_trace_exit_to_vm so that the blacklisted exit deoptimizer can resolve orig_handler, covering the case where side trace compilation is exhausted. Closes phpGH-21368.
Contributor
Author
|
Updated — adopted all suggestions:
|
Member
|
It's something wrong with github. |
c95f761 to
4cebe12
Compare
Contributor
Author
There was wrong branch set for diff, should be ok now. |
Member
|
I don't see problems. |
Contributor
Author
|
Cool, thanks your help :) |
|
This fix cause our application crash. We had to rollback to php 8.5.4 PHP 8.5.5 VS17 x64 Non Thread Safe on Windows Server 2022, running with IIS + FastCGI dump file track to this line of code crash php-src/ext/opcache/jit/zend_jit_ir.c Line 7980 in fb04706 |
iliaal
added a commit
to iliaal/php-src
that referenced
this pull request
Apr 10, 2026
…ndef PR php#21368 replaced the trace_escape stub dispatch in zend_jit_escape_if_undef with a compile-time constant load of orig_handler, computed from the exit info's op_array pointer. That pointer can be NULL (when current_frame is NULL at exit-point creation) or stale (when the underlying op_array is freed before the side trace compiles), producing an access violation inside zend_jit_escape_if_undef. Reported on PHP 8.5.5 Windows NTS. Drop the op_array parameter and emit a runtime lookup via zend_jit_orig_opline_handler() instead. That helper resolves the jit_extension through EX(func) at dispatch time, which is valid regardless of the compile-time op_array state. The gh21267 tests still pass, confirming the infinite-loop fix is preserved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #21267
When the JIT defers the
IS_UNDEFcheck forFETCH_OBJ_Rto the result type guard (theMAY_BE_GUARD+current_framepath), the deoptimization escape dispatches toopline->handlervia thetrace_escapestub. Ifopline->handlerhas been overwritten with JIT code (e.g. a function entry trace for a zero-param method whereFETCH_OBJ_Ris the first opline), this creates an infinite loop since no exit counters are bumped and no blacklisting occurs.The fix changes
zend_jit_escape_if_undef()to dispatch directly toorig_handler(the original VM handler stored in the trace extension) instead of going throughtrace_escape. This preserves the optimization of deferring theIS_UNDEFcheck to the result type guard while correctly handling the rare IS_UNDEF case during deoptimization.Additionally, sets
current_op_arrayinzend_jit_trace_exit_to_vm()so that the blacklisted exit deoptimizer can also resolveorig_handler, covering the case where side trace compilation is exhausted (jit_max_side_tracesreached).Test cases cover both the side trace path and the blacklisted exit path.