Fix GH-19653: Closure named argument unpacking between temporary closures can cause a crash

Due to closures, the `fbc` address isn't unique if the memory address is reused.
So for user closures we need to distinguish using a unique key,
while internal functions can't disappear and therefore can use the `fbc` address
as unique key.

As for performance:
This adds a load+compare for the function type, although we already load
and compare the function type down below, so that impact on performance
should be small. It also adds a load on the opcodes array, but we access
fields from the same cacheline too so that should also have only a small
impact.

Author: nielsdos

Zend/tests/closures/gh19653.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-19653 (Closure named argument unpacking between temporary closures can cause a crash)
+--CREDITS--
+ivan-u7n
+--FILE--
+<?php
+
+
+function func1(string $a1 = 'a1', string $a2 = 'a2', string $a3 = 'a3') {
+	echo __FUNCTION__ . "() a1=$a1 a2=$a2 a3=$a3\n";
+}
+
+function usage1(?Closure $func1 = null) {
+	echo __FUNCTION__ . "() ";
+	($func1 ?? func1(...))(a3: 'm3+');
+}
+usage1();
+
+$func1 = function (string ...$args) {
+	echo "[function] ";
+	func1(...$args, a2: 'm2+');
+};
+usage1(func1: $func1);
+
+?>
+--EXPECT--
+usage1() func1() a1=a1 a2=a2 a3=m3+
+usage1() [function] func1() a1=a1 a2=m2+ a3=m3+

Zend/zend_execute.c

@@ -5058,7 +5058,12 @@ static zend_never_inline zend_result ZEND_FASTCALL zend_quick_check_constant(
 
 static zend_always_inline uint32_t zend_get_arg_offset_by_name(
 		zend_function *fbc, zend_string *arg_name, void **cache_slot) {
-	if (EXPECTED(*cache_slot == fbc)) {
+	/* Due to closures, the `fbc` address isn't unique if the memory address is reused.
+	 * So for user closures we need to distinguish using a unique key, while internal functions can't disappear
+	 * and therefore can use the `fbc` address as unique key. */
+	void *unique_id = EXPECTED(fbc->type == ZEND_USER_FUNCTION) ? (void *) fbc->op_array.opcodes : (void *) fbc;
+
+	if (EXPECTED(*cache_slot == unique_id)) {
 		return *(uintptr_t *)(cache_slot + 1);
 	}
 
@@ -5069,7 +5074,7 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
 		for (uint32_t i = 0; i < num_args; i++) {
 			zend_arg_info *arg_info = &fbc->op_array.arg_info[i];
 			if (zend_string_equals(arg_name, arg_info->name)) {
-				*cache_slot = fbc;
+				*cache_slot = unique_id;
 				*(uintptr_t *)(cache_slot + 1) = i;
 				return i;
 			}
@@ -5079,15 +5084,15 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
 			zend_internal_arg_info *arg_info = &fbc->internal_function.arg_info[i];
 			size_t len = strlen(arg_info->name);
 			if (zend_string_equals_cstr(arg_name, arg_info->name, len)) {
-				*cache_slot = fbc;
+				*cache_slot = unique_id;
 				*(uintptr_t *)(cache_slot + 1) = i;
 				return i;
 			}
 		}
 	}
 
 	if (fbc->common.fn_flags & ZEND_ACC_VARIADIC) {
-		*cache_slot = fbc;
+		*cache_slot = unique_id;
 		*(uintptr_t *)(cache_slot + 1) = fbc->common.num_args;
 		return fbc->common.num_args;
 	}