Fix GH-22857: JIT wrong code for FETCH_OBJ_FUNC_ARG with property hooks

[zhaohao19941221]

Summary

Fix GH-22857: heap corruption / spurious ValueError / TypeError when a property hook is read via FETCH_OBJ_FUNC_ARG under function JIT (opcache.jit=1205).

This supersedes the earlier engine-side attempt in the previous PR (fix-22857-JIT-virtual-property-hook-as-arg-to-unqualified-namespaced-fallback-coderzhao-2026-07-22). Based on review feedback from @arnaud-lb and @iliaal, the fix has been rewritten as a JIT-only change that mirrors the shape of GH-21369 (which fixed the analogous GH-21006 for tracing JIT).

Fixes GH-22857.

Root cause

ZEND_FETCH_OBJ_FUNC_ARG dispatches into the ZEND_FETCH_OBJ_R handler for by-value argument fetches (see zend_vm_def.h). When the shared property cache slot has the SIMPLE_GET bit set, the FETCH_OBJ_R handler:

1. Pushes the getter call frame onto the VM stack.
2. Sets EG(current_execute_data) to the new frame.
3. Returns opline | ZEND_VM_ENTER_BIT to signal the caller to re-enter the VM at the new frame.

The SIMPLE_GET bit can be set in two ways:

• Self-primed: the same FETCH_OBJ_FUNC_ARG opline primed it on a previous iteration (via slow path in zend_std_read_property()).
• Sibling-primed: a preceding plain FETCH_OBJ_R on the same property primed it, and compact_literals has merged the two oplines' cache slots.

Function JIT already handles this case for ZEND_FETCH_OBJ_R via a hook-enter guard in zend_jit.c that compares the returned IP against opline+1 and tail-calls into the new frame when they differ. However, ZEND_FETCH_OBJ_FUNC_ARG was compiled through the generic default: branch 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:

• ValueError: file_get_contents(): Argument #1 ($filename) must not contain any null bytes (deterministic 20/20 in the reproducer)
• TypeError: <fn>(): Argument #N ($arg) must be of type ?string, <adjacent-class> given (observed in the real-world Hyperf/Swoole application that led to this report)
• zend_mm_heap corrupted → SIGABRT (also observed in the real-world code)

All three symptoms have the same root cause: SEND_FUNC_ARG picking up garbage from an adjacent property slot before the getter has run.

Fix

Compile ZEND_FETCH_OBJ_FUNC_ARG through the same path as ZEND_FETCH_OBJ_R so it emits the hook-enter guard. Because ZEND_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 (that fast path only exists in the FETCH_OBJ_R handler), so the guard is a run-time no-op there: the by-ref path always returns with IP == opline+1 and the IR_IF_TRUE branch is taken.

This mirrors the JIT-only shape used for the tracing JIT in GH-21369 (which fixed the analogous GH-21006 for tracing).

Alternatives considered

• Engine-side fix in zend_std_read_property() (the shape of the previous PR): only prime SIMPLE_GET when the current opline is ZEND_FETCH_OBJ_R. Rejected because it does not close the sibling-primed variant ($warm = $this->path; @file_get_contents($this->path);) that @iliaal called out — compact_literals shares the cache slot, and the JIT-compiled FUNC_ARG still consumes a bit primed by the sibling FETCH_OBJ_R.
• Splitting FETCH_OBJ_FUNC_ARG into by-value / by-ref paths in JIT (@arnaud-lb's suggestion of dispatching to zend_jit_fetch_obj() for by-value and a cold-path handler for by-ref): would work but is a larger change; the current shape reuses the exact same generic-handler + hook-enter-guard idiom the FETCH_OBJ_R case already uses, keeping the fix minimal.

Necessary conditions (established by systematic delta-debugging)

Removing any one of these makes the bug disappear. Each condition was verified with 20 runs per variant:

#	Condition
1	opcache.jit=1205 (function JIT). disable and tracing are 20/20 OK.
2	Class is inside a namespace.
3	Call is unqualified (no leading \, no use function) so INIT_NS_FCALL_BY_NAME + FETCH_OBJ_FUNC_ARG is emitted (optimizer can't resolve namespaced-fallback targets, so the FETCH_OBJ_FUNC_ARG isn't rewritten to FETCH_OBJ_R).
4	The argument is a virtual property hook (get-only, no backing storage).
5	The hook is passed directly (opcode FETCH_OBJ_FUNC_ARG). Assigning to a local first uses FETCH_OBJ_R and hides the bug.
6	The call is wrapped in @ (BEGIN_SILENCE/END_SILENCE).
7	Class layout with adjacent asymmetric-visibility fields plus two promoted asymmetric-visibility constructor parameters.
8	Hook's get => calls a static method reading the promoted asymmetric properties.

Reproducer (before the fix)

$ php -d opcache.enable_cli=1 -d opcache.jit_buffer_size=64M \
      -d opcache.jit=1205 ext/opcache/tests/jit/gh22857.phpt

Without the fix: 20/20 crashing / erroring with the symptoms above.
With opcache.jit=disable or opcache.jit=tracing: 20/20 clean.
With the fix applied: 20/20 clean for all three JIT modes.

Test

ext/opcache/tests/jit/gh22857.phpt — 200 iterations covering both trigger paths:

• Container::step() — original issue: the same FETCH_OBJ_FUNC_ARG opline primes SIMPLE_GET on iteration 1 and consumes it on iteration 2.
• Container2::step() — sibling-slot variant that @iliaal pointed out: a preceding FETCH_OBJ_R on the same property primes the shared cache slot, and the following FETCH_OBJ_FUNC_ARG consumes it.

Fails before the fix (20/20 under jit=1205), passes after.

Backport

The bug has been present since PHP 8.4 (property hooks introduced in 8.4). Targeting master (PHP 8.6-dev) as suggested; happy to also target PHP-8.4 / PHP-8.5 if maintainers prefer.

Related

• Fixes Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a virtual property hook when calling an unqualified namespaced-fallback function #22857
• Mirrors the JIT-only shape used by Fix GH-21006: JIT SEGV with FETCH_OBJ_FUNC_ARG and property hooks #21369 (which fixed the analogous JIT has wrong expectations for ZEND_FETCH_OBJ_FUNC_ARG when the prop is hooked #21006 for tracing JIT).
• Fix GH-22857: virtual property hook mis-primes SIMPLE_GET under FUNC_ARG #22867 (which was correctly rejected by @arnaud-lb and @iliaal for being both wrong-layer and incomplete against the sibling-slot variant).

Thanks @arnaud-lb and @iliaal for the review feedback that redirected this to the correct fix.

[zhaohao19941221]

Ping @arnaud-lb @iliaal — this is the JIT-only rework you suggested; would appreciate another look when you have time. Thanks!

[LamentXU123]

I don't see the reason to loop it 200 times. In your original reproduce script you loop it 5000 times

If I am understanding your bug correctly, two or three calls can reproduce the bug. But maybe I am missing something

Update: I test this locally, with only one single $c->step() call can stably reproduce the bug.

[zhaohao19941221]

I don't see the reason to loop it 200 times. In your original reproduce script you loop it 5000 times

If I am understanding your bug correctly, two or three calls can reproduce the bug. But maybe I am missing something

Update: I test this locally, with only one single $c->step() call can stably reproduce the bug.

@LamentXU123 Thanks for the review — you're right that the bug triggers after just a couple of calls, so let me clarify why the loop is there, since it's not about the number of calls needed to trigger it.

Mechanism: on the first step() the function runs in the VM and primes the SIMPLE_GET bit on the property's cache slot (slow path, getter runs correctly). The second call is JIT-compiled and consumes that primed bit; without the guard the JIT reads whatever sits in the adjacent property slot instead of running the getter. So ~2 iterations is enough to trigger the wrong behavior.

The 200-iteration loop is a reliability measure for the regression test, not a trigger requirement. The unguarded read consumes whatever bytes happen to be in the sibling slot, and whether that produces a visible failure (a thrown \Error/\TypeError from file_get_contents, or heap corruption) is data-dependent and non-deterministic. With only 2–3 iterations a buggy build can occasionally slip through (false negative), defeating the purpose of a regression test — I verified in the PR description that without the fix it fails 20/20, and the loop is what makes that consistent.

This matters most for the second variant (Container2): compact_literals can merge the cache slots of the preceding FETCH_OBJ_R and the FETCH_OBJ_FUNC_ARG, so whether the shared-slot case is actually hit is sensitive to literal compaction / JIT recompilation timing — a loop guarantees we exercise it.

If you'd prefer a smaller loop I'm happy to shrink it, or alternatively I can make the assertion deterministic by checking the property value directly instead of relying on file_get_contents fataling — that would let us shrink the loop while still reliably failing on a buggy build. Let me know which you prefer.

[LamentXU123]

for Container2, the loop itself does not make compact_literals merge the cache slots. that should be determined at compile time. The loop only repeats the bad read once that shape is already present.

I would prefer the deterministic assertion approach you mentioned.

IMO A regression test that needs 200 iterations to become reliable is usually a sign that we should assert the wrong value behavior more directly.

[zhaohao19941221]

for Container2, the loop itself does not make compact_literals merge the cache slots. that should be determined at compile time. The loop only repeats the bad read once that shape is already present.

I would prefer the deterministic assertion approach you mentioned.

IMO A regression test that needs 200 iterations to become reliable is usually a sign that we should assert the wrong value behavior more directly.

@LamentXU123 Thanks — I've reworked the test to use the deterministic assertion approach you preferred.

What changed:

The assertion no longer relies on file_get_contents() fatally erroring on whatever garbage the unguarded JIT read happened to land on (that was data-dependent, which is exactly why the old version needed 200 iterations to become reliable).
Instead, the getter now records a side effect: it sets a static flag (self::$getterRan = true). After the property is read through FETCH_OBJ_FUNC_ARG, we assert that flag is set — this directly checks whether the getter actually ran, which is the precise wrong behavior the bug introduces.
The trigger that emits FETCH_OBJ_FUNC_ARG is preserved: an unqualified namespaced-fallback call (@file_get_contents($this->path)) from inside namespace Test, with @ keeping the opcode shape intact. As you observed, a single call reproduces the bug.
Two variants are covered:

Container — self-primes the SIMPLE_GET bit on the first (VM) call and consumes it once JIT-compiled.
Container2 — a preceding plain FETCH_OBJ_R primes SIMPLE_GET on the shared cache slot (compact_literals merges the slots at compile time, as you correctly noted — the loop does not cause the merge, it only repeats the bad read), and the following FETCH_OBJ_FUNC_ARG consumes it. The flag is reset after the priming read so the assertion reflects only the FUNC_ARG read.
Because the assertion is now deterministic, the loop only needs a few calls (2–3, matching what you saw locally) instead of 200 — there's no flakiness left to paper over.

Verified locally: with the fix the test prints OK; forcing the getter flag off (simulating the buggy JIT by dropping the hook-enter guard) makes it deterministically FAIL with the RuntimeException.

Let me know if you'd prefer the loop dropped to a single call or any other tweak.

[arnaud-lb]

Unfortunately we can't exit to VM in the function JIT as the VM stack may not be up to date as some vars may be kept only in registers. It's only safe for FETCH_OBJ_R because this path is only reachable in the minimal JIT which doesn't use reg alloc.

The following test demonstrates that:

--TEST--
GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook (SIMPLE_GET fast path)
--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--
<?php

class C {
    public $prop {
        get { return 1; }
    }
}

function f(int $a0, $obj) {
    // $a is in reg: all uses in supported opcodes, in non-entry BBs
    $a = $a0;
    $b = $a + 2;
    // $obj->prop uses FETCH_FUNC_ARG
    // $a is used after returning to VM
    $c = g($obj->prop, $a ? 1 : 2);
    return $b + $c;
}

if (getenv('A')) {
    // Function is not known at compile time
    function g($a, $b) {
        var_dump($b);
        return $a;
    }
}

$c = new C();
var_dump(f(1, $c)); // Set SIMPLE_GET and inline caches
var_dump(f(1, $c));

?>
--EXPECT--
int(1)
int(4)
int(1)
int(4)

Output:

========DIFF========
     int(1)
     int(4)
003- int(1)
003+ 
004+ Warning: Undefined variable $a in test.php on line 15
005+ int(2)
     int(4)

What I suggested previously should work.

[zhaohao19941221]

Unfortunately we can't exit to VM in the function JIT as the VM stack may not be up to date as some vars may be kept only in registers. It's only safe for FETCH_OBJ_R because this path is only reachable in the minimal JIT which doesn't use reg alloc.

The following test demonstrates that:

--TEST--
GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook (SIMPLE_GET fast path)
--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--
<?php

class C {
    public $prop {
        get { return 1; }
    }
}

function f(int $a0, $obj) {
    // $a is in reg: all uses in supported opcodes, in non-entry BBs
    $a = $a0;
    $b = $a + 2;
    // $obj->prop uses FETCH_FUNC_ARG
    // $a is used after returning to VM
    $c = g($obj->prop, $a ? 1 : 2);
    return $b + $c;
}

if (getenv('A')) {
    // Function is not known at compile time
    function g($a, $b) {
        var_dump($b);
        return $a;
    }
}

$c = new C();
var_dump(f(1, $c)); // Set SIMPLE_GET and inline caches
var_dump(f(1, $c));

?>
--EXPECT--
int(1)
int(4)
int(1)
int(4)

Output:

========DIFF========
     int(1)
     int(4)
003- int(1)
003+ 
004+ Warning: Undefined variable $a in test.php on line 15
005+ int(2)
     int(4)

What I suggested previously should work.

@arnaud-lb You're right — I reproduced your counter-example locally against my branch and got the exact same diff (Undefined variable $a). The hook-enter guard is only sound when nothing lives solely in registers, which holds for the minimal-JIT-only FETCH_OBJ_R path but not for FETCH_OBJ_FUNC_ARG at INLINE level. I'll rework this along the lines you suggested: handle FETCH_OBJ_FUNC_ARG in the INLINE switch with a runtime SEND_ARG_BY_REF check — by-value dispatching to zend_jit_fetch_obj() (getter runs inside the helper, registers stay live), by-ref taking a cold-path handler — keeping the generic-switch guard only for opt levels below INLINE where there's no reg alloc. I'll also add your test as gh22857_002.phpt. Thanks for the counter-example.

[LamentXU123]

Okay. Looks good now code wise, but I am not an expert on this so let's wait for other's reviews.

[zhaohao19941221]

---

PHP : C:\obj\Release_TS\phpdbg.exe
PHP_SAPI : phpdbg
PHP_VERSION : 8.6.0-dev
ZEND_VERSION: 4.6.0-dev
PHP_OS : WINNT - Windows NT runnervmr7g38 10.0 build 26100 (Windows Server 2025) AMD64
INI actual : C:\obj\Release_TS\php.ini
More .INIs :

CWD : D:\a\php-src\php-src
Extra dirs :
VALGRIND : Not used

TIME START 2026-07-29 10:00:12

Spawning 2 workers... Done in 0.44s

ERROR: Worker 1 reported unexpected E_WARNING: file_put_contents(): Failed to open stream: Permission denied in D:\a\php-src\php-src\run-tests.php on line 1138
NMAKE : fatal error U1077: ' "C:\obj\Release_TS\php.exe" -d open_basedir= -d output_buffering=0 run-tests.php -d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.protect_memory=1 -d opcache.jit_buffer_size=64M -d opcache.jit=tracing -g FAIL,BORK,LEAK,XLEAK --no-progress -q --offline --show-diff --show-slow 1000 --set-timeout 120 --temp-source c:\tests_tmp --temp-target c:\tests_tmp -j2 -p "C:\obj\Release_TS\php.exe"' : return code '0x1'
Stop.

Microsoft (R) Program Maintenance Utility Version 14.51.36248.0
Copyright (C) Microsoft Corporation. All rights reserved.

SUCCESS: The process "snmpd.exe" with PID 6800 has been terminated.

D:\a\php-src\php-src>if 2 NEQ 0 exit /b 3
Error: Process completed with exit code 1.

[arnaud-lb]

Looks good to me otherwise

[arnaud-lb]

This looks good, but please extract this part to a new function in zend_jit_ir.c so that we keep most IR-related code in that file.

[arnaud-lb]

NB: Please also rebase on PHP-8.4 and then change the base of the PR

[zhaohao19941221]

NB: Please also rebase on PHP-8.4 and then change the base of the PR

@arnaud-lb Done — all points addressed in the latest force-push (rebased onto PHP-8.4):

1. Extract IR block to zend_jit_ir.c — the by-value/by-ref dispatch logic is now in a new zend_jit_fetch_obj_func_arg() helper in ext/opcache/jit/zend_jit_ir.c, so the IR code stays in that file. zend_jit.c only calls it from the (now merged) ZEND_FETCH_OBJ_FUNC_ARG case.
2. Merge the case block — ZEND_FETCH_OBJ_FUNC_ARG now shares the setup with ZEND_FETCH_OBJ_R/IS/W, deduping the op1/ce resolution.
3. Rebase on PHP-8.4 — done; PR base is now PHP-8.4, single clean commit on top of 2804d09.
   Your counter-example is included as gh22857_002.phpt and passes. Could you take another look? Thanks!

[arnaud-lb]

Thank you @zhaohao19941221!