From 4add960e83129def6df0cc3243cd9d256902f4de Mon Sep 17 00:00:00 2001 From: coderzhao Date: Mon, 27 Jul 2026 11:33:16 +0800 Subject: [PATCH 1/5] Fix GH-22857: JIT wrong code for FETCH_OBJ_FUNC_ARG with property hooks FETCH_OBJ_FUNC_ARG dispatches into the FETCH_OBJ_R handler for by-value argument fetches (see ZEND_FETCH_OBJ_FUNC_ARG in zend_vm_def.h). When the shared property cache slot has the SIMPLE_GET bit set (either primed by the same FETCH_OBJ_FUNC_ARG opline on a previous iteration, or by a sibling FETCH_OBJ_R on the same property whose cache slot is merged by compact_literals), the FETCH_OBJ_R handler takes the SIMPLE_GET fast path and pushes the getter call frame, returning opline | ZEND_VM_ENTER_BIT to signal the caller to re-enter the VM. Function JIT handles this only for FETCH_OBJ_R via a hook-enter guard that compares the returned IP against opline+1 and tail-calls into the new frame when they differ. FETCH_OBJ_FUNC_ARG was compiled through the generic default handler with no such guard, so the JIT-emitted SEND_FUNC_ARG that follows read the argument slot before the getter had actually run. Depending on the adjacent property slot's contents, the symptom is a spurious 'must not contain any null bytes' ValueError, a 'expected string, got ' TypeError, or a zend_mm_heap corruption abort. Compile FETCH_OBJ_FUNC_ARG through the same path as FETCH_OBJ_R so it emits the hook-enter guard. Because FETCH_OBJ_FUNC_ARG is not handled by the INLINE-only switch above, the shared 'ce' local can carry a stale value from a previous opline iteration; reset it to NULL for FETCH_OBJ_FUNC_ARG so the guard is always emitted regardless of any final/no-hooks fast path that the FETCH_OBJ_R case may otherwise skip. The by-reference dispatch (to FETCH_OBJ_W) never takes the SIMPLE_GET fast path, so the guard is a no-op there. This mirrors the JIT-only shape used for the tracing JIT in GH-21369 (which fixed the analogous GH-21006 for tracing). Fixes GH-22857 --- NEWS | 3 + ext/opcache/jit/zend_jit.c | 24 ++++++- ext/opcache/tests/jit/gh22857.phpt | 106 +++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 ext/opcache/tests/jit/gh22857.phpt diff --git a/NEWS b/NEWS index 217d6b959c81..4eecc7caadd8 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,9 @@ PHP NEWS - Opcache: . Re-enable JIT for ZTS builds on Apple Silicon. (realFlowControl) + . Fixed bug GH-22857 (Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG + on a property hook, causing heap corruption or spurious ValueError / + TypeError under opcache.jit=1205). (coderzhao) - PDO_ODBC: . Fixed bug GH-22667 (Heap buffer over-read when a column value exceeds the diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index cb2791fb45ac..1f11c79ca6c6 100644 --- a/ext/opcache/jit/zend_jit.c +++ b/ext/opcache/jit/zend_jit.c @@ -2447,6 +2447,12 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op goto jit_failure; } goto done; + /* ZEND_FETCH_OBJ_FUNC_ARG is intentionally not handled here. + * Its by-value fetch dispatches into the FETCH_OBJ_R handler, + * which can push a hook getter frame via the SIMPLE_GET fast + * path. Compiling it through the generic switch below emits + * the hook-enter guard alongside the FETCH_OBJ_R case; see + * GH-22857. */ case ZEND_FETCH_OBJ_R: case ZEND_FETCH_OBJ_IS: case ZEND_FETCH_OBJ_W: @@ -2855,14 +2861,28 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op } break; case ZEND_FETCH_OBJ_R: + case ZEND_FETCH_OBJ_FUNC_ARG: if (!zend_jit_handler(&ctx, opline, zend_may_throw(opline, ssa_op, op_array, ssa))) { goto jit_failure; } - /* Cache slot is only used for IS_CONST op2, so only that can result in hook fast path. */ + /* Cache slot is only used for IS_CONST op2, so only that can result in hook fast path. + * FETCH_OBJ_FUNC_ARG dispatches into the FETCH_OBJ_R handler for by-value argument + * fetches (see ZEND_FETCH_OBJ_FUNC_ARG in zend_vm_def.h), which may take the + * SIMPLE_GET fast path and push the hook call frame, returning opline | ENTER_BIT. + * Without a matching hook-enter guard, the JIT-compiled SEND_FUNC_ARG would read + * the argument slot before the getter has actually run. Emit the same guard as for + * the plain FETCH_OBJ_R case. The by-reference dispatch (to FETCH_OBJ_W) never + * takes the SIMPLE_GET fast path, so the guard is a no-op there. */ if (opline->op2_type == IS_CONST) { - if (JIT_G(opt_level) < ZEND_JIT_LEVEL_INLINE) { + if (opline->opcode == ZEND_FETCH_OBJ_FUNC_ARG) { + /* FETCH_OBJ_FUNC_ARG is not handled by the INLINE-only + * switch above, so `ce` here is whatever the previous + * opline iteration happened to leave in it. Reset it + * so we always emit the hook-enter guard. */ + ce = NULL; + } else if (JIT_G(opt_level) < ZEND_JIT_LEVEL_INLINE) { if (opline->op1_type == IS_UNUSED) { ce = op_array->scope; } else { diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt new file mode 100644 index 000000000000..27c129d47f8a --- /dev/null +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -0,0 +1,106 @@ +--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::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 so op1 of SEND is a + * FETCH_OBJ_FUNC_ARG (INIT_NS_FCALL_BY_NAME). @ silences the + * expected file-not-found warning. */ + $r = @file_get_contents($this->path); + if ($r !== false) { + throw new \RuntimeException('unexpected non-false return'); + } + } +} + +$c = new Container('alpha', 'beta'); +for ($i = 0; $i < 200; $i++) { + $c->step(); +} + +/* Sibling-slot variant: a preceding plain FETCH_OBJ_R can prime 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 a following FETCH_OBJ_FUNC_ARG will consume that bit and hit the + * SIMPLE_GET fast path. Without the hook-enter guard it reads garbage + * from an adjacent property slot. */ +class Container2 { + public protected(set) HandlerInterface $handler; + + public string $path { + get => 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 { + $warm = $this->path; // FETCH_OBJ_R primes SIMPLE_GET on the shared slot + $r = @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes it + if ($r !== false || !\is_string($warm)) { + throw new \RuntimeException('unexpected values'); + } + } +} + +$c2 = new Container2('alpha', 'beta'); +for ($i = 0; $i < 200; $i++) { + $c2->step(); +} + +echo "OK\n"; +?> +--EXPECT-- +OK From 287ce80bd1c7697fed9875ef9c505375da30773a Mon Sep 17 00:00:00 2001 From: coderzhao Date: Mon, 27 Jul 2026 18:17:51 +0800 Subject: [PATCH 2/5] Fix gh22857.phpt: avoid unrelated pre-existing JIT leak in the warm-up read The sibling-slot variant used '$warm = $this->path;' to prime the SIMPLE_GET bit. Storing the hook result in a local variable trips a pre-existing, unrelated function JIT memory leak that also reproduces on master without this fix (hooked FETCH_OBJ_R result assigned to a CV leaks one string per call at opcache.jit=1203+). Transfer the hook result into a property instead, which still primes the shared cache slot but keeps this test independent of that separate bug. The FETCH_OBJ_FUNC_ARG coverage is unchanged: without the JIT fix this test still crashes, and with it it passes leak-free. --- ext/opcache/tests/jit/gh22857.phpt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt index 27c129d47f8a..959a585d24bd 100644 --- a/ext/opcache/tests/jit/gh22857.phpt +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -87,9 +87,13 @@ class Container2 { } public function step(): void { - $warm = $this->path; // FETCH_OBJ_R primes SIMPLE_GET on the shared slot - $r = @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes it - if ($r !== false || !\is_string($warm)) { + /* FETCH_OBJ_R primes SIMPLE_GET on the shared slot. The hook + * result is stored into a property (ownership transfer) instead + * of a local, so this test stays independent of how the JIT + * releases temporaries of hooked property reads. */ + $this->prev = $this->path; + $r = @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes the primed bit + if ($r !== false || !\is_string($this->prev)) { throw new \RuntimeException('unexpected values'); } } From a447b398a7b291b7aa5b88fec8a9d94f9e3d7a45 Mon Sep 17 00:00:00 2001 From: coderZhao Date: Mon, 27 Jul 2026 19:43:33 +0800 Subject: [PATCH 3/5] Update ext/opcache/tests/jit/gh22857.phpt Co-authored-by: Weilin Du --- ext/opcache/tests/jit/gh22857.phpt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt index 959a585d24bd..9457c07c97b6 100644 --- a/ext/opcache/tests/jit/gh22857.phpt +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -56,9 +56,9 @@ class Container { } $c = new Container('alpha', 'beta'); -for ($i = 0; $i < 200; $i++) { - $c->step(); -} +$c->step(); +$c->step(); +$c->step(); /* Sibling-slot variant: a preceding plain FETCH_OBJ_R can prime the * SIMPLE_GET bit on the property cache slot; compact_literals shares the From 05c5cc364a3373319021a0715bd4edf3ea2628a7 Mon Sep 17 00:00:00 2001 From: coderZhao Date: Mon, 27 Jul 2026 19:43:42 +0800 Subject: [PATCH 4/5] Update ext/opcache/tests/jit/gh22857.phpt Co-authored-by: Weilin Du --- ext/opcache/tests/jit/gh22857.phpt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt index 9457c07c97b6..301a2180bf3a 100644 --- a/ext/opcache/tests/jit/gh22857.phpt +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -100,9 +100,9 @@ class Container2 { } $c2 = new Container2('alpha', 'beta'); -for ($i = 0; $i < 200; $i++) { - $c2->step(); -} +$c2->step(); +$c2->step(); +$c2->step(); echo "OK\n"; ?> From 10821a1dde7b65dd05874d69eb91a7e603273cd1 Mon Sep 17 00:00:00 2001 From: coderzhao Date: Tue, 28 Jul 2026 11:43:31 +0800 Subject: [PATCH 5/5] 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 --- ext/opcache/tests/jit/gh22857.phpt | 63 ++++++++++++++++++------------ 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/ext/opcache/tests/jit/gh22857.phpt b/ext/opcache/tests/jit/gh22857.phpt index 301a2180bf3a..fde36d19b390 100644 --- a/ext/opcache/tests/jit/gh22857.phpt +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -20,15 +20,26 @@ final class DefaultHandler implements HandlerInterface { public function noop(): void {} } -/* Repro of the original issue: virtual property hook read via - * FETCH_OBJ_FUNC_ARG under function JIT, where the getter frame is pushed - * by the SIMPLE_GET fast path in the shared FETCH_OBJ_R handler but the - * JIT-compiled FUNC_ARG opcode has no hook-enter guard. */ +/* Original issue: virtual property hook read via FETCH_OBJ_FUNC_ARG under + * function JIT. The getter frame is pushed by the SIMPLE_GET fast path in the + * shared FETCH_OBJ_R handler, but the JIT-compiled FUNC_ARG opcode had no + * hook-enter guard, so the argument slot was read before the getter ran. + * + * The assertion is deterministic: the getter sets a static flag as a side + * effect, so we check whether the getter actually ran when the property is + * passed through FETCH_OBJ_FUNC_ARG. This avoids the previous data-dependent + * failure mode (relying on file_get_contents() fataling on whatever garbage + * the unguarded JIT read happened to land on), which made the regression + * test flaky. */ class Container { + private static bool $getterRan = false; + public protected(set) HandlerInterface $handler; public string $path { - get => self::build($this->kind, $this->id); + get => (self::$getterRan = true) + ? self::build($this->kind, $this->id) + : self::build($this->kind, $this->id); } protected mixed $prev = null; @@ -45,12 +56,13 @@ class Container { } public function step(): void { - /* Unqualified namespaced-fallback call so op1 of SEND is a - * FETCH_OBJ_FUNC_ARG (INIT_NS_FCALL_BY_NAME). @ silences the - * expected file-not-found warning. */ - $r = @file_get_contents($this->path); - if ($r !== false) { - throw new \RuntimeException('unexpected non-false return'); + /* 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'); } } } @@ -60,17 +72,23 @@ $c->step(); $c->step(); $c->step(); -/* Sibling-slot variant: a preceding plain FETCH_OBJ_R can prime the +/* 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 a following FETCH_OBJ_FUNC_ARG will consume that bit and hit the - * SIMPLE_GET fast path. Without the hook-enter guard it reads garbage - * from an adjacent property slot. */ + * 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::build($this->kind, $this->id); + get => (self::$getterRan = true) + ? self::build($this->kind, $this->id) + : self::build($this->kind, $this->id); } protected mixed $prev = null; @@ -87,14 +105,11 @@ class Container2 { } public function step(): void { - /* FETCH_OBJ_R primes SIMPLE_GET on the shared slot. The hook - * result is stored into a property (ownership transfer) instead - * of a local, so this test stays independent of how the JIT - * releases temporaries of hooked property reads. */ - $this->prev = $this->path; - $r = @file_get_contents($this->path); // FETCH_OBJ_FUNC_ARG consumes the primed bit - if ($r !== false || !\is_string($this->prev)) { - throw new \RuntimeException('unexpected values'); + $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'); } } }