[GIT-PULLS] [php-src] PR #22897: Fix GH-22857: JIT wrong code for FETCH_OBJ_FUNC_ARG with property hooks

[email protected] (zhaohao19941221) Mon, 27 Jul 2026 07:07:01 +0000
Newsgroups php.git-pulls
Message-ID <[email protected]>
Pull Request: https://github.com/php/php-src/pull/22897
Author: 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)

```console
$ 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 GH-22857
- Mirrors the JIT-only shape used by GH-21369 (which fixed the analogous GH-21006 for tracing JIT).
- #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.