Skip to content
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,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
Expand Down
24 changes: 22 additions & 2 deletions ext/opcache/jit/zend_jit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down
125 changes: 125 additions & 0 deletions ext/opcache/tests/jit/gh22857.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
--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--
<?php
namespace Test;

interface HandlerInterface { public function noop(): void; }

final class DefaultHandler implements HandlerInterface {
private static ?self $i = null;
public static function getInstance(): self { return self::$i ??= new self(); }
public function noop(): void {}
}

/* 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::$getterRan = true)
? self::build($this->kind, $this->id)
: 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 (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');
}
}
}

$c = new Container('alpha', 'beta');
$c->step();
$c->step();
$c->step();

/* 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 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::$getterRan = true)
? self::build($this->kind, $this->id)
: 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 {
$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');
}
}
}

$c2 = new Container2('alpha', 'beta');
$c2->step();
$c2->step();
$c2->step();

echo "OK\n";
?>
--EXPECT--
OK