From 51a74addb7ef5b4f9459e49fc04f3d678c96a932 Mon Sep 17 00:00:00 2001 From: coderzhao Date: Thu, 30 Jul 2026 10:39:15 +0800 Subject: [PATCH] Fix GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook In the function JIT, ZEND_FETCH_OBJ_FUNC_ARG's by-value fetch dispatches into the FETCH_OBJ_R handler, which may take the SIMPLE_GET hook fast path and push a getter frame. Because the function JIT may keep values solely in registers, exiting to the VM there leaves stale stack slots. Inline the by-value path through zend_jit_fetch_obj (which runs the hook getter inside a helper and keeps all registers live), and keep the by-ref path on the generic handler (a full C call, safe under register allocation). The runtime by-ref check is required because the passing mode is only known once the callee is resolved via namespace fallback. The IR block is extracted into zend_jit_fetch_obj_func_arg() in zend_jit_ir.c so that IR-related code stays in that file, and the ZEND_FETCH_OBJ_FUNC_ARG case is merged with the ZEND_FETCH_OBJ_R/IS/W case to deduplicate the setup. This mirrors the tracing JIT fix for GH-21006 (GH-21369). --- NEWS | 4 + ext/opcache/jit/zend_jit.c | 25 ++++- ext/opcache/jit/zend_jit_ir.c | 53 +++++++++++ ext/opcache/tests/jit/gh22857.phpt | 125 +++++++++++++++++++++++++ ext/opcache/tests/jit/gh22857_002.phpt | 47 ++++++++++ 5 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 ext/opcache/tests/jit/gh22857.phpt create mode 100644 ext/opcache/tests/jit/gh22857_002.phpt diff --git a/NEWS b/NEWS index 54dea5289ff9..7f4d24f6b0a6 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,10 @@ PHP NEWS . Fixed bug GH-15836 (Use-after-free when a user stream filter accesses $this->stream during the close flush). (iliaal) +- Opcache: + . Fixed bug GH-22857 (Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a + property hook getter, losing register-held variables). (Zhao Hao) + 30 Jul 2026, PHP 8.4.24 - BCMath: diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index da73c98c4355..f4fce5e0dd99 100644 --- a/ext/opcache/jit/zend_jit.c +++ b/ext/opcache/jit/zend_jit.c @@ -2332,6 +2332,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op goto jit_failure; } goto done; + case ZEND_FETCH_OBJ_FUNC_ARG: case ZEND_FETCH_OBJ_R: case ZEND_FETCH_OBJ_IS: case ZEND_FETCH_OBJ_W: @@ -2369,11 +2370,31 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op || Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') { break; } - if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op, + if (opline->opcode == ZEND_FETCH_OBJ_FUNC_ARG) { + /* FETCH_OBJ_FUNC_ARG's by-value fetch dispatches into the */ + /* FETCH_OBJ_R handler, which may take the SIMPLE_GET hook fast */ + /* path and push a getter frame; by-ref dispatches into */ + /* FETCH_OBJ_W. The function JIT may keep values solely in */ + /* registers, so we must NOT exit to the VM (stale stack slots). */ + /* Inline the by-value path through zend_jit_fetch_obj, which runs */ + /* the hook getter inside a helper and keeps all registers live. */ + /* The by-ref path has no SIMPLE_GET fast path, so the generic */ + /* handler (a full C call, safe under register allocation) is used. */ + /* This mirrors the tracing JIT fix for GH-21006 (GH-21369); the */ + /* runtime by-ref check is required because the passing mode is */ + /* only known once the callee is resolved via namespace fallback. */ + /* See GH-22857. */ + if (!zend_jit_fetch_obj_func_arg(jit, opline, op_array, ssa, ssa_op, + op1_info, op1_addr, ce, ce_is_instanceof, on_this, RES_REG_ADDR())) { + goto jit_failure; + } + } else { + if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op, op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL, RES_REG_ADDR(), IS_UNKNOWN, zend_may_throw(opline, ssa_op, op_array, ssa))) { - goto jit_failure; + goto jit_failure; + } } goto done; case ZEND_BIND_GLOBAL: diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 2cc6a01c4107..b48058196c95 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -14647,6 +14647,59 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit, return 1; } +static int zend_jit_fetch_obj_func_arg(zend_jit_ctx *jit, const zend_op *opline, + const zend_op_array *op_array, zend_ssa *ssa, const zend_ssa_op *ssa_op, + uint32_t op1_info, zend_jit_addr op1_addr, zend_class_entry *ce, + bool ce_is_instanceof, bool on_this, zend_jit_addr res_addr) +{ + ir_ref rx, call_info, if_by_ref, end_by_ref; + + /* Both runtime paths must observe a consistent frame state. The delayed + * call chain would otherwise only be flushed inside the by-ref branch (by + * zend_jit_set_ip() in zend_jit_handler()), leaving EX(call) stale on the + * by-val path and after the merge. Flush it before branching. */ + if (jit->delayed_call_level) { + if (!zend_jit_save_call_chain(jit, jit->delayed_call_level)) { + return 0; + } + } + + /* JIT: if (ZEND_CALL_INFO(EX(call)) & ZEND_CALL_SEND_ARG_BY_REF) */ + if (jit->reuse_ip) { + rx = jit_IP(jit); + } else { + rx = ir_LOAD_A(jit_EX(call)); + } + call_info = ir_LOAD_U32(jit_CALL(rx, This.u1.type_info)); + if_by_ref = ir_IF(ir_AND_U32(call_info, ir_CONST_U32(ZEND_CALL_SEND_ARG_BY_REF))); + + /* by-ref path: the FUNC_ARG handler re-checks the flag and dispatches + * into FETCH_OBJ_W */ + ir_IF_TRUE_cold(if_by_ref); + if (!zend_jit_handler(jit, opline, zend_may_throw(opline, ssa_op, op_array, ssa))) { + return 0; + } + end_by_ref = ir_END(); + + /* zend_jit_handler() stored IP = opline + 1 on the by-ref path only; + * that compile-time knowledge is invalid for the by-val path and after + * the merge. */ + zend_jit_reset_last_valid_opline(jit); + + /* by-val path */ + ir_IF_FALSE(if_by_ref); + if (!zend_jit_fetch_obj(jit, opline, op_array, ssa, ssa_op, + op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL, + res_addr, IS_UNKNOWN, + zend_may_throw(opline, ssa_op, op_array, ssa))) { + return 0; + } + ir_MERGE_WITH(end_by_ref); + + return 1; +} + + static int zend_jit_assign_obj(zend_jit_ctx *jit, const zend_op *opline, const zend_op_array *op_array, diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt new file mode 100644 index 000000000000..2efd761f2fe7 --- /dev/null +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -0,0 +1,125 @@ +--TEST-- +GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook (SIMPLE_GET fast path) +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit_buffer_size=64M +opcache.jit=1205 +opcache.jit_hot_func=1 +--EXTENSIONS-- +opcache +--FILE-- + (self::$getterRan = true) + ? self::build($this->kind, $this->id) + : self::build($this->kind, $this->id); + } + + protected mixed $prev = null; + + public function __construct( + public protected(set) string $kind, + public protected(set) string $id, + ) { + $this->handler = DefaultHandler::getInstance(); + } + + public static function build(string $k, string $i): string { + return "/nonexistent/gh22857_{$k}_{$i}.dat"; + } + + public function step(): void { + /* Unqualified namespaced-fallback call (INIT_NS_FCALL_BY_NAME) keeps + * the FETCH_OBJ_FUNC_ARG opcode instead of letting the optimizer + * rewrite it to FETCH_OBJ_R. @ preserves the opcode shape that + * triggers the bug. */ + @file_get_contents($this->path); + if (!self::$getterRan) { + throw new \RuntimeException('getter did not run via FUNC_ARG'); + } + } +} + +$c = new Container('alpha', 'beta'); +$c->step(); +$c->step(); +$c->step(); + +/* Sibling-slot variant: a preceding plain FETCH_OBJ_R primes the + * SIMPLE_GET bit on the property cache slot; compact_literals shares the + * slot between FETCH_OBJ_R and FETCH_OBJ_FUNC_ARG for the same property, + * so the following FETCH_OBJ_FUNC_ARG consumes that bit and hits the + * SIMPLE_GET fast path. Without the hook-enter guard it passes whatever + * sits in an adjacent property slot instead of running the getter. The + * flag is reset after the priming FETCH_OBJ_R so the assertion reflects + * only whether the getter ran during the FETCH_OBJ_FUNC_ARG read. */ +class Container2 { + private static bool $getterRan = false; + + public protected(set) HandlerInterface $handler; + + public string $path { + get => (self::$getterRan = true) + ? self::build($this->kind, $this->id) + : self::build($this->kind, $this->id); + } + + protected mixed $prev = null; + + public function __construct( + public protected(set) string $kind, + public protected(set) string $id, + ) { + $this->handler = DefaultHandler::getInstance(); + } + + public static function build(string $k, string $i): string { + return "/nonexistent/gh22857b_{$k}_{$i}.dat"; + } + + public function step(): void { + $this->prev = $this->path; // FETCH_OBJ_R primes SIMPLE_GET on shared slot + self::$getterRan = false; // reset; flag now reflects only the FUNC_ARG read below + @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes the primed bit + if (!self::$getterRan) { + throw new \RuntimeException('sibling-slot variant: getter did not run via FUNC_ARG'); + } + } +} + +$c2 = new Container2('alpha', 'beta'); +$c2->step(); +$c2->step(); +$c2->step(); + +echo "OK\n"; +?> +--EXPECT-- +OK diff --git a/ext/opcache/tests/jit/gh22857_002.phpt b/ext/opcache/tests/jit/gh22857_002.phpt new file mode 100644 index 000000000000..c14ad309d96f --- /dev/null +++ b/ext/opcache/tests/jit/gh22857_002.phpt @@ -0,0 +1,47 @@ +--TEST-- +GH-22857 (reg-alloc): FETCH_OBJ_FUNC_ARG hook read must not lose register-held vars +--ENV-- +A=1 +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit_buffer_size=64M +opcache.jit=1205 +opcache.jit_hot_func=1 +--FILE-- +prop, $a ? 1 : 2); + return $b + $c; +} + +if (getenv('A')) { + function g($a, $b) { + var_dump($b); + return $a; + } +} + +$c = new C(); +var_dump(f(1, $c)); +var_dump(f(1, $c)); + +?> +--EXPECT-- +int(1) +int(4) +int(1) +int(4)