Fix GH-22857: virtual property hook mis-primes SIMPLE_GET under FUNC_ARG

[zhaohao19941221]

Summary

Fixes GH-22857: heap corruption / spurious ValueErrors / TypeErrors when a virtual property hook is read via FETCH_OBJ_FUNC_ARG under function-mode JIT (opcache.jit=1205).

Root cause

zend_std_read_property() primed the SIMPLE_GET property-hook cache-slot bit unconditionally after every successful hook invocation, ignoring which opcode had triggered the read.

ZEND_FETCH_OBJ_FUNC_ARG dispatches into the ZEND_FETCH_OBJ_R handler for by-value arguments via ZEND_VM_TAIL_CALL, keeping EX(opline) on the FUNC_ARG opcode. When the cache-slot bit was cached under such a mismatched opline, the next read went through the SIMPLE_GET fast path in zend_vm_def.h, which pushes a hook call frame and returns opline | ZEND_VM_ENTER_BIT to signal the caller to re-enter the VM.

Function-mode JIT handles this only for FETCH_OBJ_R (via the specialised hook-enter guard in zend_jit.c) — for FETCH_OBJ_FUNC_ARG the generic zend_jit_handler path is used, which has no such guard. The JIT-compiled SEND_FUNC_ARG therefore reads from the argument slot before the hook has actually run.

Depending on the adjacent property slot's contents, symptoms are:

• ValueError: file_get_contents(): Argument #1 ($filename) must not contain any null bytes
• TypeError: <fn>(): Argument #N ($arg) must be of type ?string, <adjacent-class> given
• zend_mm_heap corrupted (SIGABRT)

All three have the same root cause.

Fix

Only prime SIMPLE_GET when the currently-executing opline is a plain ZEND_FETCH_OBJ_R. This mirrors the guard already used for SIMPLE_READ a few lines above in the same function.

Reproducer (before the fix)

$ php -d opcache.enable_cli=1 -d opcache.jit_buffer_size=64M \
      -d opcache.jit=1205 Zend/tests/property_hooks/virtual_hook_as_func_arg.phpt

was 20/20 crashing; with -d opcache.jit=disable or -d opcache.jit=tracing was 20/20 clean. After the fix, both modes are clean.

The reproducer needs all of:

1. opcache.jit=1205 (function JIT).
2. Class inside a namespace.
3. Call is unqualified (no leading \, no use function) so that INIT_NS_FCALL_BY_NAME + FETCH_OBJ_FUNC_ARG is emitted.
4. Argument is a virtual property hook (get-only, no backing storage).
5. Hook passed directly — assigning to a local first uses FETCH_OBJ_R and hides the bug.
6. Call wrapped in @ (BEGIN_SILENCE / END_SILENCE).
7. Class layout with adjacent asymmetric-visibility fields and two promoted asymmetric-visibility constructor parameters.
8. Hook get => calls a static method reading the promoted asymmetric properties.

Delta-debugging established that removing any one of these hides the bug.

Test

Zend/tests/property_hooks/virtual_hook_as_func_arg.phpt — 200 iterations exercising the exact eight-condition shape from the issue. Fails before the fix (20/20 under jit=1205), passes after.

Backport

Bug is present since 8.4 (property hooks introduced in 8.4). Targeting PHP-8.4 so it lands in 8.4/8.5/master via the normal merge chain.

[arnaud-lb]

The issue is similar to GH-21006 (but for the function JIT) which was fixed by a JIT-only change in GH-21369: ZEND_FETCH_OBJ_R initiates a call, pushes a VM call frame, but the JIT code doesn't expect it.

I believe that this could be fixed in JIT only as well: We could compile ZEND_FETCH_OBJ_FUNC_ARG to something like this:

• if EX(call) & ZEND_CALL_SEND_ARG_BY_REF, call the handler (cold path)
• else use zend_jit_fetch_obj(), which should handle hooks properly (happy path)

wdyt @zhaohao19941221 @iliaal?

[iliaal]

Agree, this should be JIT-only. The interpreter is already correct: for a by-value arg, FETCH_OBJ_FUNC_ARG dispatches to the FETCH_OBJ_R handler, takes the SIMPLE_GET frame-entry with call->return_value pointing at the arg slot, and returns fine, which is why jit=disable and tracing are unaffected. The only breakage is that function JIT compiles FETCH_OBJ_FUNC_ARG through the generic handler, which lacks the hook-enter IP guard the FETCH_OBJ_R generic case already has in zend_jit.c, so once the getter frame is pushed it falls through into the compiled SEND_FUNC_ARG.

The interpreter patch here also doesn't fully close it. compact_literals shares the property cache slot between FETCH_OBJ_R and FETCH_OBJ_FUNC_ARG for $this->prop, and it only stops FETCH_OBJ_FUNC_ARG from priming SIMPLE_GET, not from consuming a bit a sibling read primed. A plain read of the same property re-arms it:

public function step(): void {
    $warm = $this->path;              // FETCH_OBJ_R primes SIMPLE_GET on the shared slot
    @file_get_contents($this->path);  // FETCH_OBJ_FUNC_ARG consumes it
}

Built on this branch with jit=1205, the original repro is clean and the above SIGSEGVs; jit=disable and tracing are clean either way.

Your split is what the VM already does: CHECK_FUNC_ARG sets ZEND_CALL_SEND_ARG_BY_REF and the FETCH_OBJ_FUNC_ARG handler reads it to choose W vs R. Compiling the by-value case through zend_jit_fetch_obj and deopting by-ref is the same shape as #21369 for tracing. I'd go that way.