Skip to content

Commit 40c3e24

Browse files
arnaud-lbarnaud-lb
committed
CS
1 parent 0b9a0f0 commit 40c3e24

2 files changed

Lines changed: 142 additions & 142 deletions

File tree

‎ext/opcache/jit/zend_jit.c‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2371,28 +2371,28 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
23712371
break;
23722372
}
23732373
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. */
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. */
23872387
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())) {
2388+
op1_info, op1_addr, ce, ce_is_instanceof, on_this, RES_REG_ADDR())) {
23892389
goto jit_failure;
23902390
}
23912391
} else {
23922392
if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
2393-
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
2394-
RES_REG_ADDR(), IS_UNKNOWN,
2395-
zend_may_throw(opline, ssa_op, op_array, ssa))) {
2393+
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
2394+
RES_REG_ADDR(), IS_UNKNOWN,
2395+
zend_may_throw(opline, ssa_op, op_array, ssa))) {
23962396
goto jit_failure;
23972397
}
23982398
}

‎ext/opcache/tests/jit/gh22857.phpt‎

Lines changed: 125 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -1,125 +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
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

0 commit comments

Comments
 (0)