From fb95f5c4d218f09a41687859e7bb4789bac7ca03 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Dec 2024 00:26:14 +0100 Subject: [PATCH 1/3] Fix GH-17257: SEGV ext/opcache/jit/zend_jit_vm_helpers.c EX(opline) / opline can be stale if the IP is not stored, like in this case on a trace enter. We always need to make sure that the opline is up to date to make sure we don't use stale data. --- ext/opcache/jit/zend_jit_ir.c | 19 ++++++++----------- ext/opcache/tests/jit/gh17257.phpt | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 ext/opcache/tests/jit/gh17257.phpt diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 6b74621517f2d..15dfdb8421ca3 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -10110,20 +10110,17 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen } } } else { - if (!trace || (trace->op == ZEND_JIT_TRACE_END - && trace->stop == ZEND_JIT_TRACE_STOP_INTERPRETER)) { - ir_ref ip; + ir_ref ip; - if (zend_accel_in_shm(func->op_array.opcodes)) { - ip = ir_CONST_ADDR(func->op_array.opcodes); - } else { - if (!func_ref) { - func_ref = ir_LOAD_A(jit_CALL(rx, func)); - } - ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes))); + if (zend_accel_in_shm(func->op_array.opcodes)) { + ip = ir_CONST_ADDR(func->op_array.opcodes); + } else { + if (!func_ref) { + func_ref = ir_LOAD_A(jit_CALL(rx, func)); } - jit_LOAD_IP(jit, ip); + ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes))); } + jit_LOAD_IP(jit, ip); if (GCC_GLOBAL_REGS) { ir_CALL(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper)); } else { diff --git a/ext/opcache/tests/jit/gh17257.phpt b/ext/opcache/tests/jit/gh17257.phpt new file mode 100644 index 0000000000000..177e00decf94c --- /dev/null +++ b/ext/opcache/tests/jit/gh17257.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-17257 (SEGV ext/opcache/jit/zend_jit_vm_helpers.c) +--EXTENSIONS-- +opcache +--INI-- +opcache.jit_buffer_size=32M +opcache.jit=1254 +opcache.jit_hot_func=1 +--FILE-- + +--EXPECT-- +Done From 319df3d0442b47fe429bcb4316f62a19b371f6ec Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Dec 2024 21:26:56 +0100 Subject: [PATCH 2/3] review --- ext/opcache/jit/zend_jit_internal.h | 1 + ext/opcache/jit/zend_jit_ir.c | 28 +++++++++++++++++---------- ext/opcache/jit/zend_jit_vm_helpers.c | 14 ++++++++++++-- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 4fe07222d8f72..ce245bdb1988e 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -231,6 +231,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_func_counter_helper(ZEND_OPCODE_H ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_counter_helper(ZEND_OPCODE_HANDLER_ARGS); void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D); +void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D); bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D); void ZEND_FASTCALL zend_jit_undefined_long_key(EXECUTE_DATA_D); void ZEND_FASTCALL zend_jit_undefined_long_key_ex(zend_long key EXECUTE_DATA_DC); diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 15dfdb8421ca3..e14345215f6f4 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3050,6 +3050,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_undefined_long_key_ex); REGISTER_HELPER(zend_jit_undefined_string_key); REGISTER_HELPER(zend_jit_copy_extra_args_helper); + REGISTER_HELPER(zend_jit_copy_extra_args_helper_no_skip_recv); REGISTER_HELPER(zend_jit_vm_stack_free_args_helper); REGISTER_HELPER(zend_free_extra_named_params); REGISTER_HELPER(zend_jit_free_call_frame); @@ -10110,21 +10111,28 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen } } } else { - ir_ref ip; + ir_ref helper; + if (!trace || (trace->op == ZEND_JIT_TRACE_END + && trace->stop == ZEND_JIT_TRACE_STOP_INTERPRETER)) { + ir_ref ip; - if (zend_accel_in_shm(func->op_array.opcodes)) { - ip = ir_CONST_ADDR(func->op_array.opcodes); - } else { - if (!func_ref) { - func_ref = ir_LOAD_A(jit_CALL(rx, func)); + if (zend_accel_in_shm(func->op_array.opcodes)) { + ip = ir_CONST_ADDR(func->op_array.opcodes); + } else { + if (!func_ref) { + func_ref = ir_LOAD_A(jit_CALL(rx, func)); + } + ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes))); } - ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes))); + jit_LOAD_IP(jit, ip); + helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper); + } else { + helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper_no_skip_recv); } - jit_LOAD_IP(jit, ip); if (GCC_GLOBAL_REGS) { - ir_CALL(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper)); + ir_CALL(IR_VOID, helper); } else { - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper), jit_FP(jit)); + ir_CALL_1(IR_VOID, helper, jit_FP(jit)); } } } else { diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 08370fc5323dc..0bd0fe3fc10f9 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -120,7 +120,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_leave_func_helper(EXECUTE_DATA_D) } } -void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) +static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_D, bool skip_recv) { zend_op_array *op_array = &EX(func)->op_array; @@ -130,7 +130,7 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) zval *end, *src, *dst; uint32_t type_flags = 0; - if (EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) { + if (skip_recv && EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) { /* Skip useless ZEND_RECV and ZEND_RECV_INIT opcodes */ #ifdef HAVE_GCC_GLOBAL_REGS opline += first_extra_arg; @@ -166,6 +166,16 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) } } +void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) +{ + zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_C, true); +} + +void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D) +{ + zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_C, false); +} + bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D) { zend_execute_data *call = (zend_execute_data *) opline; From 2b931c0a79d31dc266f45375a6ea7718e53ee8fe Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Dec 2024 21:39:04 +0100 Subject: [PATCH 3/3] fixed compile --- ext/opcache/jit/zend_jit_vm_helpers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 0bd0fe3fc10f9..f4d261cd75349 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -120,7 +120,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_leave_func_helper(EXECUTE_DATA_D) } } -static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_D, bool skip_recv) +static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(bool skip_recv EXECUTE_DATA_DC) { zend_op_array *op_array = &EX(func)->op_array; @@ -168,12 +168,12 @@ static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_D, boo void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) { - zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_C, true); + zend_jit_copy_extra_args_helper_ex(true EXECUTE_DATA_CC); } void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D) { - zend_jit_copy_extra_args_helper_ex(EXECUTE_DATA_C, false); + zend_jit_copy_extra_args_helper_ex(false EXECUTE_DATA_CC); } bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D)