Skip to content

Commit bb605d3

Browse files
author
coderzhao
committed
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).
1 parent 2804d09 commit bb605d3

5 files changed

Lines changed: 252 additions & 2 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ PHP NEWS
2727
. Fixed bug GH-15836 (Use-after-free when a user stream filter accesses
2828
$this->stream during the close flush). (iliaal)
2929

30+
- Opcache:
31+
. Fixed bug GH-22857 (Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a
32+
property hook getter, losing register-held variables). (zhaohao19941221)
33+
3034
30 Jul 2026, PHP 8.4.24
3135

3236
- BCMath:

ext/opcache/jit/zend_jit.c

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,6 +2332,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
23322332
goto jit_failure;
23332333
}
23342334
goto done;
2335+
case ZEND_FETCH_OBJ_FUNC_ARG:
23352336
case ZEND_FETCH_OBJ_R:
23362337
case ZEND_FETCH_OBJ_IS:
23372338
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
23692370
|| Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
23702371
break;
23712372
}
2372-
if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
2373+
if (opline->opcode == ZEND_FETCH_OBJ_FUNC_ARG) {
2374+
/* FETCH_OBJ_FUNC_ARG's by-value fetch dispatches into the */
2375+
/* FETCH_OBJ_R handler, which may take the SIMPLE_GET hook fast */
2376+
/* path and push a getter frame; by-ref dispatches into */
2377+
/* FETCH_OBJ_W. The function JIT may keep values solely in */
2378+
/* registers, so we must NOT exit to the VM (stale stack slots). */
2379+
/* Inline the by-value path through zend_jit_fetch_obj, which runs */
2380+
/* the hook getter inside a helper and keeps all registers live. */
2381+
/* The by-ref path has no SIMPLE_GET fast path, so the generic */
2382+
/* handler (a full C call, safe under register allocation) is used. */
2383+
/* This mirrors the tracing JIT fix for GH-21006 (GH-21369); the */
2384+
/* runtime by-ref check is required because the passing mode is */
2385+
/* only known once the callee is resolved via namespace fallback. */
2386+
/* See GH-22857. */
2387+
if (!zend_jit_fetch_obj_func_arg(jit, opline, op_array, ssa, ssa_op,
2388+
op1_info, op1_addr, ce, ce_is_instanceof, on_this, RES_REG_ADDR())) {
2389+
goto jit_failure;
2390+
}
2391+
} else {
2392+
if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
23732393
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
23742394
RES_REG_ADDR(), IS_UNKNOWN,
23752395
zend_may_throw(opline, ssa_op, op_array, ssa))) {
2376-
goto jit_failure;
2396+
goto jit_failure;
2397+
}
23772398
}
23782399
goto done;
23792400
case ZEND_BIND_GLOBAL:

ext/opcache/jit/zend_jit_ir.c

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14647,6 +14647,59 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit,
1464714647
return 1;
1464814648
}
1464914649

14650+
static int zend_jit_fetch_obj_func_arg(zend_jit_ctx *jit, const zend_op *opline,
14651+
const zend_op_array *op_array, zend_ssa *ssa, const zend_ssa_op *ssa_op,
14652+
uint32_t op1_info, zend_jit_addr op1_addr, zend_class_entry *ce,
14653+
bool ce_is_instanceof, bool on_this, zend_jit_addr res_addr)
14654+
{
14655+
ir_ref rx, call_info, if_by_ref, end_by_ref;
14656+
14657+
/* Both runtime paths must observe a consistent frame state. The delayed
14658+
* call chain would otherwise only be flushed inside the by-ref branch (by
14659+
* zend_jit_set_ip() in zend_jit_handler()), leaving EX(call) stale on the
14660+
* by-val path and after the merge. Flush it before branching. */
14661+
if (jit->delayed_call_level) {
14662+
if (!zend_jit_save_call_chain(jit, jit->delayed_call_level)) {
14663+
return 0;
14664+
}
14665+
}
14666+
14667+
/* JIT: if (ZEND_CALL_INFO(EX(call)) & ZEND_CALL_SEND_ARG_BY_REF) */
14668+
if (jit->reuse_ip) {
14669+
rx = jit_IP(jit);
14670+
} else {
14671+
rx = ir_LOAD_A(jit_EX(call));
14672+
}
14673+
call_info = ir_LOAD_U32(jit_CALL(rx, This.u1.type_info));
14674+
if_by_ref = ir_IF(ir_AND_U32(call_info, ir_CONST_U32(ZEND_CALL_SEND_ARG_BY_REF)));
14675+
14676+
/* by-ref path: the FUNC_ARG handler re-checks the flag and dispatches
14677+
* into FETCH_OBJ_W */
14678+
ir_IF_TRUE_cold(if_by_ref);
14679+
if (!zend_jit_handler(jit, opline, zend_may_throw(opline, ssa_op, op_array, ssa))) {
14680+
return 0;
14681+
}
14682+
end_by_ref = ir_END();
14683+
14684+
/* zend_jit_handler() stored IP = opline + 1 on the by-ref path only;
14685+
* that compile-time knowledge is invalid for the by-val path and after
14686+
* the merge. */
14687+
zend_jit_reset_last_valid_opline(jit);
14688+
14689+
/* by-val path */
14690+
ir_IF_FALSE(if_by_ref);
14691+
if (!zend_jit_fetch_obj(jit, opline, op_array, ssa, ssa_op,
14692+
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
14693+
res_addr, IS_UNKNOWN,
14694+
zend_may_throw(opline, ssa_op, op_array, ssa))) {
14695+
return 0;
14696+
}
14697+
ir_MERGE_WITH(end_by_ref);
14698+
14699+
return 1;
14700+
}
14701+
14702+
1465014703
static int zend_jit_assign_obj(zend_jit_ctx *jit,
1465114704
const zend_op *opline,
1465214705
const zend_op_array *op_array,

ext/opcache/tests/jit/gh22857.phpt

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
--TEST--
2+
GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook (SIMPLE_GET fast path)
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.jit_buffer_size=64M
7+
opcache.jit=1205
8+
opcache.jit_hot_func=1
9+
--EXTENSIONS--
10+
opcache
11+
--FILE--
12+
<?php
13+
namespace Test;
14+
15+
interface HandlerInterface { public function noop(): void; }
16+
17+
final class DefaultHandler implements HandlerInterface {
18+
private static ?self $i = null;
19+
public static function getInstance(): self { return self::$i ??= new self(); }
20+
public function noop(): void {}
21+
}
22+
23+
/* Original issue: virtual property hook read via FETCH_OBJ_FUNC_ARG under
24+
* function JIT. The getter frame is pushed by the SIMPLE_GET fast path in the
25+
* shared FETCH_OBJ_R handler, but the JIT-compiled FUNC_ARG opcode had no
26+
* hook-enter guard, so the argument slot was read before the getter ran.
27+
*
28+
* The assertion is deterministic: the getter sets a static flag as a side
29+
* effect, so we check whether the getter actually ran when the property is
30+
* passed through FETCH_OBJ_FUNC_ARG. This avoids the previous data-dependent
31+
* failure mode (relying on file_get_contents() fataling on whatever garbage
32+
* the unguarded JIT read happened to land on), which made the regression
33+
* test flaky. */
34+
class Container {
35+
private static bool $getterRan = false;
36+
37+
public protected(set) HandlerInterface $handler;
38+
39+
public string $path {
40+
get => (self::$getterRan = true)
41+
? self::build($this->kind, $this->id)
42+
: self::build($this->kind, $this->id);
43+
}
44+
45+
protected mixed $prev = null;
46+
47+
public function __construct(
48+
public protected(set) string $kind,
49+
public protected(set) string $id,
50+
) {
51+
$this->handler = DefaultHandler::getInstance();
52+
}
53+
54+
public static function build(string $k, string $i): string {
55+
return "/nonexistent/gh22857_{$k}_{$i}.dat";
56+
}
57+
58+
public function step(): void {
59+
/* Unqualified namespaced-fallback call (INIT_NS_FCALL_BY_NAME) keeps
60+
* the FETCH_OBJ_FUNC_ARG opcode instead of letting the optimizer
61+
* rewrite it to FETCH_OBJ_R. @ preserves the opcode shape that
62+
* triggers the bug. */
63+
@file_get_contents($this->path);
64+
if (!self::$getterRan) {
65+
throw new \RuntimeException('getter did not run via FUNC_ARG');
66+
}
67+
}
68+
}
69+
70+
$c = new Container('alpha', 'beta');
71+
$c->step();
72+
$c->step();
73+
$c->step();
74+
75+
/* Sibling-slot variant: a preceding plain FETCH_OBJ_R primes the
76+
* SIMPLE_GET bit on the property cache slot; compact_literals shares the
77+
* slot between FETCH_OBJ_R and FETCH_OBJ_FUNC_ARG for the same property,
78+
* so the following FETCH_OBJ_FUNC_ARG consumes that bit and hits the
79+
* SIMPLE_GET fast path. Without the hook-enter guard it passes whatever
80+
* sits in an adjacent property slot instead of running the getter. The
81+
* flag is reset after the priming FETCH_OBJ_R so the assertion reflects
82+
* only whether the getter ran during the FETCH_OBJ_FUNC_ARG read. */
83+
class Container2 {
84+
private static bool $getterRan = false;
85+
86+
public protected(set) HandlerInterface $handler;
87+
88+
public string $path {
89+
get => (self::$getterRan = true)
90+
? self::build($this->kind, $this->id)
91+
: self::build($this->kind, $this->id);
92+
}
93+
94+
protected mixed $prev = null;
95+
96+
public function __construct(
97+
public protected(set) string $kind,
98+
public protected(set) string $id,
99+
) {
100+
$this->handler = DefaultHandler::getInstance();
101+
}
102+
103+
public static function build(string $k, string $i): string {
104+
return "/nonexistent/gh22857b_{$k}_{$i}.dat";
105+
}
106+
107+
public function step(): void {
108+
$this->prev = $this->path; // FETCH_OBJ_R primes SIMPLE_GET on shared slot
109+
self::$getterRan = false; // reset; flag now reflects only the FUNC_ARG read below
110+
@file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes the primed bit
111+
if (!self::$getterRan) {
112+
throw new \RuntimeException('sibling-slot variant: getter did not run via FUNC_ARG');
113+
}
114+
}
115+
}
116+
117+
$c2 = new Container2('alpha', 'beta');
118+
$c2->step();
119+
$c2->step();
120+
$c2->step();
121+
122+
echo "OK\n";
123+
?>
124+
--EXPECT--
125+
OK
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
GH-22857 (reg-alloc): FETCH_OBJ_FUNC_ARG hook read must not lose register-held vars
3+
--ENV--
4+
A=1
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=64M
9+
opcache.jit=1205
10+
opcache.jit_hot_func=1
11+
--FILE--
12+
<?php
13+
14+
class C {
15+
public $prop {
16+
get { return 1; }
17+
}
18+
}
19+
20+
function f(int $a0, $obj) {
21+
// $a lives only in a register: every use is in a supported opcode and in a
22+
// non-entry basic block, so the register allocator never spills it to the
23+
// VM stack. Exiting to the VM (the buggy hook-enter guard) would read a
24+
// stale stack slot for $a.
25+
$a = $a0;
26+
$b = $a + 2;
27+
$c = g($obj->prop, $a ? 1 : 2);
28+
return $b + $c;
29+
}
30+
31+
if (getenv('A')) {
32+
function g($a, $b) {
33+
var_dump($b);
34+
return $a;
35+
}
36+
}
37+
38+
$c = new C();
39+
var_dump(f(1, $c));
40+
var_dump(f(1, $c));
41+
42+
?>
43+
--EXPECT--
44+
int(1)
45+
int(4)
46+
int(1)
47+
int(4)

0 commit comments

Comments
 (0)