Support named params in call_user_func_array

nikic · nikic · commit dbbf0c8316d1 · 2020-07-06T15:13:08.000+02:00
diff --git a/Zend/tests/named_params/call_user_func.phpt b/Zend/tests/named_params/call_user_func.phpt
@@ -9,11 +9,15 @@ $test = function($a = 'a', $b = 'b', $c = 'c') {
 $test_variadic = function(...$args) {
     var_dump($args);
 };
+$test_ref = function(&$ref) {
+    $ref++;
+};
 
 call_user_func($test, 'A', c: 'C');
 call_user_func($test, c: 'C', a: 'A');
 call_user_func($test, c: 'C');
 call_user_func($test_variadic, 'A', c: 'C');
+call_user_func($test_ref, ref: null);
 var_dump(call_user_func('call_user_func', $test, c: 'D'));
 var_dump(call_user_func('array_slice', [1, 2, 3, 4, 5], length: 2));
 echo "\n";
@@ -24,7 +28,7 @@ $test->call(new class {}, 'A', c: 'C');
 $test_variadic->call(new class {}, 'A', c: 'C');
 
 ?>
---EXPECT--
+--EXPECTF--
 a = A, b = b, c = C
 a = A, b = b, c = C
 a = a, b = b, c = C
@@ -34,6 +38,8 @@ array(2) {
   ["c"]=>
   string(1) "C"
 }
+
+Warning: {closure}(): Argument #1 ($ref) must be passed by reference, value given in %s on line %d
 a = a, b = b, c = D
 NULL
 array(2) {
diff --git a/Zend/tests/named_params/call_user_func_array.phpt b/Zend/tests/named_params/call_user_func_array.phpt
@@ -3,25 +3,70 @@ Behavior of call_user_func_array() with named parameters
 --FILE--
 <?php
 
-function test($a, $b) {
-    var_dump($a, $b);
-}
+namespace {
+    $test = function($a = 'a', $b = 'b', $c = 'c') {
+        echo "a = $a, b = $b, c = $c\n";
+    };
+    $test_variadic = function(...$args) {
+        var_dump($args);
+    };
 
-// Keys are ignored by call_user_func_array().
-// Use "..." if you want named params support!
+    call_user_func_array($test, ['A', 'B']);
+    call_user_func_array($test, [1 => 'A', 0 => 'B']);
+    call_user_func_array($test, ['A', 'c' => 'C']);
+    call_user_func_array($test_variadic, ['A', 'c' => 'C']);
+    try {
+        call_user_func_array($test, ['d' => 'D']);
+    } catch (\Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+    try {
+        call_user_func_array($test, ['c' => 'C', 'A']);
+    } catch (\Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+    echo "\n";
+}
 
-call_user_func_array('test', [0 => 'a', 1 => 'b']);
-call_user_func_array('test', [1 => 'a', 0 => 'b']);
-call_user_func_array('test', ['b' => 'a', 'a' => 'b']);
-call_user_func_array('test', ['c' => 'a', 'd' => 'b']);
+namespace Foo {
+    call_user_func_array($test, ['A', 'B']);
+    call_user_func_array($test, [1 => 'A', 0 => 'B']);
+    call_user_func_array($test, ['A', 'c' => 'C']);
+    call_user_func_array($test_variadic, ['A', 'c' => 'C']);
+    try {
+        call_user_func_array($test, ['d' => 'D']);
+    } catch (\Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+    try {
+        call_user_func_array($test, ['c' => 'C', 'A']);
+    } catch (\Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
 
 ?>
 --EXPECT--
-string(1) "a"
-string(1) "b"
-string(1) "a"
-string(1) "b"
-string(1) "a"
-string(1) "b"
-string(1) "a"
-string(1) "b"
+a = A, b = B, c = c
+a = A, b = B, c = c
+a = A, b = b, c = C
+array(2) {
+  [0]=>
+  string(1) "A"
+  ["c"]=>
+  string(1) "C"
+}
+Unknown named parameter $d
+Cannot use positional argument after named argument
+
+a = A, b = B, c = c
+a = A, b = B, c = c
+a = A, b = b, c = C
+array(2) {
+  [0]=>
+  string(1) "A"
+  ["c"]=>
+  string(1) "C"
+}
+Unknown named parameter $d
+Cannot use positional argument after named argument
diff --git a/Zend/zend_API.h b/Zend/zend_API.h
@@ -48,6 +48,10 @@ typedef struct _zend_fcall_info {
 	zend_object *object;
 	zend_bool no_separation;
 	uint32_t param_count;
+	/* This hashtable can also contain positional arguments (with integer keys),
+	 * which will be appended to the normal params[]. This makes it easier to
+	 * integrate APIs like call_user_func_array(). The usual restriction that
+	 * there may not be position arguments after named arguments applies. */
 	HashTable *named_params;
 } zend_fcall_info;
 
diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
@@ -753,6 +753,7 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) /
 					zend_param_must_be_ref(func, i + 1);
 					if (UNEXPECTED(EG(exception))) {
 						ZEND_CALL_NUM_ARGS(call) = i;
+cleanup_args:
 						zend_vm_stack_free_args(call);
 						zend_vm_stack_free_call_frame(call);
 						if (EG(current_execute_data) == &dummy_execute_data) {
@@ -774,27 +775,57 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) /
 	}
 
 	if (fci->named_params) {
+		ZEND_ASSERT(fci->no_separation && "Do we want to support separation?");
+
 		zend_string *name;
-		zval *val;
-		ZEND_HASH_FOREACH_STR_KEY_VAL(fci->named_params, name, val) {
-			ZEND_ASSERT(name && "named_params may only contain named params");
-
-			void *cache_slot = NULL;
-			uint32_t arg_num;
-			zval *target = zend_handle_named_arg(&call, name, &arg_num, &cache_slot);
-			if (!target) {
-				zend_vm_stack_free_args(call);
-				zend_vm_stack_free_call_frame(call);
-				if (EG(current_execute_data) == &dummy_execute_data) {
-					EG(current_execute_data) = dummy_execute_data.prev_execute_data;
+		zval *arg;
+		uint32_t arg_num = ZEND_CALL_NUM_ARGS(call) + 1;
+		zend_bool have_named_params = 0;
+		ZEND_HASH_FOREACH_STR_KEY_VAL(fci->named_params, name, arg) {
+			zval *target;
+			if (name) {
+				void *cache_slot = NULL;
+				have_named_params = 1;
+				target = zend_handle_named_arg(&call, name, &arg_num, &cache_slot);
+				if (!target) {
+					goto cleanup_args;
 				}
-				return FAILURE;
+			} else {
+				if (have_named_params) {
+					zend_throw_error(NULL,
+						"Cannot use positional argument after named argument");
+					goto cleanup_args;
+				}
+
+				target = ZEND_CALL_ARG(call, arg_num);
 			}
 
-			/* TODO */
-			/*if (ARG_SHOULD_BE_SENT_BY_REF(func, arg_num)) {
-			}*/
-			ZVAL_COPY(target, val);
+			if (ARG_SHOULD_BE_SENT_BY_REF(func, arg_num)) {
+				if (UNEXPECTED(!Z_ISREF_P(arg))) {
+					if (!fci->no_separation) {
+						/* TODO ? */
+					} else if (!ARG_MAY_BE_SENT_BY_REF(func, arg_num)) {
+						/* By-value send is not allowed -- emit a warning,
+						 * but still perform the call with a by-value send. */
+						zend_param_must_be_ref(func, arg_num);
+						if (UNEXPECTED(EG(exception))) {
+							goto cleanup_args;
+						}
+					}
+				}
+			} else {
+				if (Z_ISREF_P(arg) &&
+					!(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
+					/* don't separate references for __call */
+					arg = Z_REFVAL_P(arg);
+				}
+			}
+
+			ZVAL_COPY(target, arg);
+			if (!name) {
+				ZEND_CALL_NUM_ARGS(call)++;
+				arg_num++;
+			}
 		} ZEND_HASH_FOREACH_END();
 	}
 
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
@@ -5160,10 +5160,11 @@ ZEND_VM_HANDLER(119, ZEND_SEND_ARRAY, ANY, ANY, NUM)
 		HashTable *ht;
 		zval *arg, *param;
 
-
 ZEND_VM_C_LABEL(send_array):
 		ht = Z_ARRVAL_P(args);
 		if (OP2_TYPE != IS_UNUSED) {
+			/* We don't need to handle named params in this case,
+			 * because array_slice() is called with $preserve_keys == false. */
 			zval *op2 = GET_OP2_ZVAL_PTR(BP_VAR_R);
 			uint32_t skip = opline->extended_value;
 			uint32_t count = zend_hash_num_elements(ht);
@@ -5208,10 +5209,28 @@ ZEND_VM_C_LABEL(send_array):
 			}
 			FREE_OP2();
 		} else {
+			zend_string *name;
+			zend_bool have_named_params;
 			zend_vm_stack_extend_call_frame(&EX(call), 0, zend_hash_num_elements(ht));
 			arg_num = 1;
 			param = ZEND_CALL_ARG(EX(call), 1);
-			ZEND_HASH_FOREACH_VAL(ht, arg) {
+			have_named_params = 0;
+			ZEND_HASH_FOREACH_STR_KEY_VAL(ht, name, arg) {
+				if (name) {
+					void *cache_slot = NULL;
+					have_named_params = 1;
+					param = zend_handle_named_arg(&EX(call), name, &arg_num, &cache_slot);
+					if (!param) {
+						FREE_OP1();
+						HANDLE_EXCEPTION();
+					}
+				} else if (have_named_params) {
+					zend_throw_error(NULL,
+						"Cannot use positional argument after named argument");
+					FREE_OP1();
+					HANDLE_EXCEPTION();
+				}
+
 				if (ARG_SHOULD_BE_SENT_BY_REF(EX(call)->func, arg_num)) {
 					if (UNEXPECTED(!Z_ISREF_P(arg))) {
 						if (!ARG_MAY_BE_SENT_BY_REF(EX(call)->func, arg_num)) {
@@ -5227,10 +5246,13 @@ ZEND_VM_C_LABEL(send_array):
 						arg = Z_REFVAL_P(arg);
 					}
 				}
+
 				ZVAL_COPY(param, arg);
-				ZEND_CALL_NUM_ARGS(EX(call))++;
-				arg_num++;
-				param++;
+				if (!name) {
+					ZEND_CALL_NUM_ARGS(EX(call))++;
+					arg_num++;
+					param++;
+				}
 			} ZEND_HASH_FOREACH_END();
 		}
 	}
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
@@ -2105,10 +2105,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_SEND_ARRAY_SPEC_HANDLER(ZEND_O
 		HashTable *ht;
 		zval *arg, *param;
 
-
 send_array:
 		ht = Z_ARRVAL_P(args);
 		if (opline->op2_type != IS_UNUSED) {
+			/* We don't need to handle named params in this case,
+			 * because array_slice() is called with $preserve_keys == false. */
 			zval *op2 = get_zval_ptr(opline->op2_type, opline->op2, BP_VAR_R);
 			uint32_t skip = opline->extended_value;
 			uint32_t count = zend_hash_num_elements(ht);
@@ -2153,10 +2154,28 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_SEND_ARRAY_SPEC_HANDLER(ZEND_O
 			}
 			FREE_OP(opline->op2_type, opline->op2.var);
 		} else {
+			zend_string *name;
+			zend_bool have_named_params;
 			zend_vm_stack_extend_call_frame(&EX(call), 0, zend_hash_num_elements(ht));
 			arg_num = 1;
 			param = ZEND_CALL_ARG(EX(call), 1);
-			ZEND_HASH_FOREACH_VAL(ht, arg) {
+			have_named_params = 0;
+			ZEND_HASH_FOREACH_STR_KEY_VAL(ht, name, arg) {
+				if (name) {
+					void *cache_slot = NULL;
+					have_named_params = 1;
+					param = zend_handle_named_arg(&EX(call), name, &arg_num, &cache_slot);
+					if (!param) {
+						FREE_OP(opline->op1_type, opline->op1.var);
+						HANDLE_EXCEPTION();
+					}
+				} else if (have_named_params) {
+					zend_throw_error(NULL,
+						"Cannot use positional argument after named argument");
+					FREE_OP(opline->op1_type, opline->op1.var);
+					HANDLE_EXCEPTION();
+				}
+
 				if (ARG_SHOULD_BE_SENT_BY_REF(EX(call)->func, arg_num)) {
 					if (UNEXPECTED(!Z_ISREF_P(arg))) {
 						if (!ARG_MAY_BE_SENT_BY_REF(EX(call)->func, arg_num)) {
@@ -2172,10 +2191,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_SEND_ARRAY_SPEC_HANDLER(ZEND_O
 						arg = Z_REFVAL_P(arg);
 					}
 				}
+
 				ZVAL_COPY(param, arg);
-				ZEND_CALL_NUM_ARGS(EX(call))++;
-				arg_num++;
-				param++;
+				if (!name) {
+					ZEND_CALL_NUM_ARGS(EX(call))++;
+					arg_num++;
+					param++;
+				}
 			} ZEND_HASH_FOREACH_END();
 		}
 	}
diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
@@ -1589,16 +1589,17 @@ PHP_FUNCTION(call_user_func)
    Warning: This function is special-cased by zend_compile.c and so is usually bypassed */
 PHP_FUNCTION(call_user_func_array)
 {
-	zval *params, retval;
+	zval retval;
+	HashTable *params;
 	zend_fcall_info fci;
 	zend_fcall_info_cache fci_cache;
 
 	ZEND_PARSE_PARAMETERS_START(2, 2)
 		Z_PARAM_FUNC(fci, fci_cache)
-		Z_PARAM_ARRAY(params)
+		Z_PARAM_ARRAY_HT(params)
 	ZEND_PARSE_PARAMETERS_END();
 
-	zend_fcall_info_args(&fci, params);
+	fci.named_params = params;
 	fci.retval = &retval;
 
 	if (zend_call_function(&fci, &fci_cache) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
@@ -1607,8 +1608,6 @@ PHP_FUNCTION(call_user_func_array)
 		}
 		ZVAL_COPY_VALUE(return_value, &retval);
 	}
-
-	zend_fcall_info_args_clear(&fci, 1);
 }
 /* }}} */
 
