Fix array_map optimization with non-literal function or non-literal args

Zend/zend_compile.c

@@ -5301,11 +5301,64 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
 		return FAILURE;
 	}
 
+	/* Bail out if callback is not an FCC */
 	zend_ast *callback = args->child[0];
 	if (callback->kind != ZEND_AST_CALL && callback->kind != ZEND_AST_STATIC_CALL) {
 		return FAILURE;
 	}
 
+	zend_ast *args_ast = zend_ast_call_get_args(callback);
+	if (args_ast->kind != ZEND_AST_CALLABLE_CONVERT) {
+		return FAILURE;
+	}
+
+	/* PFAs with non-literal pre-bound arguments are not optimizable because we
+	 * can't memoize arguments without breaking pass-by-reference.
+	 * TODO: Support PFAs when the function is known to not receive by ref. */
+	zend_ast_fcc *fcc = (zend_ast_fcc*)args_ast;
+	zend_ast_list *fcc_args = zend_ast_get_list(fcc->args);
+	for (uint32_t i = 0; i < fcc_args->children; i++) {
+		zend_ast *arg = fcc_args->child[i];
+		if (arg->kind == ZEND_AST_NAMED_ARG) {
+			arg = arg->child[1];
+		}
+
+		if (arg->kind == ZEND_AST_PLACEHOLDER_ARG) {
+			continue;
+		}
+
+		if (arg->kind != ZEND_AST_ZVAL) {
+			return FAILURE;
+		}
+	}
+
+	/* Evaluate class name */
+	znode class_node;
+	if (callback->kind == ZEND_AST_STATIC_CALL) {
+		znode result;
+		zend_compile_expr(&result, callback->child[0]);
+		if (result.op_type == IS_CONST) {
+			class_node = result;
+		} else {
+			class_node = result;
+			zend_emit_op_tmp(&class_node, ZEND_QM_ASSIGN, &result, NULL);
+		}
+	} else {
+		class_node.op_type = IS_UNUSED;
+	}
+
+	/* Evaluate function name */
+	znode func_node;
+	{
+		znode result;
+		zend_compile_expr(&result, callback->child[callback->kind == ZEND_AST_CALL ? 0 : 1]);
+		if (result.op_type == IS_CONST) {
+			func_node = result;
+		} else {
+			zend_emit_op_tmp(&func_node, ZEND_QM_ASSIGN, &result, NULL);
+		}
+	}
+
 	znode value;
 	value.op_type = IS_TMP_VAR;
 	value.u.op.var = get_temporary_variable();
@@ -5314,6 +5367,12 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
 			zend_ast_create_znode(&value));
 	if (!call_args) {
 		CG(active_op_array)->T--;
+		if (func_node.op_type == IS_CONST) {
+			zval_ptr_dtor_nogc(&func_node.u.constant);
+		}
+		if (class_node.op_type == IS_CONST) {
+			zval_ptr_dtor_nogc(&class_node.u.constant);
+		}
 		/* The callback is not a FCC/PFA, or is not optimizable */
 		return FAILURE;
 	}
@@ -5353,14 +5412,35 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
 
 	/* loop body */
 	znode call_result;
+	zend_ast *func_ast;
+	if (func_node.op_type == IS_CONST) {
+		func_ast = zend_ast_create_znode(&func_node);
+	} else {
+		znode copy_node;
+		zend_emit_op_tmp(&copy_node, ZEND_COPY_TMP, &func_node, NULL);
+		func_ast = zend_ast_create_znode(&copy_node);
+	}
 	switch (callback->kind) {
-		case ZEND_AST_CALL:
-			zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_CALL, callback->child[0], call_args));
+		case ZEND_AST_CALL: {
+			zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_CALL, func_ast, call_args));
 			break;
-		case ZEND_AST_STATIC_CALL:
-			zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_STATIC_CALL, callback->child[0], callback->child[1], call_args));
+		}
+		case ZEND_AST_STATIC_CALL: {
+			zend_ast *class_ast;
+			if (class_node.op_type == IS_CONST) {
+				class_ast = zend_ast_create_znode(&class_node);
+			} else {
+				znode copy_node;
+				zend_emit_op_tmp(&copy_node, ZEND_COPY_TMP, &class_node, NULL);
+				class_ast = zend_ast_create_znode(&copy_node);
+			}
+
+			zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_STATIC_CALL, class_ast, func_ast, call_args));
+			zend_ast_destroy(class_ast);
 			break;
+		}
 	}
+	zend_ast_destroy(func_ast);
 	opline = zend_emit_op(NULL, ZEND_ADD_ARRAY_ELEMENT, &call_result, &key);
 	SET_NODE(opline->result, result);
 	/* end loop body */
@@ -5375,6 +5455,12 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
 
 	zend_end_loop(opnum_fetch, &reset_node);
 	zend_emit_op(NULL, ZEND_FE_FREE, &reset_node, NULL);
+	if (func_node.op_type != IS_CONST) {
+		zend_emit_op(NULL, ZEND_FREE, &func_node, NULL);
+	}
+	if (class_node.op_type != IS_UNUSED && class_node.op_type != IS_CONST) {
+		zend_emit_op(NULL, ZEND_FREE, &class_node, NULL);
+	}
 
 	return SUCCESS;
 }

ext/opcache/tests/array_map_foreach_optimization_006.phpt

@@ -32,26 +32,30 @@ $_main:
 0003 T3 = DO_ICALL
 0004 ASSIGN CV0($array) T3
 0005 ASSIGN CV1($plus1) string("plus1")
-0006 TYPE_ASSERT 131079 string("array_map") CV0($array)
-0007 T3 = INIT_ARRAY 0 (packed) NEXT
-0008 V4 = FE_RESET_R CV0($array) 0015
-0009 T6 = FE_FETCH_R V4 T5 0015
-0010 INIT_DYNAMIC_CALL 1 CV1($plus1)
-0011 SEND_VAL_EX T5 1
-0012 T5 = DO_FCALL
-0013 T3 = ADD_ARRAY_ELEMENT T5 T6
-0014 JMP 0009
-0015 FE_FREE V4
-0016 ASSIGN CV2($foo) T3
-0017 INIT_FCALL 1 %d string("var_dump")
-0018 SEND_VAR CV2($foo) 1
-0019 DO_ICALL
-0020 RETURN int(1)
+0006 T4 = QM_ASSIGN CV1($plus1)
+0007 TYPE_ASSERT 131079 string("array_map") CV0($array)
+0008 T3 = INIT_ARRAY 0 (packed) NEXT
+0009 V5 = FE_RESET_R CV0($array) 0017
+0010 T7 = FE_FETCH_R V5 T6 0017
+0011 T8 = COPY_TMP T4
+0012 INIT_DYNAMIC_CALL 1 T8
+0013 SEND_VAL_EX T6 1
+0014 T6 = DO_FCALL
+0015 T3 = ADD_ARRAY_ELEMENT T6 T7
+0016 JMP 0010
+0017 FE_FREE V5
+0018 FREE T4
+0019 ASSIGN CV2($foo) T3
+0020 INIT_FCALL 1 %d string("var_dump")
+0021 SEND_VAR CV2($foo) 1
+0022 DO_ICALL
+0023 RETURN int(1)
 LIVE RANGES:
-     3: 0008 - 0016 (tmp/var)
-     4: 0009 - 0015 (loop)
-     5: 0010 - 0011 (tmp/var)
-     6: 0010 - 0013 (tmp/var)
+     4: 0007 - 0018 (tmp/var)
+     3: 0009 - 0019 (tmp/var)
+     5: 0010 - 0017 (loop)
+     6: 0011 - 0013 (tmp/var)
+     7: 0011 - 0015 (tmp/var)
 
 plus1:
      ; (lines=3, args=1, vars=1, tmps=%d)

ext/opcache/tests/array_map_foreach_optimization_009.phpt

@@ -0,0 +1,93 @@
+--TEST--
+array_map(): foreach optimization - dynamic-call drift bug
+--CREDITS--
+Ryan @ Calif.io
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+--FILE--
+<?php
+
+function cand_86013_trusted(string $value): string
+{
+    return 'trusted:' . $value;
+}
+
+function cand_86013_unexpected(string $value): string
+{
+    return 'unexpected:' . $value;
+}
+
+function cand_86013_input_changes_target(): array
+{
+    global $callback;
+    global $obj;
+    $callback = 'cand_86013_unexpected';
+    $obj = new Unexpected;
+    return ['payload'];
+}
+
+class Trusted {
+    static function f($value) {
+        return 'trusted:' . $value;
+    }
+}
+
+class Unexpected {
+    static function f($value) {
+        return 'unexpected:' . $value;
+    }
+}
+
+$callback = 'cand_86013_trusted';
+echo "direct array_map\n";
+var_dump(array_map($callback(...), cand_86013_input_changes_target()));
+
+$callback = 'cand_86013_trusted';
+$array_map = 'array_map';
+echo "dynamic-call control\n";
+var_dump($array_map($callback(...), cand_86013_input_changes_target()));
+
+$missing = 'cand_86013_missing';
+echo "empty direct\n";
+try {
+    var_dump(array_map($missing(...), []));
+} catch (Throwable $e) {
+    echo get_class($e), ': ', $e->getMessage(), "\n";
+}
+
+echo "empty dynamic-call control\n";
+try {
+    var_dump($array_map($missing(...), []));
+} catch (Throwable $e) {
+    echo get_class($e), ': ', $e->getMessage(), "\n";
+}
+
+$obj = new Trusted;
+echo "direct array_map static call\n";
+var_dump(array_map($obj::f(...), cand_86013_input_changes_target()));
+
+?>
+--EXPECT--
+direct array_map
+array(1) {
+  [0]=>
+  string(15) "trusted:payload"
+}
+dynamic-call control
+array(1) {
+  [0]=>
+  string(15) "trusted:payload"
+}
+empty direct
+array(0) {
+}
+empty dynamic-call control
+Error: Call to undefined function cand_86013_missing()
+direct array_map static call
+array(1) {
+  [0]=>
+  string(15) "trusted:payload"
+}

ext/opcache/tests/array_map_foreach_optimization_010.phpt

@@ -0,0 +1,68 @@
+--TEST--
+array_map(): foreach optimization - pfa pre-bound arg reexecution bug
+--CREDITS--
+Ryan @ Calif.io
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+--FILE--
+<?php
+
+function cand_86013_bound_value(): string
+{
+    global $bound_calls;
+    echo 'BOUND:', ++$bound_calls, "\n";
+    return 'b';
+}
+
+function cand_86013_input(): array
+{
+    echo "INPUT\n";
+    return ['a', 'a', 'a'];
+}
+
+$bound_calls = 0;
+echo "direct array_map\n";
+$direct = array_map(
+    str_replace('a', cand_86013_bound_value(), ?),
+    cand_86013_input(),
+);
+var_dump($direct, $bound_calls);
+
+$bound_calls = 0;
+$array_map = 'array_map';
+echo "dynamic-call control\n";
+$control = $array_map(
+    str_replace('a', cand_86013_bound_value(), ?),
+    cand_86013_input(),
+);
+var_dump($control, $bound_calls);
+
+?>
+--EXPECT--
+direct array_map
+BOUND:1
+INPUT
+array(3) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "b"
+  [2]=>
+  string(1) "b"
+}
+int(1)
+dynamic-call control
+BOUND:1
+INPUT
+array(3) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "b"
+  [2]=>
+  string(1) "b"
+}
+int(1)