Skip to content

Commit 8731fb2

Browse files
iluuu1994iluuu1994
committed
Fix caching of default params with side-effects
Fixes GH-9965 Closes GH-9935
1 parent 649083d commit 8731fb2

8 files changed

+84
-43
lines changed

‎NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ PHP NEWS
1616
executed on threads not managed by TSRM. (Kévin Dunglas)
1717
. Fixed potential NULL pointer dereference Windows shm*() functions. (cmb)
1818
. Added shadow stack support for fibers. (Chen Hu)
19+
. Fix bug GH-9965 (Fix accidental caching of default arguments with side
20+
effects). (ilutov)
1921

2022
- Fileinfo:
2123
. Upgrade bundled libmagic to 5.43. (Anatol)

‎Zend/tests/function_default_argument_cache.phpt

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Function default argument is not cached when its evaluation had side effects
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __toString() {
8+
static $i = 0;
9+
return (string) $i++;
10+
}
11+
}
12+
13+
function test(string $foo = new Foo() . '') {
14+
var_dump($foo);
15+
}
16+
17+
test();
18+
test();
19+
test();
20+
21+
?>
22+
--EXPECT--
23+
string(1) "0"
24+
string(1) "1"
25+
string(1) "2"

‎Zend/zend_ast.c

+36-37
Original file line numberDiff line numberDiff line change
@@ -498,17 +498,17 @@ zend_class_entry *zend_ast_fetch_class(zend_ast *ast, zend_class_entry *scope)
498498
return zend_fetch_class_with_scope(zend_ast_get_str(ast), (ast->attr >> ZEND_CONST_EXPR_NEW_FETCH_TYPE_SHIFT) | ZEND_FETCH_CLASS_EXCEPTION, scope);
499499
}
500500

501-
static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, bool *short_circuited_ptr)
501+
ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx)
502502
{
503503
zval op1, op2;
504504
zend_result ret = SUCCESS;
505-
*short_circuited_ptr = false;
505+
ctx->short_circuited = false;
506506

507507
switch (ast->kind) {
508508
case ZEND_AST_BINARY_OP:
509-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
509+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
510510
ret = FAILURE;
511-
} else if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
511+
} else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
512512
zval_ptr_dtor_nogc(&op1);
513513
ret = FAILURE;
514514
} else {
@@ -520,9 +520,9 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
520520
break;
521521
case ZEND_AST_GREATER:
522522
case ZEND_AST_GREATER_EQUAL:
523-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
523+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
524524
ret = FAILURE;
525-
} else if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
525+
} else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
526526
zval_ptr_dtor_nogc(&op1);
527527
ret = FAILURE;
528528
} else {
@@ -535,7 +535,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
535535
}
536536
break;
537537
case ZEND_AST_UNARY_OP:
538-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
538+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
539539
ret = FAILURE;
540540
} else {
541541
unary_op_type op = get_unary_op(ast->attr);
@@ -588,12 +588,12 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
588588
}
589589
break;
590590
case ZEND_AST_AND:
591-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
591+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
592592
ret = FAILURE;
593593
break;
594594
}
595595
if (zend_is_true(&op1)) {
596-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
596+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
597597
zval_ptr_dtor_nogc(&op1);
598598
ret = FAILURE;
599599
break;
@@ -606,14 +606,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
606606
zval_ptr_dtor_nogc(&op1);
607607
break;
608608
case ZEND_AST_OR:
609-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
609+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
610610
ret = FAILURE;
611611
break;
612612
}
613613
if (zend_is_true(&op1)) {
614614
ZVAL_TRUE(result);
615615
} else {
616-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
616+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
617617
zval_ptr_dtor_nogc(&op1);
618618
ret = FAILURE;
619619
break;
@@ -624,23 +624,23 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
624624
zval_ptr_dtor_nogc(&op1);
625625
break;
626626
case ZEND_AST_CONDITIONAL:
627-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
627+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
628628
ret = FAILURE;
629629
break;
630630
}
631631
if (zend_is_true(&op1)) {
632632
if (!ast->child[1]) {
633633
*result = op1;
634634
} else {
635-
if (UNEXPECTED(zend_ast_evaluate(result, ast->child[1], scope) != SUCCESS)) {
635+
if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) {
636636
zval_ptr_dtor_nogc(&op1);
637637
ret = FAILURE;
638638
break;
639639
}
640640
zval_ptr_dtor_nogc(&op1);
641641
}
642642
} else {
643-
if (UNEXPECTED(zend_ast_evaluate(result, ast->child[2], scope) != SUCCESS)) {
643+
if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[2], scope, ctx) != SUCCESS)) {
644644
zval_ptr_dtor_nogc(&op1);
645645
ret = FAILURE;
646646
break;
@@ -649,14 +649,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
649649
}
650650
break;
651651
case ZEND_AST_COALESCE:
652-
if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) {
652+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
653653
ret = FAILURE;
654654
break;
655655
}
656656
if (Z_TYPE(op1) > IS_NULL) {
657657
*result = op1;
658658
} else {
659-
if (UNEXPECTED(zend_ast_evaluate(result, ast->child[1], scope) != SUCCESS)) {
659+
if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) {
660660
zval_ptr_dtor_nogc(&op1);
661661
ret = FAILURE;
662662
break;
@@ -665,7 +665,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
665665
}
666666
break;
667667
case ZEND_AST_UNARY_PLUS:
668-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[0], scope) != SUCCESS)) {
668+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) {
669669
ret = FAILURE;
670670
} else {
671671
ZVAL_LONG(&op1, 0);
@@ -674,7 +674,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
674674
}
675675
break;
676676
case ZEND_AST_UNARY_MINUS:
677-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[0], scope) != SUCCESS)) {
677+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) {
678678
ret = FAILURE;
679679
} else {
680680
ZVAL_LONG(&op1, 0);
@@ -695,7 +695,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
695695
for (i = 0; i < list->children; i++) {
696696
zend_ast *elem = list->child[i];
697697
if (elem->kind == ZEND_AST_UNPACK) {
698-
if (UNEXPECTED(zend_ast_evaluate(&op1, elem->child[0], scope) != SUCCESS)) {
698+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[0], scope, ctx) != SUCCESS)) {
699699
zval_ptr_dtor_nogc(result);
700700
return FAILURE;
701701
}
@@ -708,14 +708,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
708708
continue;
709709
}
710710
if (elem->child[1]) {
711-
if (UNEXPECTED(zend_ast_evaluate(&op1, elem->child[1], scope) != SUCCESS)) {
711+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[1], scope, ctx) != SUCCESS)) {
712712
zval_ptr_dtor_nogc(result);
713713
return FAILURE;
714714
}
715715
} else {
716716
ZVAL_UNDEF(&op1);
717717
}
718-
if (UNEXPECTED(zend_ast_evaluate(&op2, elem->child[0], scope) != SUCCESS)) {
718+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, elem->child[0], scope, ctx) != SUCCESS)) {
719719
zval_ptr_dtor_nogc(&op1);
720720
zval_ptr_dtor_nogc(result);
721721
return FAILURE;
@@ -734,13 +734,11 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
734734
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use [] for reading");
735735
}
736736

737-
bool short_circuited;
738-
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited) != SUCCESS)) {
737+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
739738
ret = FAILURE;
740739
break;
741740
}
742-
if (short_circuited) {
743-
*short_circuited_ptr = true;
741+
if (ctx->short_circuited) {
744742
ZVAL_NULL(result);
745743
return SUCCESS;
746744
}
@@ -753,7 +751,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
753751
break;
754752
}
755753

756-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
754+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
757755
zval_ptr_dtor_nogc(&op1);
758756
ret = FAILURE;
759757
break;
@@ -787,7 +785,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
787785
zval case_value_zv;
788786
ZVAL_UNDEF(&case_value_zv);
789787
if (case_value_ast != NULL) {
790-
if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) {
788+
if (UNEXPECTED(zend_ast_evaluate_ex(&case_value_zv, case_value_ast, scope, ctx) != SUCCESS)) {
791789
return FAILURE;
792790
}
793791
}
@@ -834,6 +832,9 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
834832
return FAILURE;
835833
}
836834

835+
/* Even if there is no constructor, the object can have cause side-effects in various ways (__toString(), __get(), __isset(), etc). */
836+
ctx->had_side_effects = true;
837+
837838
zend_ast_list *args_ast = zend_ast_get_list(ast->child[1]);
838839
if (args_ast->attr) {
839840
/* Has named arguments. */
@@ -846,7 +847,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
846847
name = zend_ast_get_str(arg_ast->child[0]);
847848
arg_ast = arg_ast->child[1];
848849
}
849-
if (zend_ast_evaluate(&arg, arg_ast, scope) == FAILURE) {
850+
if (zend_ast_evaluate_ex(&arg, arg_ast, scope, ctx) == FAILURE) {
850851
zend_array_destroy(args);
851852
zval_ptr_dtor(result);
852853
return FAILURE;
@@ -876,7 +877,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
876877
ALLOCA_FLAG(use_heap)
877878
zval *args = do_alloca(sizeof(zval) * args_ast->children, use_heap);
878879
for (uint32_t i = 0; i < args_ast->children; i++) {
879-
if (zend_ast_evaluate(&args[i], args_ast->child[i], scope) == FAILURE) {
880+
if (zend_ast_evaluate_ex(&args[i], args_ast->child[i], scope, ctx) == FAILURE) {
880881
for (uint32_t j = 0; j < i; j++) {
881882
zval_ptr_dtor(&args[j]);
882883
}
@@ -908,22 +909,20 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
908909
case ZEND_AST_PROP:
909910
case ZEND_AST_NULLSAFE_PROP:
910911
{
911-
bool short_circuited;
912-
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited) != SUCCESS)) {
912+
if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) {
913913
return FAILURE;
914914
}
915-
if (short_circuited) {
916-
*short_circuited_ptr = true;
915+
if (ctx->short_circuited) {
917916
ZVAL_NULL(result);
918917
return SUCCESS;
919918
}
920919
if (ast->kind == ZEND_AST_NULLSAFE_PROP && Z_TYPE(op1) == IS_NULL) {
921-
*short_circuited_ptr = true;
920+
ctx->short_circuited = true;
922921
ZVAL_NULL(result);
923922
return SUCCESS;
924923
}
925924

926-
if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) {
925+
if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) {
927926
zval_ptr_dtor_nogc(&op1);
928927
return FAILURE;
929928
}
@@ -976,8 +975,8 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as
976975

977976
ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope)
978977
{
979-
bool short_circuited;
980-
return zend_ast_evaluate_ex(result, ast, scope, &short_circuited);
978+
zend_ast_evaluate_ctx ctx = {0};
979+
return zend_ast_evaluate_ex(result, ast, scope, &ctx);
981980
}
982981

983982
static size_t ZEND_FASTCALL zend_ast_tree_size(zend_ast *ast)

‎Zend/zend_ast.h

+6
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,13 @@ ZEND_API zend_ast *zend_ast_create_decl(
297297
zend_string *name, zend_ast *child0, zend_ast *child1, zend_ast *child2, zend_ast *child3, zend_ast *child4
298298
);
299299

300+
typedef struct {
301+
bool had_side_effects;
302+
bool short_circuited;
303+
} zend_ast_evaluate_ctx;
304+
300305
ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope);
306+
ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx);
301307
ZEND_API zend_string *zend_ast_export(const char *prefix, zend_ast *ast, const char *suffix);
302308

303309
ZEND_API zend_ast_ref * ZEND_FASTCALL zend_ast_copy(zend_ast *ast);

‎Zend/zend_execute.h

+1
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval
179179

180180
ZEND_API zend_result ZEND_FASTCALL zval_update_constant(zval *pp);
181181
ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *pp, zend_class_entry *scope);
182+
ZEND_API zend_result ZEND_FASTCALL zval_update_constant_with_ctx(zval *pp, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx);
182183

183184
/* dedicated Zend executor functions - do not use! */
184185
struct _zend_vm_stack {

‎Zend/zend_execute_API.c

+8-2
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ ZEND_API bool zend_is_executing(void) /* {{{ */
670670
}
671671
/* }}} */
672672

673-
ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_entry *scope) /* {{{ */
673+
ZEND_API zend_result ZEND_FASTCALL zval_update_constant_with_ctx(zval *p, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx)
674674
{
675675
if (Z_TYPE_P(p) == IS_CONSTANT_AST) {
676676
zend_ast *ast = Z_ASTVAL_P(p);
@@ -687,7 +687,7 @@ ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_e
687687
} else {
688688
zval tmp;
689689

690-
if (UNEXPECTED(zend_ast_evaluate(&tmp, ast, scope) != SUCCESS)) {
690+
if (UNEXPECTED(zend_ast_evaluate_ex(&tmp, ast, scope, ctx) != SUCCESS)) {
691691
return FAILURE;
692692
}
693693
zval_ptr_dtor_nogc(p);
@@ -698,6 +698,12 @@ ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_e
698698
}
699699
/* }}} */
700700

701+
ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_entry *scope)
702+
{
703+
zend_ast_evaluate_ctx ctx = {0};
704+
return zval_update_constant_with_ctx(p, scope, &ctx);
705+
}
706+
701707
ZEND_API zend_result ZEND_FASTCALL zval_update_constant(zval *pp) /* {{{ */
702708
{
703709
return zval_update_constant_ex(pp, EG(current_execute_data) ? zend_get_executed_scope() : CG(active_class_entry));

‎Zend/zend_vm_def.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -5510,12 +5510,13 @@ ZEND_VM_HOT_HANDLER(64, ZEND_RECV_INIT, NUM, CONST, CACHE_SLOT)
55105510
} else {
55115511
SAVE_OPLINE();
55125512
ZVAL_COPY(param, default_value);
5513-
if (UNEXPECTED(zval_update_constant_ex(param, EX(func)->op_array.scope) != SUCCESS)) {
5513+
zend_ast_evaluate_ctx ctx = {0};
5514+
if (UNEXPECTED(zval_update_constant_with_ctx(param, EX(func)->op_array.scope, &ctx) != SUCCESS)) {
55145515
zval_ptr_dtor_nogc(param);
55155516
ZVAL_UNDEF(param);
55165517
HANDLE_EXCEPTION();
55175518
}
5518-
if (!Z_REFCOUNTED_P(param)) {
5519+
if (!Z_REFCOUNTED_P(param) && !ctx.had_side_effects) {
55195520
ZVAL_COPY_VALUE(cache_val, param);
55205521
}
55215522
}

‎Zend/zend_vm_execute.h

+3-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)