[php-src] master: Merge branch 'PHP-8.5'
Arnaud Le Blanc <[email protected]> Thu, 30 Jul 2026 07:49:08 +0000
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Arnaud Le Blanc (arnaud-lb)
Date: 2026-07-30T09:48:21+02:00
Commit: https://github.com/php/php-src/commit/0b5d9801ec3b53e84388239a5b9f85d005318b64
Raw diff: https://github.com/php/php-src/commit/0b5d9801ec3b53e84388239a5b9f85d005318b64.diff
Merge branch 'PHP-8.5'
* PHP-8.5:
CS
Fix GH-22857: Function JIT emits wrong code for FETCH_OBJ_FUNC_ARG on a property hook (#22897)
Changed paths:
A ext/opcache/tests/jit/gh22857.phpt
A ext/opcache/tests/jit/gh22857_002.phpt
M ext/opcache/jit/zend_jit.c
M ext/opcache/jit/zend_jit_ir.c
Diff:
diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c
index 6533f00ede6e..e91da6aeb8d3 100644
--- a/ext/opcache/jit/zend_jit.c
+++ b/ext/opcache/jit/zend_jit.c
@@ -2447,6 +2447,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
goto jit_failure;
}
goto done;
+ case ZEND_FETCH_OBJ_FUNC_ARG:
case ZEND_FETCH_OBJ_R:
case ZEND_FETCH_OBJ_IS:
case ZEND_FETCH_OBJ_W:
@@ -2484,11 +2485,31 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
|| Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
break;
}
- if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
- op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
- RES_REG_ADDR(), IS_UNKNOWN,
- zend_may_throw(opline, ssa_op, op_array, ssa))) {
- goto jit_failure;
+ if (opline->opcode == ZEND_FETCH_OBJ_FUNC_ARG) {
+ /* FETCH_OBJ_FUNC_ARG's by-value fetch dispatches into the
+ * FETCH_OBJ_R handler, which may take the SIMPLE_GET hook fast
+ * path and push a getter frame; by-ref dispatches into
+ * FETCH_OBJ_W. The function JIT may keep values solely in
+ * registers, so we must NOT exit to the VM (stale stack slots).
+ * Inline the by-value path through zend_jit_fetch_obj, which runs
+ * the hook getter inside a helper and keeps all registers live.
+ * The by-ref path has no SIMPLE_GET fast path, so the generic
+ * handler (a full C call, safe under register allocation) is used.
+ * This mirrors the tracing JIT fix for GH-21006 (GH-21369); the
+ * runtime by-ref check is required because the passing mode is
+ * only known once the callee is resolved via namespace fallback.
+ * See GH-22857. */
+ if (!zend_jit_fetch_obj_func_arg(jit, opline, op_array, ssa, ssa_op,
+ op1_info, op1_addr, ce, ce_is_instanceof, on_this, RES_REG_ADDR())) {
+ goto jit_failure;
+ }
+ } else {
+ if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
+ op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
+ RES_REG_ADDR(), IS_UNKNOWN,
+ zend_may_throw(opline, ssa_op, op_array, ssa))) {
+ goto jit_failure;
+ }
}
goto done;
case ZEND_FETCH_STATIC_PROP_R:
diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index 8f6804cb8915..2c904edd4cfc 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -14787,6 +14787,59 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit,
return 1;
}
+static int zend_jit_fetch_obj_func_arg(zend_jit_ctx *jit, const zend_op *opline,
+ const zend_op_array *op_array, zend_ssa *ssa, const zend_ssa_op *ssa_op,
+ uint32_t op1_info, zend_jit_addr op1_addr, zend_class_entry *ce,
+ bool ce_is_instanceof, bool on_this, zend_jit_addr res_addr)
+{
+ ir_ref rx, call_info, if_by_ref, end_by_ref;
+
+ /* Both runtime paths must observe a consistent frame state. The delayed
+ * call chain would otherwise only be flushed inside the by-ref branch (by
+ * zend_jit_set_ip() in zend_jit_handler()), leaving EX(call) stale on the
+ * by-val path and after the merge. Flush it before branching. */
+ if (jit->delayed_call_level) {
+ if (!zend_jit_save_call_chain(jit, jit->delayed_call_level)) {
+ return 0;
+ }
+ }
+
+ /* JIT: if (ZEND_CALL_INFO(EX(call)) & ZEND_CALL_SEND_ARG_BY_REF) */
+ if (jit->reuse_ip) {
+ rx = jit_IP(jit);
+ } else {
+ rx = ir_LOAD_A(jit_EX(call));
+ }
+ call_info = ir_LOAD_U32(jit_CALL(rx, This.u1.type_info));
+ if_by_ref = ir_IF(ir_AND_U32(call_info, ir_CONST_U32(ZEND_CALL_SEND_ARG_BY_REF)));
+
+ /* by-ref path: the FUNC_ARG handler re-checks the flag and dispatches
+ * into FETCH_OBJ_W */
+ ir_IF_TRUE_cold(if_by_ref);
+ if (!zend_jit_handler(jit, opline, zend_may_throw(opline, ssa_op, op_array, ssa))) {
+ return 0;
+ }
+ end_by_ref = ir_END();
+
+ /* zend_jit_handler() stored IP = opline + 1 on the by-ref path only;
+ * that compile-time knowledge is invalid for the by-val path and after
+ * the merge. */
+ zend_jit_reset_last_valid_opline(jit);
+
+ /* by-val path */
+ ir_IF_FALSE(if_by_ref);
+ if (!zend_jit_fetch_obj(jit, opline, op_array, ssa, ssa_op,
+ op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
+ res_addr, IS_UNKNOWN,
+ zend_may_throw(opline, ssa_op, op_array, ssa))) {
+ return 0;
+ }
+ ir_MERGE_WITH(end_by_ref);
+
+ return 1;
+}
+
+
static int zend_jit_assign_obj(zend_jit_ctx *jit,
const zend_op *opline,
const zend_op_array *op_array,
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--
+<?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
diff --git a/ext/opcache/tests/jit/gh22857_002.phpt b/ext/opcache/tests/jit/gh22857_002.phpt
new file mode 100644
index 000000000000..c14ad309d96f
--- /dev/null
+++ b/ext/opcache/tests/jit/gh22857_002.phpt
@@ -0,0 +1,47 @@
+--TEST--
+GH-22857 (reg-alloc): FETCH_OBJ_FUNC_ARG hook read must not lose register-held vars
+--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 lives only in a register: every use is in a supported opcode and in a
+ // non-entry basic block, so the register allocator never spills it to the
+ // VM stack. Exiting to the VM (the buggy hook-enter guard) would read a
+ // stale stack slot for $a.
+ $a = $a0;
+ $b = $a + 2;
+ $c = g($obj->prop, $a ? 1 : 2);
+ return $b + $c;
+}
+
+if (getenv('A')) {
+ function g($a, $b) {
+ var_dump($b);
+ return $a;
+ }
+}
+
+$c = new C();
+var_dump(f(1, $c));
+var_dump(f(1, $c));
+
+?>
+--EXPECT--
+int(1)
+int(4)
+int(1)
+int(4)