diff --git a/NEWS b/NEWS index b1e02edfbd2..5b28e16265e 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,8 @@ PHP NEWS executed on threads not managed by TSRM. (Kévin Dunglas) . Fixed potential NULL pointer dereference Windows shm*() functions. (cmb) . Added shadow stack support for fibers. (Chen Hu) + . Fix bug GH-9965 (Fix accidental caching of default arguments with side + effects). (ilutov) - Fileinfo: . Upgrade bundled libmagic to 5.43. (Anatol) diff --git a/Zend/tests/function_default_argument_cache.phpt b/Zend/tests/function_default_argument_cache.phpt new file mode 100644 index 00000000000..0861566c09e --- /dev/null +++ b/Zend/tests/function_default_argument_cache.phpt @@ -0,0 +1,25 @@ +--TEST-- +Function default argument is not cached when its evaluation had side effects +--FILE-- + +--EXPECT-- +string(1) "0" +string(1) "1" +string(1) "2" diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 63146fc5dd2..b007c08c0e5 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -498,17 +498,17 @@ zend_class_entry *zend_ast_fetch_class(zend_ast *ast, zend_class_entry *scope) 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); } -static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, bool *short_circuited_ptr) +ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx) { zval op1, op2; zend_result ret = SUCCESS; - *short_circuited_ptr = false; + ctx->short_circuited = false; switch (ast->kind) { case ZEND_AST_BINARY_OP: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; - } else if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; } else { @@ -520,9 +520,9 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as break; case ZEND_AST_GREATER: case ZEND_AST_GREATER_EQUAL: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; - } else if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + } else if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; } else { @@ -535,7 +535,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as } break; case ZEND_AST_UNARY_OP: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; } else { 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 } break; case ZEND_AST_AND: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; break; } if (zend_is_true(&op1)) { - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -606,14 +606,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as zval_ptr_dtor_nogc(&op1); break; case ZEND_AST_OR: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; break; } if (zend_is_true(&op1)) { ZVAL_TRUE(result); } else { - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -624,7 +624,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as zval_ptr_dtor_nogc(&op1); break; case ZEND_AST_CONDITIONAL: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; break; } @@ -632,7 +632,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as if (!ast->child[1]) { *result = op1; } else { - if (UNEXPECTED(zend_ast_evaluate(result, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -640,7 +640,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as zval_ptr_dtor_nogc(&op1); } } else { - if (UNEXPECTED(zend_ast_evaluate(result, ast->child[2], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[2], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -649,14 +649,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as } break; case ZEND_AST_COALESCE: - if (UNEXPECTED(zend_ast_evaluate(&op1, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; break; } if (Z_TYPE(op1) > IS_NULL) { *result = op1; } else { - if (UNEXPECTED(zend_ast_evaluate(result, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(result, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -665,7 +665,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as } break; case ZEND_AST_UNARY_PLUS: - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; } else { ZVAL_LONG(&op1, 0); @@ -674,7 +674,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as } break; case ZEND_AST_UNARY_MINUS: - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; } else { ZVAL_LONG(&op1, 0); @@ -695,7 +695,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as for (i = 0; i < list->children; i++) { zend_ast *elem = list->child[i]; if (elem->kind == ZEND_AST_UNPACK) { - if (UNEXPECTED(zend_ast_evaluate(&op1, elem->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[0], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(result); return FAILURE; } @@ -708,14 +708,14 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as continue; } if (elem->child[1]) { - if (UNEXPECTED(zend_ast_evaluate(&op1, elem->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, elem->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(result); return FAILURE; } } else { ZVAL_UNDEF(&op1); } - if (UNEXPECTED(zend_ast_evaluate(&op2, elem->child[0], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, elem->child[0], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); zval_ptr_dtor_nogc(result); return FAILURE; @@ -734,13 +734,11 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as zend_error_noreturn(E_COMPILE_ERROR, "Cannot use [] for reading"); } - bool short_circuited; - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { ret = FAILURE; break; } - if (short_circuited) { - *short_circuited_ptr = true; + if (ctx->short_circuited) { ZVAL_NULL(result); return SUCCESS; } @@ -753,7 +751,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as break; } - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); ret = FAILURE; break; @@ -787,7 +785,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as zval case_value_zv; ZVAL_UNDEF(&case_value_zv); if (case_value_ast != NULL) { - if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&case_value_zv, case_value_ast, scope, ctx) != SUCCESS)) { return FAILURE; } } @@ -834,6 +832,9 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as return FAILURE; } + /* Even if there is no constructor, the object can have cause side-effects in various ways (__toString(), __get(), __isset(), etc). */ + ctx->had_side_effects = true; + zend_ast_list *args_ast = zend_ast_get_list(ast->child[1]); if (args_ast->attr) { /* Has named arguments. */ @@ -846,7 +847,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as name = zend_ast_get_str(arg_ast->child[0]); arg_ast = arg_ast->child[1]; } - if (zend_ast_evaluate(&arg, arg_ast, scope) == FAILURE) { + if (zend_ast_evaluate_ex(&arg, arg_ast, scope, ctx) == FAILURE) { zend_array_destroy(args); zval_ptr_dtor(result); return FAILURE; @@ -876,7 +877,7 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as ALLOCA_FLAG(use_heap) zval *args = do_alloca(sizeof(zval) * args_ast->children, use_heap); for (uint32_t i = 0; i < args_ast->children; i++) { - if (zend_ast_evaluate(&args[i], args_ast->child[i], scope) == FAILURE) { + if (zend_ast_evaluate_ex(&args[i], args_ast->child[i], scope, ctx) == FAILURE) { for (uint32_t j = 0; j < i; j++) { zval_ptr_dtor(&args[j]); } @@ -908,22 +909,20 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as case ZEND_AST_PROP: case ZEND_AST_NULLSAFE_PROP: { - bool short_circuited; - if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, &short_circuited) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op1, ast->child[0], scope, ctx) != SUCCESS)) { return FAILURE; } - if (short_circuited) { - *short_circuited_ptr = true; + if (ctx->short_circuited) { ZVAL_NULL(result); return SUCCESS; } if (ast->kind == ZEND_AST_NULLSAFE_PROP && Z_TYPE(op1) == IS_NULL) { - *short_circuited_ptr = true; + ctx->short_circuited = true; ZVAL_NULL(result); return SUCCESS; } - if (UNEXPECTED(zend_ast_evaluate(&op2, ast->child[1], scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&op2, ast->child[1], scope, ctx) != SUCCESS)) { zval_ptr_dtor_nogc(&op1); return FAILURE; } @@ -976,8 +975,8 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *as ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope) { - bool short_circuited; - return zend_ast_evaluate_ex(result, ast, scope, &short_circuited); + zend_ast_evaluate_ctx ctx = {0}; + return zend_ast_evaluate_ex(result, ast, scope, &ctx); } static size_t ZEND_FASTCALL zend_ast_tree_size(zend_ast *ast) diff --git a/Zend/zend_ast.h b/Zend/zend_ast.h index d2940bbe633..e957bdf1fdf 100644 --- a/Zend/zend_ast.h +++ b/Zend/zend_ast.h @@ -297,7 +297,13 @@ ZEND_API zend_ast *zend_ast_create_decl( zend_string *name, zend_ast *child0, zend_ast *child1, zend_ast *child2, zend_ast *child3, zend_ast *child4 ); +typedef struct { + bool had_side_effects; + bool short_circuited; +} zend_ast_evaluate_ctx; + ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope); +ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_ex(zval *result, zend_ast *ast, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx); ZEND_API zend_string *zend_ast_export(const char *prefix, zend_ast *ast, const char *suffix); ZEND_API zend_ast_ref * ZEND_FASTCALL zend_ast_copy(zend_ast *ast); diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index f8171d9a0c0..ed6d6646186 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -179,6 +179,7 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval ZEND_API zend_result ZEND_FASTCALL zval_update_constant(zval *pp); ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *pp, zend_class_entry *scope); +ZEND_API zend_result ZEND_FASTCALL zval_update_constant_with_ctx(zval *pp, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx); /* dedicated Zend executor functions - do not use! */ struct _zend_vm_stack { diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index c0607c628d9..8f993190ab1 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -670,7 +670,7 @@ ZEND_API bool zend_is_executing(void) /* {{{ */ } /* }}} */ -ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_entry *scope) /* {{{ */ +ZEND_API zend_result ZEND_FASTCALL zval_update_constant_with_ctx(zval *p, zend_class_entry *scope, zend_ast_evaluate_ctx *ctx) { if (Z_TYPE_P(p) == IS_CONSTANT_AST) { 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 } else { zval tmp; - if (UNEXPECTED(zend_ast_evaluate(&tmp, ast, scope) != SUCCESS)) { + if (UNEXPECTED(zend_ast_evaluate_ex(&tmp, ast, scope, ctx) != SUCCESS)) { return FAILURE; } zval_ptr_dtor_nogc(p); @@ -698,6 +698,12 @@ ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_e } /* }}} */ +ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *p, zend_class_entry *scope) +{ + zend_ast_evaluate_ctx ctx = {0}; + return zval_update_constant_with_ctx(p, scope, &ctx); +} + ZEND_API zend_result ZEND_FASTCALL zval_update_constant(zval *pp) /* {{{ */ { return zval_update_constant_ex(pp, EG(current_execute_data) ? zend_get_executed_scope() : CG(active_class_entry)); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 48f894117e8..b1c629d8bbb 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -5510,12 +5510,13 @@ ZEND_VM_HOT_HANDLER(64, ZEND_RECV_INIT, NUM, CONST, CACHE_SLOT) } else { SAVE_OPLINE(); ZVAL_COPY(param, default_value); - if (UNEXPECTED(zval_update_constant_ex(param, EX(func)->op_array.scope) != SUCCESS)) { + zend_ast_evaluate_ctx ctx = {0}; + if (UNEXPECTED(zval_update_constant_with_ctx(param, EX(func)->op_array.scope, &ctx) != SUCCESS)) { zval_ptr_dtor_nogc(param); ZVAL_UNDEF(param); HANDLE_EXCEPTION(); } - if (!Z_REFCOUNTED_P(param)) { + if (!Z_REFCOUNTED_P(param) && !ctx.had_side_effects) { ZVAL_COPY_VALUE(cache_val, param); } } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index e06a374b35b..b86c90be2b3 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3834,12 +3834,13 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_RECV_INIT_SPEC_CON } else { SAVE_OPLINE(); ZVAL_COPY(param, default_value); - if (UNEXPECTED(zval_update_constant_ex(param, EX(func)->op_array.scope) != SUCCESS)) { + zend_ast_evaluate_ctx ctx = {0}; + if (UNEXPECTED(zval_update_constant_with_ctx(param, EX(func)->op_array.scope, &ctx) != SUCCESS)) { zval_ptr_dtor_nogc(param); ZVAL_UNDEF(param); HANDLE_EXCEPTION(); } - if (!Z_REFCOUNTED_P(param)) { + if (!Z_REFCOUNTED_P(param) && !ctx.had_side_effects) { ZVAL_COPY_VALUE(cache_val, param); } }