Skip to content

Commit 10821a1

Browse files
author
coderzhao
committed
fix: Improve gh22857.phpt: use deterministic assertion for FETCH_OBJ_FUNC_ARG JIT bug
The previous version relied on file_get_contents() throwing when the unguarded JIT read landed on whatever garbage sat in an adjacent property slot. That failure is data-dependent, which is why the old test needed a 200-iteration loop to become reliable. The getter now records a side effect (a static flag). After the property is read through FETCH_OBJ_FUNC_ARG we assert the flag is set, i.e. that the getter actually ran. This makes the failure deterministic, so the loop drops to a few calls. Both the self-priming variant and the sibling-slot variant (compact_literals sharing the cache slot, decided at compile time) are preserved, and the trigger that emits FETCH_OBJ_FUNC_ARG (unqualified namespaced-fallback call under @) is unchanged. GH-22857
1 parent 5c4816c commit 10821a1

1 file changed

Lines changed: 39 additions & 24 deletions

File tree

ext/opcache/tests/jit/gh22857.phpt

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,26 @@ final class DefaultHandler implements HandlerInterface {
2020
public function noop(): void {}
2121
}
2222

23-
/* Repro of the original issue: virtual property hook read via
24-
* FETCH_OBJ_FUNC_ARG under function JIT, where the getter frame is pushed
25-
* by the SIMPLE_GET fast path in the shared FETCH_OBJ_R handler but the
26-
* JIT-compiled FUNC_ARG opcode has no hook-enter guard. */
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. */
2734
class Container {
35+
private static bool $getterRan = false;
36+
2837
public protected(set) HandlerInterface $handler;
2938

3039
public string $path {
31-
get => self::build($this->kind, $this->id);
40+
get => (self::$getterRan = true)
41+
? self::build($this->kind, $this->id)
42+
: self::build($this->kind, $this->id);
3243
}
3344

3445
protected mixed $prev = null;
@@ -45,12 +56,13 @@ class Container {
4556
}
4657

4758
public function step(): void {
48-
/* Unqualified namespaced-fallback call so op1 of SEND is a
49-
* FETCH_OBJ_FUNC_ARG (INIT_NS_FCALL_BY_NAME). @ silences the
50-
* expected file-not-found warning. */
51-
$r = @file_get_contents($this->path);
52-
if ($r !== false) {
53-
throw new \RuntimeException('unexpected non-false return');
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');
5466
}
5567
}
5668
}
@@ -60,17 +72,23 @@ $c->step();
6072
$c->step();
6173
$c->step();
6274

63-
/* Sibling-slot variant: a preceding plain FETCH_OBJ_R can prime the
75+
/* Sibling-slot variant: a preceding plain FETCH_OBJ_R primes the
6476
* SIMPLE_GET bit on the property cache slot; compact_literals shares the
6577
* slot between FETCH_OBJ_R and FETCH_OBJ_FUNC_ARG for the same property,
66-
* so a following FETCH_OBJ_FUNC_ARG will consume that bit and hit the
67-
* SIMPLE_GET fast path. Without the hook-enter guard it reads garbage
68-
* from an adjacent property slot. */
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. */
6983
class Container2 {
84+
private static bool $getterRan = false;
85+
7086
public protected(set) HandlerInterface $handler;
7187

7288
public string $path {
73-
get => self::build($this->kind, $this->id);
89+
get => (self::$getterRan = true)
90+
? self::build($this->kind, $this->id)
91+
: self::build($this->kind, $this->id);
7492
}
7593

7694
protected mixed $prev = null;
@@ -87,14 +105,11 @@ class Container2 {
87105
}
88106

89107
public function step(): void {
90-
/* FETCH_OBJ_R primes SIMPLE_GET on the shared slot. The hook
91-
* result is stored into a property (ownership transfer) instead
92-
* of a local, so this test stays independent of how the JIT
93-
* releases temporaries of hooked property reads. */
94-
$this->prev = $this->path;
95-
$r = @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes the primed bit
96-
if ($r !== false || !\is_string($this->prev)) {
97-
throw new \RuntimeException('unexpected values');
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');
98113
}
99114
}
100115
}

0 commit comments

Comments
 (0)