diff --git a/NEWS b/NEWS index 0c2fd04ad469..a7b2bbebeb53 100644 --- a/NEWS +++ b/NEWS @@ -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 diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index 6533f00ede6e..0214969e40f7 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..fde36d19b390 --- /dev/null +++ b/ext/opcache/tests/jit/gh22857.phpt @@ -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-- + (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