From c5d3a9ceab9ea51b8dbee54959d76a6bb62922b0 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 23 Jul 2024 15:34:20 -0600 Subject: [PATCH 01/10] Check VM interrupt while internal frame is on top --- UPGRADING.INTERNALS | 5 ++ Zend/tests/fibers/signal-async.phpt | 11 ++-- Zend/zend.h | 15 +++++ Zend/zend_vm_def.h | 33 +++++++++- Zend/zend_vm_execute.h | 99 ++++++++++++++++++++++++++--- 5 files changed, 146 insertions(+), 17 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 56882988ff81b..3cc95301e0970 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -333,6 +333,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES 4. OpCode changes ======================== +* DO_ICALL, DO_FCALL, and DO_FCALL_BY_NAME now call zend_interrupt_function + while the internal frame is still on the stack. This means interrupt handlers + will now see the internal call. If your interrupt handler does something like + switching EG(current_execute_data), it should not do so if an internal func + is on top. * New FRAMELESS_ICALL_[0,3] opcodes for faster internal function calls have been added. These opcodes don't create a stack frame, but pass arguments via opcode operands. They only work for functions that are known at compile-time, and diff --git a/Zend/tests/fibers/signal-async.phpt b/Zend/tests/fibers/signal-async.phpt index 2487f874d30dc..9543e8e079d1f 100644 --- a/Zend/tests/fibers/signal-async.phpt +++ b/Zend/tests/fibers/signal-async.phpt @@ -17,7 +17,7 @@ pcntl_signal(SIGUSR1, function (): void { $fiber = new Fiber(function (): void { echo "Fiber start\n"; posix_kill(posix_getpid(), SIGUSR1); - time_nanosleep(1); + time_nanosleep(1, 0); echo "Fiber end\n"; }); @@ -30,8 +30,9 @@ Fiber start Fatal error: Uncaught FiberError: Cannot switch fibers in current execution context in %ssignal-async.php:%d Stack trace: #0 %ssignal-async.php(%d): Fiber::suspend() -#1 %ssignal-async.php(%d): {closure:%s:%d}(%d, Array) -#2 [internal function]: {closure:%s:%d}() -#3 %ssignal-async.php(%d): Fiber->start() -#4 {main} +#1 [internal function]: {closure:%s:%d}(%d, Array) +#2 %ssignal-async.php(%d): posix_kill(%d, %d) +#3 [internal function]: {closure:%s:%d}() +#4 %ssignal-async.php(%d): Fiber->start() +#5 {main} thrown in %ssignal-async.php on line %d diff --git a/Zend/zend.h b/Zend/zend.h index 4fe0703d42f69..1506b868e1690 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -341,7 +341,22 @@ extern ZEND_API size_t (*zend_printf)(const char *format, ...) ZEND_ATTRIBUTE_PT extern ZEND_API zend_write_func_t zend_write; extern ZEND_API FILE *(*zend_fopen)(zend_string *filename, zend_string **opened_path); extern ZEND_API void (*zend_ticks_function)(int ticks); + +/* Called by the VM in certain places like at the loop header, user function + * entry, and after internal function calls, if EG(vm_interrupt) has been set. + * + * If this is used to switch the EG(current_execute_data), such as implementing + * a coroutine scheduler, then it needs to check the top frame to see if it's + * an internal function. If an internal function is on top, then the frame + * shouldn't be switched away. + * + * Prior to PHP 8.0, this check was not necessary. In PHP 8.0, + * zend_call_function started calling zend_interrupt_function, and in 8.4 the + * DO_*CALL* opcodes started calling the zend_interrupt_function while the + * internal frame is still on top. + */ extern ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data); + extern ZEND_API void (*zend_error_cb)(int type, zend_string *error_filename, const uint32_t error_lineno, zend_string *message); extern ZEND_API void (*zend_on_timeout)(int seconds); extern ZEND_API zend_result (*zend_stream_open_function)(zend_file_handle *handle); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 77c1487b65c62..40d6e19f10b93 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -4075,6 +4075,14 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -4097,7 +4105,8 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -4196,6 +4205,14 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER)) #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; ZEND_VM_C_GOTO(fcall_by_name_end); @@ -4225,7 +4242,8 @@ ZEND_VM_C_LABEL(fcall_by_name_end): zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -4316,6 +4334,14 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; ZEND_VM_C_GOTO(fcall_end); @@ -4344,7 +4370,8 @@ ZEND_VM_C_LABEL(fcall_end): HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index b32ef2a6cc132..edaafa53d33ea 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1299,6 +1299,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1321,7 +1329,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1361,6 +1370,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1383,7 +1400,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1425,6 +1443,14 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_OBS #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1447,7 +1473,8 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_OBS HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1591,6 +1618,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_by_name_end; @@ -1620,7 +1655,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1691,6 +1727,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_by_name_end; @@ -1720,7 +1764,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1794,6 +1839,14 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_ #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_by_name_end; @@ -1823,7 +1876,8 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_ zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -1912,6 +1966,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_end; @@ -1940,7 +2002,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -2029,6 +2092,14 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_end; @@ -2057,7 +2128,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } @@ -2148,6 +2220,14 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_OBS #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(execute_data); + } + } + EG(current_execute_data) = execute_data; goto fcall_end; @@ -2176,7 +2256,8 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_OBS HANDLE_EXCEPTION(); } - ZEND_VM_SET_OPCODE(opline + 1); + CHECK_SYMBOL_TABLES() + OPLINE = opline + 1; ZEND_VM_CONTINUE(); } From 053090f5878c1e35e238c93e0aaa96ac46ca1d5d Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 26 Jul 2024 09:27:17 -0600 Subject: [PATCH 02/10] Use tab instead of spaces --- ext/opcache/jit/zend_jit_ir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 80099291d886d..e0da8b6289611 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -10342,7 +10342,7 @@ static int zend_jit_constructor(zend_jit_ctx *jit, const zend_op *opline, const } } - /* override predecessors of the next block */ + /* override predecessors of the next block */ ZEND_ASSERT(jit->ssa->cfg.blocks[next_block].predecessors_count == 1); if (!jit->ctx.control) { ZEND_ASSERT(jit->bb_edges[jit->bb_predecessors[next_block]]); From 72369cedacb602bca3f6f3785cc87f6af81cab86 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 26 Jul 2024 13:27:28 -0600 Subject: [PATCH 03/10] fix frame used in interrupt and refactor --- Zend/zend_execute.c | 9 +++++ Zend/zend_execute.h | 12 +++++++ Zend/zend_vm_def.h | 27 ++------------- Zend/zend_vm_execute.h | 75 +++++------------------------------------- 4 files changed, 33 insertions(+), 90 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 89904af06d2ad..09678469fc00e 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4056,6 +4056,15 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec } /* }}} */ +ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call) +{ + if (zend_atomic_bool_load_ex(&EG(timed_out))) { + zend_timeout(); + } else if (zend_interrupt_function) { + zend_interrupt_function(call); + } +} + #define ZEND_VM_INTERRUPT_CHECK() do { \ if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \ ZEND_VM_INTERRUPT(); \ diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index fe854f305150c..143d0adb55986 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -539,6 +539,18 @@ ZEND_COLD void zend_magic_get_property_type_inconsistency_error(const zend_prope ZEND_COLD void zend_match_unhandled_error(const zval *value); +/* Call this to handle the timeout or the interrupt function. It will not clear + * the EG(vm_interrupt). + */ +ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call); + +static zend_always_inline void zend_interrupt_or_timeout_check(zend_execute_data *call) +{ + if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) { + zend_interrupt_or_timeout(call); + } +} + static zend_always_inline void *zend_get_bad_ptr(void) { ZEND_UNREACHABLE(); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 40d6e19f10b93..826dd41d9aa32 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -4074,14 +4074,7 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -4204,14 +4197,7 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -4333,14 +4319,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index edaafa53d33ea..a7a6ca63d1a48 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1299,13 +1299,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1370,13 +1364,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1442,14 +1430,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_OBS } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1618,13 +1599,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -1727,13 +1702,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -1838,14 +1807,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_ } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -1966,13 +1928,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -2092,13 +2048,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; @@ -2219,14 +2169,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_OBS } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - - if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { - if (zend_atomic_bool_load_ex(&EG(timed_out))) { - zend_timeout(); - } else if (zend_interrupt_function) { - zend_interrupt_function(execute_data); - } - } + zend_interrupt_or_timeout_check(call); EG(current_execute_data) = execute_data; From c56ff0e829292625281dfbbbed8729ae6a0ace7c Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 26 Jul 2024 14:52:01 -0600 Subject: [PATCH 04/10] remove unused failures for zend_jit_check_timeout --- ext/opcache/jit/zend_jit.c | 4 +--- ext/opcache/jit/zend_jit_ir.c | 11 +++-------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index 17333fc0d5067..343cb7b946330 100644 --- a/ext/opcache/jit/zend_jit.c +++ b/ext/opcache/jit/zend_jit.c @@ -1425,9 +1425,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op zend_jit_set_last_valid_opline(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start); } if (ssa->cfg.blocks[b].flags & ZEND_BB_LOOP_HEADER) { - if (!zend_jit_check_timeout(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start, NULL)) { - goto jit_failure; - } + zend_jit_check_timeout(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start, NULL); } if (!ssa->cfg.blocks[b].len) { zend_jit_bb_end(&ctx, b); diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index e0da8b6289611..3af893e478cda 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -1857,7 +1857,7 @@ static void jit_OBJ_RELEASE(zend_jit_ctx *jit, ir_ref ref) ir_MERGE_list(end_inputs); } -static int zend_jit_check_timeout(zend_jit_ctx *jit, const zend_op *opline, const void *exit_addr) +static void zend_jit_check_timeout(zend_jit_ctx *jit, const zend_op *opline, const void *exit_addr) { ir_ref ref = ir_LOAD_U8(jit_EG(vm_interrupt)); @@ -1873,7 +1873,6 @@ static int zend_jit_check_timeout(zend_jit_ctx *jit, const zend_op *opline, cons ir_IJMP(jit_STUB_ADDR(jit, jit_stub_interrupt_handler)); ir_IF_FALSE(if_timeout); } - return 1; } /* stubs */ @@ -2451,9 +2450,7 @@ static int zend_jit_trace_exit_stub(zend_jit_ctx *jit) } // check for interrupt (try to avoid this ???) - if (!zend_jit_check_timeout(jit, NULL, NULL)) { - return 0; - } + zend_jit_check_timeout(jit, NULL, NULL); addr = zend_jit_orig_opline_handler(jit); if (GCC_GLOBAL_REGS) { @@ -10306,9 +10303,7 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen exit_addr = NULL; } - if (!zend_jit_check_timeout(jit, opline + 1, exit_addr)) { - return 0; - } + zend_jit_check_timeout(jit, opline + 1, exit_addr); if ((!trace || !func) && opline->opcode != ZEND_DO_ICALL) { jit_LOAD_IP_ADDR(jit, opline + 1); From 0b06725ad36735a1a79ac3551011e188ff033954 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 26 Jul 2024 15:24:42 -0600 Subject: [PATCH 05/10] Fix JIT support Co-authored-by: Bob Weinand --- Zend/zend_execute.c | 2 +- Zend/zend_execute.h | 2 +- ext/opcache/jit/zend_jit_ir.c | 21 +++++++-------------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 09678469fc00e..64452f485e3eb 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4056,7 +4056,7 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec } /* }}} */ -ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call) +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call) { if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index 143d0adb55986..b8a95d922c732 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -542,7 +542,7 @@ ZEND_COLD void zend_match_unhandled_error(const zval *value); /* Call this to handle the timeout or the interrupt function. It will not clear * the EG(vm_interrupt). */ -ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call); +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call); static zend_always_inline void zend_interrupt_or_timeout_check(zend_execute_data *call) { diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 3af893e478cda..f7b7b1a21b974 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3075,6 +3075,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_pre_dec_obj_helper); REGISTER_HELPER(zend_jit_post_dec_obj_helper); REGISTER_HELPER(zend_jit_rope_end); + REGISTER_HELPER(zend_interrupt_or_timeout); #ifndef ZTS REGISTER_DATA(EG(current_execute_data)); @@ -10191,6 +10192,12 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen jit_observer_fcall_end(jit, rx, res_ref); } + // JIT: if (EG(vm_interrupt)) zend_interrupt_or_timeout(execute_data); + ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); + ir_IF_TRUE_cold(if_interrupt); + ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_interrupt_or_timeout), rx); + ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); + // JIT: EG(current_execute_data) = execute_data; ir_STORE(jit_EG(current_execute_data), jit_FP(jit)); @@ -10291,20 +10298,6 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen ir_GUARD_NOT(ir_LOAD_A(jit_EG_exception(jit)), jit_STUB_ADDR(jit, jit_stub_icall_throw)); - // TODO: Can we avoid checking for interrupts after each call ??? - if (trace && jit->last_valid_opline != opline) { - int32_t exit_point = zend_jit_trace_get_exit_point(opline + 1, ZEND_JIT_EXIT_TO_VM); - - exit_addr = zend_jit_trace_get_exit_addr(exit_point); - if (!exit_addr) { - return 0; - } - } else { - exit_addr = NULL; - } - - zend_jit_check_timeout(jit, opline + 1, exit_addr); - if ((!trace || !func) && opline->opcode != ZEND_DO_ICALL) { jit_LOAD_IP_ADDR(jit, opline + 1); } else if (trace From cf2bcd9ed3184c9d4d8899fd5cd5da929b12b83a Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Thu, 1 Aug 2024 16:31:33 -0600 Subject: [PATCH 06/10] Fix the missing store to vm_interrupt --- Zend/zend_execute.c | 2 +- Zend/zend_execute.h | 2 +- ext/opcache/jit/zend_jit_helpers.c | 6 ++++++ ext/opcache/jit/zend_jit_ir.c | 6 +++--- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 64452f485e3eb..16f6479c2efa2 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4056,7 +4056,7 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec } /* }}} */ -ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call) +ZEND_API ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call) { if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index b8a95d922c732..dbf4fd66a6d05 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -542,7 +542,7 @@ ZEND_COLD void zend_match_unhandled_error(const zval *value); /* Call this to handle the timeout or the interrupt function. It will not clear * the EG(vm_interrupt). */ -ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call); +ZEND_API ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call); static zend_always_inline void zend_interrupt_or_timeout_check(zend_execute_data *call) { diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c index 8ed378d7552b0..f4919c1f9f5f6 100644 --- a/ext/opcache/jit/zend_jit_helpers.c +++ b/ext/opcache/jit/zend_jit_helpers.c @@ -3272,3 +3272,9 @@ static zend_string* ZEND_FASTCALL zend_jit_rope_end(zend_string **rope, uint32_t *target = '\0'; return ret; } + +ZEND_COLD static void ZEND_FASTCALL zend_jit_interrupt_or_timeout(zend_execute_data *call) +{ + zend_atomic_bool_store_ex(&EG(vm_interrupt), false); + zend_interrupt_or_timeout(call); +} diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index f7b7b1a21b974..30d2b2fe9fc5b 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3075,7 +3075,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_pre_dec_obj_helper); REGISTER_HELPER(zend_jit_post_dec_obj_helper); REGISTER_HELPER(zend_jit_rope_end); - REGISTER_HELPER(zend_interrupt_or_timeout); + REGISTER_HELPER(zend_jit_interrupt_or_timeout); #ifndef ZTS REGISTER_DATA(EG(current_execute_data)); @@ -10192,10 +10192,10 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen jit_observer_fcall_end(jit, rx, res_ref); } - // JIT: if (EG(vm_interrupt)) zend_interrupt_or_timeout(execute_data); + // JIT: if (EG(vm_interrupt)) zend_jit_interrupt_or_timeout(execute_data); ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); ir_IF_TRUE_cold(if_interrupt); - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_interrupt_or_timeout), rx); + ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_interrupt_or_timeout), rx); ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); // JIT: EG(current_execute_data) = execute_data; From ef2652814a42caac7439a5177cd03eaeb71256e2 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Thu, 1 Aug 2024 17:24:31 -0600 Subject: [PATCH 07/10] Rename new functions --- Zend/zend_execute.c | 8 +++++++- Zend/zend_execute.h | 11 ++--------- Zend/zend_vm_def.h | 6 +++--- Zend/zend_vm_execute.h | 18 +++++++++--------- ext/opcache/jit/zend_jit_helpers.c | 4 ++-- ext/opcache/jit/zend_jit_ir.c | 6 +++--- 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 16f6479c2efa2..b0872752196c0 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4056,7 +4056,7 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec } /* }}} */ -ZEND_API ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call) +ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call) { if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); @@ -4077,6 +4077,12 @@ ZEND_API ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call) } \ } while (0) +#define ZEND_VM_FCALL_INTERRUPT_CHECK(call) do { \ + if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) { \ + zend_fcall_interrupt(call); \ + } \ + } while (0) + /* * Stack Frame Layout (the whole stack frame is allocated at once) * ================== diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index dbf4fd66a6d05..44f1fdd250acd 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -540,16 +540,9 @@ ZEND_COLD void zend_magic_get_property_type_inconsistency_error(const zend_prope ZEND_COLD void zend_match_unhandled_error(const zval *value); /* Call this to handle the timeout or the interrupt function. It will not clear - * the EG(vm_interrupt). + * the EG(vm_interrupt); this is done by the fcall/jit handler. */ -ZEND_API ZEND_COLD void zend_interrupt_or_timeout(zend_execute_data *call); - -static zend_always_inline void zend_interrupt_or_timeout_check(zend_execute_data *call) -{ - if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) { - zend_interrupt_or_timeout(call); - } -} +ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call); static zend_always_inline void *zend_get_bad_ptr(void) { diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 826dd41d9aa32..051d633fb933b 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -4074,7 +4074,7 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -4197,7 +4197,7 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -4319,7 +4319,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) } #endif ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index a7a6ca63d1a48..1a6b39a694f1d 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1299,7 +1299,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1364,7 +1364,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1430,7 +1430,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_OBS } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; zend_vm_stack_free_args(call); @@ -1599,7 +1599,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -1702,7 +1702,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -1807,7 +1807,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_ } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -1928,7 +1928,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -2048,7 +2048,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV } #endif - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; @@ -2169,7 +2169,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_OBS } #endif zend_observer_fcall_end(call, EG(exception) ? NULL : ret); - zend_interrupt_or_timeout_check(call); + ZEND_VM_FCALL_INTERRUPT_CHECK(call); EG(current_execute_data) = execute_data; diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c index f4919c1f9f5f6..c8223f458b2ea 100644 --- a/ext/opcache/jit/zend_jit_helpers.c +++ b/ext/opcache/jit/zend_jit_helpers.c @@ -3273,8 +3273,8 @@ static zend_string* ZEND_FASTCALL zend_jit_rope_end(zend_string **rope, uint32_t return ret; } -ZEND_COLD static void ZEND_FASTCALL zend_jit_interrupt_or_timeout(zend_execute_data *call) +ZEND_COLD static void ZEND_FASTCALL zend_jit_fcall_interrupt(zend_execute_data *call) { zend_atomic_bool_store_ex(&EG(vm_interrupt), false); - zend_interrupt_or_timeout(call); + zend_fcall_interrupt(call); } diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 30d2b2fe9fc5b..ebe0404d56128 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3075,7 +3075,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_pre_dec_obj_helper); REGISTER_HELPER(zend_jit_post_dec_obj_helper); REGISTER_HELPER(zend_jit_rope_end); - REGISTER_HELPER(zend_jit_interrupt_or_timeout); + REGISTER_HELPER(zend_jit_fcall_interrupt); #ifndef ZTS REGISTER_DATA(EG(current_execute_data)); @@ -10192,10 +10192,10 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen jit_observer_fcall_end(jit, rx, res_ref); } - // JIT: if (EG(vm_interrupt)) zend_jit_interrupt_or_timeout(execute_data); + // JIT: if (EG(vm_interrupt)) zend_jit_fcall_interrupt(execute_data); ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); ir_IF_TRUE_cold(if_interrupt); - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_interrupt_or_timeout), rx); + ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_fcall_interrupt), rx); ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); // JIT: EG(current_execute_data) = execute_data; From 914c2835ccbe8d9ccd52c81782e5b25643ed7e4a Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Thu, 1 Aug 2024 17:53:27 -0600 Subject: [PATCH 08/10] Special case zend_interrupt_function in JIT code --- Zend/zend_system_id.c | 12 ++++++++---- ext/opcache/jit/zend_jit_ir.c | 36 ++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/Zend/zend_system_id.c b/Zend/zend_system_id.c index 37bed895a7b96..2c3ebab0f4807 100644 --- a/Zend/zend_system_id.c +++ b/Zend/zend_system_id.c @@ -55,10 +55,11 @@ void zend_startup_system_id(void) zend_system_id[0] = '\0'; } -#define ZEND_HOOK_AST_PROCESS (1 << 0) -#define ZEND_HOOK_COMPILE_FILE (1 << 1) -#define ZEND_HOOK_EXECUTE_EX (1 << 2) -#define ZEND_HOOK_EXECUTE_INTERNAL (1 << 3) +#define ZEND_HOOK_AST_PROCESS (1 << 0) +#define ZEND_HOOK_COMPILE_FILE (1 << 1) +#define ZEND_HOOK_EXECUTE_EX (1 << 2) +#define ZEND_HOOK_EXECUTE_INTERNAL (1 << 3) +#define ZEND_HOOK_INTERRUPT_FUNCTION (1 << 4) void zend_finalize_system_id(void) { @@ -77,6 +78,9 @@ void zend_finalize_system_id(void) if (zend_execute_internal) { hooks |= ZEND_HOOK_EXECUTE_INTERNAL; } + if (zend_interrupt_function) { + hooks |= ZEND_HOOK_INTERRUPT_FUNCTION; + } PHP_MD5Update(&context, &hooks, sizeof hooks); for (int16_t i = 0; i < 256; i++) { diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index ebe0404d56128..8399ec62c0db3 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -10192,11 +10192,18 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen jit_observer_fcall_end(jit, rx, res_ref); } - // JIT: if (EG(vm_interrupt)) zend_jit_fcall_interrupt(execute_data); - ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); - ir_IF_TRUE_cold(if_interrupt); - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_fcall_interrupt), rx); - ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); + /* When zend_interrupt_function is set, it gets called while + * the frame is still on top. This is less efficient than + * doing it later once it's popped off. There is code further + * down that handles when there isn't an interrupt function. + */ + if (zend_interrupt_function) { + // JIT: if (EG(vm_interrupt)) zend_jit_fcall_interrupt(execute_data); + ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); + ir_IF_TRUE_cold(if_interrupt); + ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_fcall_interrupt), rx); + ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); + } // JIT: EG(current_execute_data) = execute_data; ir_STORE(jit_EG(current_execute_data), jit_FP(jit)); @@ -10298,6 +10305,25 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen ir_GUARD_NOT(ir_LOAD_A(jit_EG_exception(jit)), jit_STUB_ADDR(jit, jit_stub_icall_throw)); + /* If there isn't a zend_interrupt_function, the timeout is + * handled here because it's more efficient. + */ + if (!zend_interrupt_function) { + // TODO: Can we avoid checking for interrupts after each call ??? + if (trace && jit->last_valid_opline != opline) { + int32_t exit_point = zend_jit_trace_get_exit_point(opline + 1, ZEND_JIT_EXIT_TO_VM); + + exit_addr = zend_jit_trace_get_exit_addr(exit_point); + if (!exit_addr) { + return 0; + } + } else { + exit_addr = NULL; + } + + zend_jit_check_timeout(jit, opline + 1, exit_addr); + } + if ((!trace || !func) && opline->opcode != ZEND_DO_ICALL) { jit_LOAD_IP_ADDR(jit, opline + 1); } else if (trace From 1255c719cae4f85b44be39e4fb74431533421ffb Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 14 Aug 2024 11:18:33 -0600 Subject: [PATCH 09/10] refactor to use ZEND_VM_SET_OPCODE_NO_INTERRUPT --- Zend/zend_execute.c | 7 +++++-- Zend/zend_vm_def.h | 10 +++------- Zend/zend_vm_execute.h | 30 +++++++++--------------------- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ae9b4e21a7ff7..c171da145c3ef 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -5508,9 +5508,12 @@ static zend_always_inline zend_execute_data *_zend_vm_stack_push_call_frame(uint CHECK_SYMBOL_TABLES() \ OPLINE = new_op -#define ZEND_VM_SET_OPCODE(new_op) \ +#define ZEND_VM_SET_OPCODE_NO_INTERRUPT(new_op) \ CHECK_SYMBOL_TABLES() \ - OPLINE = new_op; \ + OPLINE = new_op + +#define ZEND_VM_SET_OPCODE(new_op) \ + ZEND_VM_SET_OPCODE_NO_INTERRUPT(new_op); \ ZEND_VM_INTERRUPT_CHECK() #define ZEND_VM_SET_RELATIVE_OPCODE(opline, offset) \ diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 34c166f61b963..92cec7c059c7f 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -4098,8 +4098,7 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER)) HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -4228,8 +4227,7 @@ ZEND_VM_C_LABEL(fcall_by_name_end): zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -4348,9 +4346,7 @@ ZEND_VM_C_LABEL(fcall_end): zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index dd9fb6214833b..a7cd17da96b85 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1323,8 +1323,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1388,8 +1387,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1454,8 +1452,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_OBS HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1630,8 +1627,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1733,8 +1729,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1838,8 +1833,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_ zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -1957,9 +1951,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -2077,9 +2069,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } @@ -2198,9 +2188,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_OBS zend_rethrow_exception(execute_data); HANDLE_EXCEPTION(); } - - CHECK_SYMBOL_TABLES() - OPLINE = opline + 1; + ZEND_VM_SET_OPCODE_NO_INTERRUPT(opline + 1); ZEND_VM_CONTINUE(); } From 8fa433a16aac9782b71ae420b310be5fa3e2e7a3 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 14 Aug 2024 11:30:27 -0600 Subject: [PATCH 10/10] Split atomic exchange into load + store It is difficult to determine performance of atomics sometimes. In this case, the separate load+store is still correct, and a load does not cause a modification, and might be faster for some platforms than an exchange. A load+store is slower than an exchange, but we're fine trading the penalty to the slow path and keeping the happy path faster. --- Zend/zend_execute.c | 5 +++-- Zend/zend_execute.h | 6 +++--- ext/opcache/jit/zend_jit_helpers.c | 6 ------ ext/opcache/jit/zend_jit_ir.c | 6 +++--- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index c171da145c3ef..785816c2d30de 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4054,8 +4054,9 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec } /* }}} */ -ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call) +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_fcall_interrupt(zend_execute_data *call) { + zend_atomic_bool_store_ex(&EG(vm_interrupt), false); if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { @@ -4076,7 +4077,7 @@ ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call) } while (0) #define ZEND_VM_FCALL_INTERRUPT_CHECK(call) do { \ - if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) { \ + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \ zend_fcall_interrupt(call); \ } \ } while (0) diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index 44f1fdd250acd..c6b52b59e0a6c 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -539,10 +539,10 @@ ZEND_COLD void zend_magic_get_property_type_inconsistency_error(const zend_prope ZEND_COLD void zend_match_unhandled_error(const zval *value); -/* Call this to handle the timeout or the interrupt function. It will not clear - * the EG(vm_interrupt); this is done by the fcall/jit handler. +/* Call this to handle the timeout or the interrupt function. It will set + * EG(vm_interrupt) to false. */ -ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call); +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_fcall_interrupt(zend_execute_data *call); static zend_always_inline void *zend_get_bad_ptr(void) { diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c index c8223f458b2ea..8ed378d7552b0 100644 --- a/ext/opcache/jit/zend_jit_helpers.c +++ b/ext/opcache/jit/zend_jit_helpers.c @@ -3272,9 +3272,3 @@ static zend_string* ZEND_FASTCALL zend_jit_rope_end(zend_string **rope, uint32_t *target = '\0'; return ret; } - -ZEND_COLD static void ZEND_FASTCALL zend_jit_fcall_interrupt(zend_execute_data *call) -{ - zend_atomic_bool_store_ex(&EG(vm_interrupt), false); - zend_fcall_interrupt(call); -} diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 380a41f1657b5..37c512ff198fa 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3075,7 +3075,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_pre_dec_obj_helper); REGISTER_HELPER(zend_jit_post_dec_obj_helper); REGISTER_HELPER(zend_jit_rope_end); - REGISTER_HELPER(zend_jit_fcall_interrupt); + REGISTER_HELPER(zend_fcall_interrupt); #ifndef ZTS REGISTER_DATA(EG(current_execute_data)); @@ -10203,10 +10203,10 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen * down that handles when there isn't an interrupt function. */ if (zend_interrupt_function) { - // JIT: if (EG(vm_interrupt)) zend_jit_fcall_interrupt(execute_data); + // JIT: if (EG(vm_interrupt)) zend_fcall_interrupt(execute_data); ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt))); ir_IF_TRUE_cold(if_interrupt); - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_fcall_interrupt), rx); + ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_fcall_interrupt), rx); ir_MERGE_WITH_EMPTY_FALSE(if_interrupt); }