Fix GH-17868: Cannot allocate memory with tracing JIT on 8.4.4 #17869
Conversation
The generated code tries to initialize the run time cache for even internal closures, but it should only initialize the run time cache for user closures. We fix this by adding a check for the function type. If `func` is known, then we can check the type at code generation time.
ext/opcache/jit/zend_jit_ir.c
Outdated
| // JIT: if (closure->func.op_array.run_time_cache__ptr) | ||
| if_cond = ir_IF(ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_closure, func.op_array.run_time_cache__ptr)))); | ||
| ir_IF_FALSE(if_cond); |
There was a problem hiding this comment.
Why do we need this additional check?
We checked !RUN_TIME_CACHE(op_array) in zend_jit_init_func_run_time_cache_helper().
There was a problem hiding this comment.
I see. The check for !RUN_TIME_CACHE(op_array) already existed and I forgot that zend_jit_init_func_run_time_cache_helper() already made this check. We can remove the !RUN_TIME_CACHE check from the generated code. I'll commit that change.
On master, I see that zend_jit_init_func_run_time_cache_helper() also checks function type (not on PHP-8.4), but I think it's preferable to check it in the generated code so that we can sometimes skip generating a call to zend_jit_init_func_run_time_cache_helper() when func is known to be an internal function.
There was a problem hiding this comment.
The function type check on master was introduced in 097edc8.
The only other place that calls zend_jit_init_func_run_time_cache_helper() already knows that this is a user function.
Perhaps it's even beneficial to create 2 helper variants of zend_jit_init_func_run_time_cache_helper(): one with a function type check and one without? Then we could use the one with the check in zend_jit_push_call_frame if !func, otherwise call the helper that skips the check if it's a user function, and generate no call if it's an internal function.
There was a problem hiding this comment.
Please check, how this affects the generated JIT code size. E.g. function/tracing JIT on Symfony demo.
I suppose, this shouldn't make a big difference.
There was a problem hiding this comment.
Symfony-demo:
On function JIT:
tracing with check in IR code: 21268045
tracing with check in helper (2 helper variants): 21268045
So no difference somehow.
On tracing JIT the difference is very very small:
tracing with check in IR code: 628824
tracing with check in helper (2 helper variants): 628808
Only a difference of 16! So probably not worth creating helper variants.
* PHP-8.3: Backport GH-17869 to PHP 8.3 JIT
* PHP-8.4: Backport GH-17869 to PHP 8.3 JIT
The generated code tries to initialize the run time cache for even internal closures, but it should only initialize the run time cache for user closures. This results in an out of memory error because
op_array.cache_sizeis a nonsense value.We fix this by adding a check for the function type. If
funcis known, then we can check the type at code generation time.This was already an issue before 28b448a, but became more visible after that commit.
This also needs fixing on PHP-8.3, I can backport this after this is handled for 8.4.