JIT: persist the SHM op_array in trace exit_info - #21710
Conversation
|
It would be great to understand where the failure comes from. As I see, JIT_G(current_frame) is always set during trace compilation (it can't be NULL) , so this part of deduction can not be the reason of the failure. The fix generates more expensive JIT code that performs run-time resolution, I believe it should be possible to make this resolution at compile time. |
|
I can reproduce this bug consistently. If somone can help compile a Windows version, I can test this fix |
4c71cf2 to
7c69d4b
Compare
PR updated. |
| @@ -8094,7 +8094,7 @@ static int zend_jit_escape_if_undef(zend_jit_ctx *jit, int var, uint32_t flags, | |||
|
|
|||
| /* 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); | |||
| (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(jit->current_op_array); | |||
There was a problem hiding this comment.
Are you sure jit->current_op_array is always initialized?
It seems, it may be NULL on the following patch - zend_jit_trace_exit() -> zend_jit_trace_hot_side() -> zend_jit_blacklist_trace_exit() -> zend_jit_trace_exit_to_vm() - > zend_jit_trace_deoptimization() -> zend_jit_escape_if_undef().
May be I'm wrong...
There was a problem hiding this comment.
exit_info.op_array is written in one place, zend_jit_trace_get_exit_point (trace.c:197), and the surrounding code at trace.c:146-164 assigns op_array and stack_size together: JIT_G(current_frame) non-NULL gives a real op_array with some stack_size; the else branch sets both op_array = NULL and stack_size = 0 in the same step.
So on your path, ctx.current_op_array == NULL only when exit_info.stack_size == 0, which means zend_jit_trace_deoptimization loops over zero entries, check2 stays at -1, and zend_jit_escape_if_undef at trace.c:3606 isn't called. The NULL never reaches ZEND_FUNC_INFO(jit->current_op_array).
Can add ZEND_ASSERT(exit_info[exit_num].op_array || exit_info[exit_num].stack_size == 0) to make it explicit.
|
Should the original fix be reverted until we fully understand the issue? I don't have the time to look at the problem, and also don't have a Windows setup. Otherwise we'll miss the next release cycle (I don't feel comfortable merging this without an RC-phase). |
|
Possibly, same as you I cannot reproduce the issue due to lack of windows env, fix is made based on code analysis. Ultimately be nice to have windows build the reporter could try, maybe revert from 8.5/8.4 keep in master? And then try fully patched version for 8.4/8.5 back port after current release is out? |
|
|
|
Yes, |
|
I’ve built Windows binaries for this PR here: |
Oh, that's awesome thank you! I'll ask the original bug reporter to try, hopefully good news. Just to confirm these are based on latest state of the PR? |
Yes, it is based on the latest state of the PR. |
@vibbow Can you please try the builds shivammathur prepared (https://github.com/php/php-windows-builder/actions/runs/25253712864 ) if that fixes the issue. As you can tell from the convo, none of us are able to reproduce the issue. |
|
@iliaal @shivammathur The origional crash is happened on php 8.5.5 Today I tested with PHP 8.4.20 and PHP 8.4.21-dev (link provided), none of them crashed. And PHP 8.5.5 is still crashed. Can you made a php 8.5.6-dev build so I can test with it? |
…#21368) 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 GH-21368.
7c69d4b to
0777c51
Compare
zend_jit_escape_if_undef received an op_array pointer captured at parent-trace compile time. That pointer can go stale by the time a side trace compiles for the exit. Drop the parameter and read jit->current_op_array instead; zend_jit_trace_start sets it for trace compilation, and zend_jit_trace_exit_to_vm now seeds it from exit_info->op_array so the deoptimizer path has it too. Closes phpGH-21710
0777c51 to
3079a72
Compare
|
Today I upgrade to php 8.5.7, and still get this issue. And this time I created a full process crash dump. Hope this can help with solve this issue: Following is the crash dump analysis: |
|
Update: If I change opcache.jit from 1254 to 1205, it crashed at different place: |
|
Tried with PHP 8.5.8 If I set |
|
PHP 8.5.9, this issue still exists. But when I try to run PHP direct with So it must be something wrong only with IIS + PHP FastCGI, maybe the shared memory model. |
|
Since I'm guessing it's something wrong with shared memory, so I add following And PHP no longer crashs ! Hope this information can help you guys to debug this issue. |
|
@arnaud-lb Is this maybe something you can review? |
|
I'm able to reproduce on Linux: The root cause is that This can happen at least for methods of linked classes that couldn't be cached in the inheritance cache, because the op_array of these methods is copied to the heap during linking and never persisted back to SHM, but since the original op_array was instrumented with JIT, it is JIT-able. I guess that it happens more often on Windows because linked classes are more likely to not be cached on this platforms, but it can happen on others too. Reproducer: $pid = pcntl_fork();
if ($pid) {
pcntl_waitpid($pid, $status, 0);
$buf = [];
for ($i = 0; $i < 100; $i++) {
$buf[] = str_repeat('a', $i*100);
}
require 'test.inc';
for ($i = 0; $i < 1000; $i++) {
// Using getenv() to disable optimizations
// Using call_user_func() so that C::f() is the root of the trace
getenv('call_user_func')('C::f', [true]);
}
} else {
require 'test.inc';
for ($i = 0; $i < 1000; $i++) {
getenv('call_user_func')('C::f', [false]);
}
}// test.inc
if (getenv('call_user_func')) {
eval('class P {}');
}
// This class is not cached after linking because the parent is not in SHM
class C extends P {
static function f($v) {
// $v[0] triggers escape_if_undef when the type changes
return $v[0];
if ($a) {
return 1;
} else {
return 2;
}
}
}A possible fix is to change php-src/ext/opcache/jit/zend_jit_trace.c Line 203 in 2f3a1ec so that it fetches the original op_array from the func info as we do in php-src/ext/opcache/jit/zend_jit_trace.c Lines 7536 to 7539 in 2f3a1ec |
|
Independent reproduction, and a repro that needs neither IIS nor FastCGI — just ReproOfficial Windows builds from downloads.php.net, x64 TS, no extra software: ; php.ini
extension_dir = "ext"
extension=mbstring
extension=openssl
extension=fileinfo
extension=pdo_sqlite
extension=sqlite3
opcache.enable = 1
opcache.enable_cli = 1
opcache.memory_consumption = 256
opcache.interned_strings_buffer = 16
opcache.max_accelerated_files = 20000
opcache.validate_timestamps = 0
opcache.jit = tracing
opcache.jit_buffer_size = 64MThen BisectSame app, same ini, 150 requests per cell, one process per cell. "Served" counts
That window brackets #21368 (merged 2026-03-16 to Stack (8.5.9, official debug pack)
With where the parent (trace 201, Workarounds that holdBoth survive 150 requests on 8.5.9 where the default dies at 3:
Why this may matter more than the IIS report suggestsAny SAPI that keeps one process alive across many requests with a shared OPcache hits this, not just IIS+FastCGI — the built-in server above is single-process and reproduces it in seconds. It also bites embedded/long-lived SAPIs: we hit it in ePHPm (PHP embedded in a Rust application server), where a stock Laravel app killed the whole server process after three requests with no PHP error at all. We have turned the tracing JIT off by default on Windows until #21710 lands. Happy to test a patched build if that helps. |
|
Just some Saturday morning vibe coding and didn't know Claude would post all the above. Sorry about that, I'm trying to stay in my repos with all the slop and avoid upstream. Hope it's helpful though as I did double check the results. |
exit_info.op_array was taken from the current frame. Methods of linked classes that miss the inheritance cache use a heap copy of the op_array header, so that pointer is invalid in other processes and later requests. Store the original from the JIT extension, as root traces already do. Closes phpGH-21710
3079a72 to
3144bbb
Compare
|
Done, 3144bbb. |
iliaal
changed the title
exit_info.op_array was taken from the current frame. Methods of linked classes that miss the inheritance cache use a heap copy of the op_array header, so that pointer is invalid in other processes and later requests. Store the original from the JIT extension, as root traces already do. Closes phpGH-21710
3144bbb to
c0814c9
Compare
7 participants
zend_jit_trace_get_exit_pointstoredexit_info.op_arrayfrom the current frame. For methods of linked classes that miss the inheritance cache, that pointer is a heap copy of the op_array header. Trace metadata lives in SHM, so a later request or process then crashes inzend_jit_escape_if_undef. It now stores the original from the JIT extension, as root traces already do.