From 978c01ce158897498f07cff6a0465305647c8f8f Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Thu, 10 Apr 2025 16:22:08 +0200 Subject: [PATCH] JIT: Check exception on exit Add a new exit flag (ZEND_JIT_EXIT_CHECK_EXCEPTION) that enables exception checking during exit/deoptimization. We already checked for exceptions during exit/deoptimization, but only when ZEND_JIT_EXIT_FREE_OP1 or ZEND_JIT_EXIT_FREE_OP2 were set (presumably to handle exceptions thrown during dtor). The new flag makes it possible to request it explicitly. This also fixes two issues in zend_jit_trace_exit(): - By returning 1, we were telling the caller (zend_jit_trace_exit_stub()) to execute the original op handler of EG(current_execute_data)->opline, but in reality we want to execute EX(opline), which should be EG(exception_op). - EX(opline) is set to the value of %r15 in zend_jit_trace_exit_stub() before calling zend_jit_trace_exit(), but this may be the address of a zend_execute_data when the register is being reused to cache EX(call). Fixes GH-18262 Closes GH-18297 --- NEWS | 2 ++ ext/opcache/jit/zend_jit_internal.h | 23 ++++++++-------- ext/opcache/jit/zend_jit_ir.c | 4 +-- ext/opcache/jit/zend_jit_trace.c | 16 ++++++++--- ext/opcache/tests/jit/gh18262-001.phpt | 37 ++++++++++++++++++++++++++ ext/opcache/tests/jit/gh18262-002.phpt | 34 +++++++++++++++++++++++ ext/opcache/tests/jit/gh18262-003.phpt | 35 ++++++++++++++++++++++++ ext/opcache/tests/jit/gh18262-004.phpt | 36 +++++++++++++++++++++++++ 8 files changed, 170 insertions(+), 17 deletions(-) create mode 100644 ext/opcache/tests/jit/gh18262-001.phpt create mode 100644 ext/opcache/tests/jit/gh18262-002.phpt create mode 100644 ext/opcache/tests/jit/gh18262-003.phpt create mode 100644 ext/opcache/tests/jit/gh18262-004.phpt diff --git a/NEWS b/NEWS index af78129214f..2331df67b60 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS - Opcache: . Fixed bug GH-18417 (Windows SHM reattachment fails when increasing memory_consumption or jit_buffer_size). (nielsdos) + . Fixed bug GH-18297 (Exception not handled when jit guard is triggered). + (Arnaud) - SPL: . Fixed bug GH-18421 (Integer overflow with large numbers in LimitIterator). diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index ce245bdb198..1bf62e1f914 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -311,17 +311,18 @@ typedef enum _zend_jit_trace_stop { #define ZEND_JIT_TRACE_SUPPORTED 0 -#define ZEND_JIT_EXIT_JITED (1<<0) -#define ZEND_JIT_EXIT_BLACKLISTED (1<<1) -#define ZEND_JIT_EXIT_TO_VM (1<<2) /* exit to VM without attempt to create a side trace */ -#define ZEND_JIT_EXIT_RESTORE_CALL (1<<3) /* deoptimizer should restore EX(call) chain */ -#define ZEND_JIT_EXIT_POLYMORPHISM (1<<4) /* exit because of polymorphic call */ -#define ZEND_JIT_EXIT_FREE_OP1 (1<<5) -#define ZEND_JIT_EXIT_FREE_OP2 (1<<6) -#define ZEND_JIT_EXIT_PACKED_GUARD (1<<7) -#define ZEND_JIT_EXIT_CLOSURE_CALL (1<<8) /* exit because of polymorphic INIT_DYNAMIC_CALL call */ -#define ZEND_JIT_EXIT_METHOD_CALL (1<<9) /* exit because of polymorphic INIT_METHOD_CALL call */ -#define ZEND_JIT_EXIT_INVALIDATE (1<<10) /* invalidate current trace */ +#define ZEND_JIT_EXIT_JITED (1<<0) +#define ZEND_JIT_EXIT_BLACKLISTED (1<<1) +#define ZEND_JIT_EXIT_TO_VM (1<<2) /* exit to VM without attempt to create a side trace */ +#define ZEND_JIT_EXIT_RESTORE_CALL (1<<3) /* deoptimizer should restore EX(call) chain */ +#define ZEND_JIT_EXIT_POLYMORPHISM (1<<4) /* exit because of polymorphic call */ +#define ZEND_JIT_EXIT_FREE_OP1 (1<<5) +#define ZEND_JIT_EXIT_FREE_OP2 (1<<6) +#define ZEND_JIT_EXIT_PACKED_GUARD (1<<7) +#define ZEND_JIT_EXIT_CLOSURE_CALL (1<<8) /* exit because of polymorphic INIT_DYNAMIC_CALL call */ +#define ZEND_JIT_EXIT_METHOD_CALL (1<<9) /* exit because of polymorphic INIT_METHOD_CALL call */ +#define ZEND_JIT_EXIT_INVALIDATE (1<<10) /* invalidate current trace */ +#define ZEND_JIT_EXIT_CHECK_EXCEPTION (1<<11) #define ZEND_JIT_EXIT_FIXED (1U<<31) /* the exit_info can't be changed by zend_jit_snapshot_handler() */ diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index e9b1a9f01e1..064e88c39c1 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -14483,12 +14483,12 @@ result_fetched: ZEND_ASSERT(end_inputs == IR_UNUSED); if ((res_info & MAY_BE_GUARD) && JIT_G(current_frame)) { uint8_t type = concrete_type(res_info); - uint32_t flags = 0; + uint32_t flags = ZEND_JIT_EXIT_CHECK_EXCEPTION; if ((opline->op1_type & (IS_VAR|IS_TMP_VAR)) && !delayed_fetch_this && !op1_avoid_refcounting) { - flags = ZEND_JIT_EXIT_FREE_OP1; + flags |= ZEND_JIT_EXIT_FREE_OP1; } if ((opline->result_type & (IS_VAR|IS_TMP_VAR)) diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 4ee0a8a413c..c0ee09e6e63 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -3474,7 +3474,7 @@ static int zend_jit_trace_exit_needs_deoptimization(uint32_t trace_num, uint32_t uint32_t stack_size; zend_jit_trace_stack *stack; - if (opline || (flags & (ZEND_JIT_EXIT_RESTORE_CALL|ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2))) { + if (opline || (flags & (ZEND_JIT_EXIT_RESTORE_CALL|ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2|ZEND_JIT_EXIT_CHECK_EXCEPTION))) { return 1; } @@ -3634,7 +3634,7 @@ static int zend_jit_trace_deoptimization( } } - if (flags & (ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2)) { + if (flags & (ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2|ZEND_JIT_EXIT_CHECK_EXCEPTION)) { zend_jit_check_exception(jit); } @@ -7943,6 +7943,9 @@ static void zend_jit_dump_exit_info(zend_jit_trace_info *t) if (t->exit_info[i].flags & ZEND_JIT_EXIT_FREE_OP2) { fprintf(stderr, "/FREE_OP2"); } + if (t->exit_info[i].flags & ZEND_JIT_EXIT_CHECK_EXCEPTION) { + fprintf(stderr, "/CHK_EXC"); + } for (j = 0; j < stack_size; j++) { uint8_t type = STACK_TYPE(stack, j); if (type != IS_UNKNOWN) { @@ -8654,9 +8657,14 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf EX(opline) = opline-1; zval_ptr_dtor_nogc(EX_VAR((opline-1)->op1.var)); } - if (t->exit_info[exit_num].flags & (ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2)) { + if (t->exit_info[exit_num].flags & (ZEND_JIT_EXIT_FREE_OP1|ZEND_JIT_EXIT_FREE_OP2|ZEND_JIT_EXIT_CHECK_EXCEPTION)) { if (EG(exception)) { - return 1; + /* EX(opline) was overridden in zend_jit_trace_exit_stub(), + * and may be wrong when IP is reused. */ + if (GCC_GLOBAL_REGS) { + EX(opline) = EG(exception_op); + } + return 0; } } if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_METHOD_CALL) { diff --git a/ext/opcache/tests/jit/gh18262-001.phpt b/ext/opcache/tests/jit/gh18262-001.phpt new file mode 100644 index 00000000000..345b56a5e51 --- /dev/null +++ b/ext/opcache/tests/jit/gh18262-001.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-18262 001 (Assertion failure Zend/zend_vm_execute.h JIT) +--CREDITS-- +YuanchengJiang +--FILE-- +newLazyProxy(function ($obj) use ($instance) { + $instance->b = 1; + return $instance; + }); + var_dump($obj->b); +} +?> +--EXPECTF-- +int(1) +int(1) + +Fatal error: Uncaught TypeError: %s in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d diff --git a/ext/opcache/tests/jit/gh18262-002.phpt b/ext/opcache/tests/jit/gh18262-002.phpt new file mode 100644 index 00000000000..b82723abaa2 --- /dev/null +++ b/ext/opcache/tests/jit/gh18262-002.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-18262 002 (Assertion failure Zend/zend_vm_execute.h JIT) +--FILE-- +b = $init; + } + } +} + +$tests = [ + new B(1), + new B(0), +]; + +set_error_handler(function ($_, $errstr) { + throw new \Exception($errstr); +}); + +foreach ($tests as $obj) { + var_dump($obj->b); +} +?> +--EXPECTF-- +int(1) + +Fatal error: Uncaught Exception: Undefined property: B::$b in %s:%d +Stack trace: +#0 %s(%d): {closure:%s:%d}(2, 'Undefined prope...', '%s', %d) +#1 {main} + thrown in %s on line %d diff --git a/ext/opcache/tests/jit/gh18262-003.phpt b/ext/opcache/tests/jit/gh18262-003.phpt new file mode 100644 index 00000000000..a3c3e037632 --- /dev/null +++ b/ext/opcache/tests/jit/gh18262-003.phpt @@ -0,0 +1,35 @@ +--TEST-- +GH-18262 003 (Assertion failure Zend/zend_vm_execute.h JIT) +--FILE-- +b = $init; + } + } +} + +$tests = [ + new B(1), + new B('str'), // slow deoptimization, create linked side trace + new B(0), // jump to side trace with fast deoptimization +]; + +set_error_handler(function ($_, $errstr) { + throw new \Exception($errstr); +}); + +foreach ($tests as $obj) { + try { + var_dump($obj->b); + } catch (Exception $e) { + printf("%s: %s\n", $e::class, $e->getMessage()); + } +} +?> +--EXPECT-- +int(1) +string(3) "str" +Exception: Undefined property: B::$b diff --git a/ext/opcache/tests/jit/gh18262-004.phpt b/ext/opcache/tests/jit/gh18262-004.phpt new file mode 100644 index 00000000000..c8dccd30c98 --- /dev/null +++ b/ext/opcache/tests/jit/gh18262-004.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-18262 004 (Assertion failure Zend/zend_vm_execute.h JIT) +--FILE-- +throw === '1' ? 'str' : 1; + } + public function __destruct() { + if ($this->throw === '1') { + throw new Exception(__METHOD__); + } + } +} + +$tests = [ + '0', + '1', +]; + +foreach ($tests as $test) { + // Second iteration exits, and free op1 throws + var_dump((new B($test))->b); +} +?> +--EXPECTF-- +int(1) + +Fatal error: Uncaught Exception: B::__destruct in %s:%d +Stack trace: +#0 %s(%d): B->__destruct() +#1 {main} + thrown in %s on line %d