[pull] master from php:master

ext/odbc/tests/odbc_utils.phpt

@@ -17,11 +17,14 @@ $with_end_curly2 = "{foo}bar}";
 // 2. See $with_end_curly2; should_quote doesn't care about if the string is already quoted.
 $with_end_curly3 = "{foo}}bar}";
 // 1. No, it's not quoted.
-// 2. It doesn't need to be quoted because of no s
+// 2. It doesn't need to be quoted because of no special characters.
 $with_no_end_curly1 = "foobar";
 // 1. Yes, it is quoted and any characters are properly escaped.
 // 2. See $with_end_curly2.
 $with_no_end_curly2 = "{foobar}";
+// 1. No, it's not quoted.
+// 2. Yes, spaces should be quoted, as drivers can interpret them differently.
+$with_spaces = "  foobar  ";
 
 echo "# Is quoted?\n";
 echo "With end curly brace 1: ";
@@ -34,6 +37,8 @@ echo "Without end curly brace 1: ";
 var_dump(odbc_connection_string_is_quoted($with_no_end_curly1));
 echo "Without end curly brace 2: ";
 var_dump(odbc_connection_string_is_quoted($with_no_end_curly2));
+echo "With spaces: ";
+var_dump(odbc_connection_string_is_quoted($with_spaces));
 
 echo "# Should quote?\n";
 echo "With end curly brace 1: ";
@@ -46,6 +51,8 @@ echo "Without end curly brace 1: ";
 var_dump(odbc_connection_string_should_quote($with_no_end_curly1));
 echo "Without end curly brace 2: ";
 var_dump(odbc_connection_string_should_quote($with_no_end_curly2));
+echo "With spaces: ";
+var_dump(odbc_connection_string_should_quote($with_spaces));
 
 echo "# Quote?\n";
 echo "With end curly brace 1: ";
@@ -58,6 +65,8 @@ echo "Without end curly brace 1: ";
 var_dump(odbc_connection_string_quote($with_no_end_curly1));
 echo "Without end curly brace 2: ";
 var_dump(odbc_connection_string_quote($with_no_end_curly2));
+echo "With spaces: ";
+var_dump(odbc_connection_string_quote($with_spaces));
 
 ?>
 --EXPECTF--
@@ -67,15 +76,18 @@ With end curly brace 2: bool(false)
 With end curly brace 3: bool(true)
 Without end curly brace 1: bool(false)
 Without end curly brace 2: bool(true)
+With spaces: bool(false)
 # Should quote?
 With end curly brace 1: bool(true)
 With end curly brace 2: bool(true)
 With end curly brace 3: bool(true)
 Without end curly brace 1: bool(false)
 Without end curly brace 2: bool(true)
+With spaces: bool(true)
 # Quote?
 With end curly brace 1: string(10) "{foo}}bar}"
 With end curly brace 2: string(13) "{{foo}}bar}}}"
 With end curly brace 3: string(15) "{{foo}}}}bar}}}"
 Without end curly brace 1: string(8) "{foobar}"
 Without end curly brace 2: string(11) "{{foobar}}}"
+With spaces: string(12) "{  foobar  }"

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:

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,

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

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)

main/io/php_io_copy_linux.c

@@ -21,11 +21,12 @@
 
 #if !defined(HAVE_COPY_FILE_RANGE) && defined(__NR_copy_file_range)
 #define HAVE_COPY_FILE_RANGE 1
-static inline ssize_t copy_file_range(
+static inline ssize_t php_copy_file_range(
 		int fd_in, off_t *off_in, int fd_out, off_t *off_out, size_t len, unsigned int flags)
 {
 	return syscall(__NR_copy_file_range, fd_in, off_in, fd_out, off_out, len, flags);
 }
+#define copy_file_range php_copy_file_range
 #endif
 
 #ifdef HAVE_SENDFILE

main/php_odbc_utils.c

@@ -62,12 +62,17 @@ PHPAPI bool php_odbc_connstr_is_quoted(const char *str)
  * attribute values that contain the characters []{}(),;?*=!@ not enclosed
  * with braces should be avoided."
  *
+ * We also add spaces too, as driver connection string parameter parsing isn't
+ * standardized and can be quite sloppy; it can lead to significant whitespace
+ * not being parsed or confusing parsing of different sections. Note that this
+ * lacks security impact as we already quote the ';=' characters when parsing.
+ *
  * Note that it assumes that the string is *not* already quoted. You should
  * check beforehand.
  */
 PHPAPI bool php_odbc_connstr_should_quote(const char *str)
 {
-	return strpbrk(str, "[]{}(),;?*=!@") != NULL;
+	return strpbrk(str, "[]{}(),;?*=!@ ") != NULL;
 }
 
 /**