Skip to content

Commit 111e785

Browse files
arnaud-lbarnaud-lb
committed
Merge branch 'PHP-8.6'
* PHP-8.6: Fix array_map optimization with non-literal function or non-literal args (#23254)
2 parents 6507d8b + 40a8468 commit 111e785

7 files changed

Lines changed: 726 additions & 23 deletions

‎Zend/zend_compile.c‎

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5335,11 +5335,64 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
53355335
return FAILURE;
53365336
}
53375337

5338+
/* Bail out if callback is not a first class callable */
53385339
zend_ast *callback = args->child[0];
53395340
if (callback->kind != ZEND_AST_CALL && callback->kind != ZEND_AST_STATIC_CALL) {
53405341
return FAILURE;
53415342
}
53425343

5344+
zend_ast *args_ast = zend_ast_call_get_args(callback);
5345+
if (args_ast->kind != ZEND_AST_CALLABLE_CONVERT) {
5346+
return FAILURE;
5347+
}
5348+
5349+
/* PFAs with non-literal pre-bound arguments are not optimizable because we
5350+
* can't memoize arguments without breaking pass-by-reference.
5351+
* TODO: Support PFAs when the function is known to not receive by ref. */
5352+
zend_ast_fcc *fcc = (zend_ast_fcc*)args_ast;
5353+
zend_ast_list *fcc_args = zend_ast_get_list(fcc->args);
5354+
for (uint32_t i = 0; i < fcc_args->children; i++) {
5355+
zend_ast *arg = fcc_args->child[i];
5356+
if (arg->kind == ZEND_AST_NAMED_ARG) {
5357+
arg = arg->child[1];
5358+
}
5359+
5360+
if (arg->kind == ZEND_AST_PLACEHOLDER_ARG) {
5361+
continue;
5362+
}
5363+
5364+
if (arg->kind != ZEND_AST_ZVAL) {
5365+
return FAILURE;
5366+
}
5367+
}
5368+
5369+
/* Evaluate class name */
5370+
znode class_node;
5371+
if (callback->kind == ZEND_AST_STATIC_CALL) {
5372+
znode result;
5373+
zend_compile_expr(&result, callback->child[0]);
5374+
if (result.op_type == IS_CONST || result.op_type == IS_TMP_VAR) {
5375+
class_node = result;
5376+
} else {
5377+
class_node = result;
5378+
zend_emit_op_tmp(&class_node, ZEND_QM_ASSIGN, &result, NULL);
5379+
}
5380+
} else {
5381+
class_node.op_type = IS_UNUSED;
5382+
}
5383+
5384+
/* Evaluate function name */
5385+
znode func_node;
5386+
{
5387+
znode result;
5388+
zend_compile_expr(&result, callback->child[callback->kind == ZEND_AST_CALL ? 0 : 1]);
5389+
if (result.op_type == IS_CONST || result.op_type == IS_TMP_VAR) {
5390+
func_node = result;
5391+
} else {
5392+
zend_emit_op_tmp(&func_node, ZEND_QM_ASSIGN, &result, NULL);
5393+
}
5394+
}
5395+
53435396
znode value;
53445397
value.op_type = IS_TMP_VAR;
53455398
value.u.op.var = get_temporary_variable();
@@ -5348,6 +5401,12 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
53485401
zend_ast_create_znode(&value));
53495402
if (!call_args) {
53505403
CG(active_op_array)->T--;
5404+
if (func_node.op_type == IS_CONST) {
5405+
zval_ptr_dtor_nogc(&func_node.u.constant);
5406+
}
5407+
if (class_node.op_type == IS_CONST) {
5408+
zval_ptr_dtor_nogc(&class_node.u.constant);
5409+
}
53515410
/* The callback is not a FCC/PFA, or is not optimizable */
53525411
return FAILURE;
53535412
}
@@ -5387,14 +5446,35 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
53875446

53885447
/* loop body */
53895448
znode call_result;
5449+
zend_ast *func_ast;
5450+
if (func_node.op_type == IS_CONST) {
5451+
func_ast = zend_ast_create_znode(&func_node);
5452+
} else {
5453+
znode copy_node;
5454+
zend_emit_op_tmp(&copy_node, ZEND_COPY_TMP, &func_node, NULL);
5455+
func_ast = zend_ast_create_znode(&copy_node);
5456+
}
53905457
switch (callback->kind) {
5391-
case ZEND_AST_CALL:
5392-
zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_CALL, callback->child[0], call_args));
5458+
case ZEND_AST_CALL: {
5459+
zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_CALL, func_ast, call_args));
53935460
break;
5394-
case ZEND_AST_STATIC_CALL:
5395-
zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_STATIC_CALL, callback->child[0], callback->child[1], call_args));
5461+
}
5462+
case ZEND_AST_STATIC_CALL: {
5463+
zend_ast *class_ast;
5464+
if (class_node.op_type == IS_CONST) {
5465+
class_ast = zend_ast_create_znode(&class_node);
5466+
} else {
5467+
znode copy_node;
5468+
zend_emit_op_tmp(&copy_node, ZEND_COPY_TMP, &class_node, NULL);
5469+
class_ast = zend_ast_create_znode(&copy_node);
5470+
}
5471+
5472+
zend_compile_expr(&call_result, zend_ast_create(ZEND_AST_STATIC_CALL, class_ast, func_ast, call_args));
5473+
zend_ast_destroy(class_ast);
53965474
break;
5475+
}
53975476
}
5477+
zend_ast_destroy(func_ast);
53985478
opline = zend_emit_op(NULL, ZEND_ADD_ARRAY_ELEMENT, &call_result, &key);
53995479
SET_NODE(opline->result, result);
54005480
/* end loop body */
@@ -5409,6 +5489,12 @@ static zend_result zend_compile_func_array_map(znode *result, zend_ast_list *arg
54095489

54105490
zend_end_loop(opnum_fetch, &reset_node);
54115491
zend_emit_op(NULL, ZEND_FE_FREE, &reset_node, NULL);
5492+
if (func_node.op_type != IS_CONST) {
5493+
zend_emit_op(NULL, ZEND_FREE, &func_node, NULL);
5494+
}
5495+
if (class_node.op_type != IS_UNUSED && class_node.op_type != IS_CONST) {
5496+
zend_emit_op(NULL, ZEND_FREE, &class_node, NULL);
5497+
}
54125498

54135499
return SUCCESS;
54145500
}

‎ext/opcache/tests/array_map_foreach_optimization_006.phpt‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,30 @@ $_main:
3232
0003 T3 = DO_ICALL
3333
0004 ASSIGN CV0($array) T3
3434
0005 ASSIGN CV1($plus1) string("plus1")
35-
0006 TYPE_ASSERT 131079 string("array_map") CV0($array)
36-
0007 T3 = INIT_ARRAY 0 (packed) NEXT
37-
0008 V4 = FE_RESET_R CV0($array) 0015
38-
0009 T6 = FE_FETCH_R V4 T5 0015
39-
0010 INIT_DYNAMIC_CALL 1 CV1($plus1)
40-
0011 SEND_VAL_EX T5 1
41-
0012 T5 = DO_FCALL
42-
0013 T3 = ADD_ARRAY_ELEMENT T5 T6
43-
0014 JMP 0009
44-
0015 FE_FREE V4
45-
0016 ASSIGN CV2($foo) T3
46-
0017 INIT_FCALL 1 %d string("var_dump")
47-
0018 SEND_VAR CV2($foo) 1
48-
0019 DO_ICALL
49-
0020 RETURN int(1)
35+
0006 T4 = QM_ASSIGN CV1($plus1)
36+
0007 TYPE_ASSERT 131079 string("array_map") CV0($array)
37+
0008 T3 = INIT_ARRAY 0 (packed) NEXT
38+
0009 V5 = FE_RESET_R CV0($array) 0017
39+
0010 T7 = FE_FETCH_R V5 T6 0017
40+
0011 T8 = COPY_TMP T4
41+
0012 INIT_DYNAMIC_CALL 1 T8
42+
0013 SEND_VAL_EX T6 1
43+
0014 T6 = DO_FCALL
44+
0015 T3 = ADD_ARRAY_ELEMENT T6 T7
45+
0016 JMP 0010
46+
0017 FE_FREE V5
47+
0018 FREE T4
48+
0019 ASSIGN CV2($foo) T3
49+
0020 INIT_FCALL 1 %d string("var_dump")
50+
0021 SEND_VAR CV2($foo) 1
51+
0022 DO_ICALL
52+
0023 RETURN int(1)
5053
LIVE RANGES:
51-
3: 0008 - 0016 (tmp/var)
52-
4: 0009 - 0015 (loop)
53-
5: 0010 - 0011 (tmp/var)
54-
6: 0010 - 0013 (tmp/var)
54+
4: 0007 - 0018 (tmp/var)
55+
3: 0009 - 0019 (tmp/var)
56+
5: 0010 - 0017 (loop)
57+
6: 0011 - 0013 (tmp/var)
58+
7: 0011 - 0015 (tmp/var)
5559

5660
plus1:
5761
; (lines=3, args=1, vars=1, tmps=%d)

‎ext/opcache/tests/array_map_foreach_optimization_010.phpt‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
--TEST--
2+
array_map(): foreach optimization - dynamic-call drift bug
3+
--CREDITS--
4+
Ryan @ Calif.io
5+
--EXTENSIONS--
6+
opcache
7+
--INI--
8+
opcache.enable=1
9+
opcache.enable_cli=1
10+
--FILE--
11+
<?php
12+
13+
function cand_86013_trusted(string $value): string
14+
{
15+
return 'trusted:' . $value;
16+
}
17+
18+
function cand_86013_unexpected(string $value): string
19+
{
20+
return 'unexpected:' . $value;
21+
}
22+
23+
function cand_86013_input_changes_target(): array
24+
{
25+
global $callback;
26+
global $obj;
27+
$callback = 'cand_86013_unexpected';
28+
$obj = new Unexpected;
29+
return ['payload'];
30+
}
31+
32+
class Trusted {
33+
static function f($value) {
34+
return 'trusted:' . $value;
35+
}
36+
}
37+
38+
class Unexpected {
39+
static function f($value) {
40+
return 'unexpected:' . $value;
41+
}
42+
}
43+
44+
$callback = 'cand_86013_trusted';
45+
echo "direct array_map\n";
46+
var_dump(array_map($callback(...), cand_86013_input_changes_target()));
47+
48+
$callback = 'cand_86013_trusted';
49+
$array_map = 'array_map';
50+
echo "dynamic-call control\n";
51+
var_dump($array_map($callback(...), cand_86013_input_changes_target()));
52+
53+
$missing = 'cand_86013_missing';
54+
echo "empty direct\n";
55+
try {
56+
var_dump(array_map($missing(...), []));
57+
} catch (Throwable $e) {
58+
echo get_class($e), ': ', $e->getMessage(), "\n";
59+
}
60+
61+
echo "empty dynamic-call control\n";
62+
try {
63+
var_dump($array_map($missing(...), []));
64+
} catch (Throwable $e) {
65+
echo get_class($e), ': ', $e->getMessage(), "\n";
66+
}
67+
68+
$obj = new Trusted;
69+
echo "direct array_map static call\n";
70+
var_dump(array_map($obj::f(...), cand_86013_input_changes_target()));
71+
72+
?>
73+
--EXPECT--
74+
direct array_map
75+
array(1) {
76+
[0]=>
77+
string(15) "trusted:payload"
78+
}
79+
dynamic-call control
80+
array(1) {
81+
[0]=>
82+
string(15) "trusted:payload"
83+
}
84+
empty direct
85+
array(0) {
86+
}
87+
empty dynamic-call control
88+
Error: Call to undefined function cand_86013_missing()
89+
direct array_map static call
90+
array(1) {
91+
[0]=>
92+
string(15) "trusted:payload"
93+
}

‎ext/opcache/tests/array_map_foreach_optimization_011.phpt‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
--TEST--
2+
array_map(): foreach optimization - pfa pre-bound arg reexecution bug
3+
--CREDITS--
4+
Ryan @ Calif.io
5+
--EXTENSIONS--
6+
opcache
7+
--INI--
8+
opcache.enable=1
9+
opcache.enable_cli=1
10+
--FILE--
11+
<?php
12+
13+
function cand_86013_bound_value(): string
14+
{
15+
global $bound_calls;
16+
echo 'BOUND:', ++$bound_calls, "\n";
17+
return 'b';
18+
}
19+
20+
function cand_86013_input(): array
21+
{
22+
echo "INPUT\n";
23+
return ['a', 'a', 'a'];
24+
}
25+
26+
$bound_calls = 0;
27+
echo "direct array_map\n";
28+
$direct = array_map(
29+
str_replace('a', cand_86013_bound_value(), ?),
30+
cand_86013_input(),
31+
);
32+
var_dump($direct, $bound_calls);
33+
34+
$bound_calls = 0;
35+
$array_map = 'array_map';
36+
echo "dynamic-call control\n";
37+
$control = $array_map(
38+
str_replace('a', cand_86013_bound_value(), ?),
39+
cand_86013_input(),
40+
);
41+
var_dump($control, $bound_calls);
42+
43+
?>
44+
--EXPECT--
45+
direct array_map
46+
BOUND:1
47+
INPUT
48+
array(3) {
49+
[0]=>
50+
string(1) "b"
51+
[1]=>
52+
string(1) "b"
53+
[2]=>
54+
string(1) "b"
55+
}
56+
int(1)
57+
dynamic-call control
58+
BOUND:1
59+
INPUT
60+
array(3) {
61+
[0]=>
62+
string(1) "b"
63+
[1]=>
64+
string(1) "b"
65+
[2]=>
66+
string(1) "b"
67+
}
68+
int(1)

0 commit comments

Comments
 (0)