Skip to content

Don't make argument nullable based on AST null initializer #4720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Don't make argument nullable based on AST null initializer #4720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/tests/bug68446.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function a(array $a = FOO) {
var_dump($a);
}

function b(array $b = BAR) {
function b(?array $b = BAR) {
var_dump($b);
}

Expand Down
30 changes: 26 additions & 4 deletions Zend/tests/type_declarations/scalar_constant_defaults.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ function int_val_default_null(int $a = NULL_VAL) {
return $a;
}

function nullable_int_val_default_null(?int $a = NULL_VAL) {
return $a;
}

echo "Testing int val" . PHP_EOL;
var_dump(int_val());

Expand All @@ -58,13 +62,27 @@ echo "Testing string add val" . PHP_EOL;
var_dump(string_add_val());

echo "Testing int with default null constant" . PHP_EOL;
var_dump(int_val_default_null());
try {
var_dump(int_val_default_null());
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

echo "Testing int with null null constant" . PHP_EOL;
var_dump(int_val_default_null(null));
try {
var_dump(int_val_default_null(null));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

echo "Testing nullable int with default null constant" . PHP_EOL;
var_dump(nullable_int_val_default_null());

echo "Testing nullable int with null null constant" . PHP_EOL;
var_dump(nullable_int_val_default_null(null));

?>
--EXPECT--
--EXPECTF--
Testing int val
int(10)
Testing float val
Expand All @@ -78,6 +96,10 @@ float(10.7)
Testing string add val
string(14) "this is a test"
Testing int with default null constant
NULL
Argument 1 passed to int_val_default_null() must be of the type int, null given, called in %s on line %d
Testing int with null null constant
Argument 1 passed to int_val_default_null() must be of the type int, null given, called in %s on line %d
Testing nullable int with default null constant
NULL
Testing nullable int with null null constant
NULL
39 changes: 11 additions & 28 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,23 +793,6 @@ static ZEND_COLD void zend_verify_arg_error(
}
}

static int is_null_constant(zend_class_entry *scope, zval *default_value)
{
if (Z_TYPE_P(default_value) == IS_CONSTANT_AST) {
zval constant;

ZVAL_COPY(&constant, default_value);
if (UNEXPECTED(zval_update_constant_ex(&constant, scope) != SUCCESS)) {
return 0;
}
if (Z_TYPE(constant) == IS_NULL) {
return 1;
}
zval_ptr_dtor_nogc(&constant);
}
return 0;
}

static zend_bool zend_verify_weak_scalar_type_hint(zend_uchar type_hint, zval *arg)
{
switch (type_hint) {
Expand Down Expand Up @@ -1043,7 +1026,7 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf
static zend_always_inline zend_bool zend_check_type(
zend_type type,
zval *arg, zend_class_entry **ce, void **cache_slot,
zval *default_value, zend_class_entry *scope,
zend_class_entry *scope,
zend_bool is_return_type, zend_bool is_internal)
{
zend_reference *ref = NULL;
Expand All @@ -1063,19 +1046,19 @@ static zend_always_inline zend_bool zend_check_type(
} else {
*ce = zend_fetch_class(ZEND_TYPE_NAME(type), (ZEND_FETCH_CLASS_AUTO | ZEND_FETCH_CLASS_NO_AUTOLOAD));
if (UNEXPECTED(!*ce)) {
return Z_TYPE_P(arg) == IS_NULL && (ZEND_TYPE_ALLOW_NULL(type) || (default_value && is_null_constant(scope, default_value)));
return Z_TYPE_P(arg) == IS_NULL && ZEND_TYPE_ALLOW_NULL(type);
}
*cache_slot = (void *) *ce;
}
if (EXPECTED(Z_TYPE_P(arg) == IS_OBJECT)) {
return instanceof_function(Z_OBJCE_P(arg), *ce);
}
return Z_TYPE_P(arg) == IS_NULL && (ZEND_TYPE_ALLOW_NULL(type) || (default_value && is_null_constant(scope, default_value)));
return Z_TYPE_P(arg) == IS_NULL && ZEND_TYPE_ALLOW_NULL(type);
} else if (EXPECTED(ZEND_TYPE_CODE(type) == Z_TYPE_P(arg))) {
return 1;
}

if (Z_TYPE_P(arg) == IS_NULL && (ZEND_TYPE_ALLOW_NULL(type) || (default_value && is_null_constant(scope, default_value)))) {
if (Z_TYPE_P(arg) == IS_NULL && ZEND_TYPE_ALLOW_NULL(type)) {
/* Null passed to nullable type */
return 1;
}
Expand Down Expand Up @@ -1104,7 +1087,7 @@ static zend_always_inline zend_bool zend_check_type(
* because this case is already checked at compile-time. */
}

static zend_always_inline int zend_verify_recv_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, zval *default_value, void **cache_slot)
static zend_always_inline int zend_verify_recv_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, void **cache_slot)
{
zend_arg_info *cur_arg_info = &zf->common.arg_info[arg_num-1];
zend_class_entry *ce;
Expand All @@ -1113,15 +1096,15 @@ static zend_always_inline int zend_verify_recv_arg_type(zend_function *zf, uint3
cur_arg_info = &zf->common.arg_info[arg_num-1];

ce = NULL;
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0, 0))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, zf->common.scope, 0, 0))) {
zend_verify_arg_error(zf, cur_arg_info, arg_num, ce, arg);
return 0;
}

return 1;
}

static zend_always_inline int zend_verify_variadic_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, zval *default_value, void **cache_slot)
static zend_always_inline int zend_verify_variadic_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, void **cache_slot)
{
zend_arg_info *cur_arg_info;
zend_class_entry *ce;
Expand All @@ -1131,7 +1114,7 @@ static zend_always_inline int zend_verify_variadic_arg_type(zend_function *zf, u
cur_arg_info = &zf->common.arg_info[zf->common.num_args];

ce = NULL;
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0, 0))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, zf->common.scope, 0, 0))) {
zend_verify_arg_error(zf, cur_arg_info, arg_num, ce, arg);
return 0;
}
Expand All @@ -1158,7 +1141,7 @@ static zend_never_inline ZEND_ATTRIBUTE_UNUSED int zend_verify_internal_arg_type
break;
}

if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, &dummy_cache_slot, NULL, fbc->common.scope, 0, /* is_internal */ 1))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, &dummy_cache_slot, fbc->common.scope, 0, /* is_internal */ 1))) {
return 0;
}
arg++;
Expand Down Expand Up @@ -1254,7 +1237,7 @@ static int zend_verify_internal_return_type(zend_function *zf, zval *ret)
return 1;
}

if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, &dummy_cache_slot, NULL, NULL, 1, /* is_internal */ 1))) {
if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, &dummy_cache_slot, NULL, 1, /* is_internal */ 1))) {
zend_verify_internal_return_error(zf, ce, ret);
return 0;
}
Expand All @@ -1268,7 +1251,7 @@ static zend_always_inline void zend_verify_return_type(zend_function *zf, zval *
zend_arg_info *ret_info = zf->common.arg_info - 1;
zend_class_entry *ce = NULL;

if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, cache_slot, NULL, NULL, 1, 0))) {
if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, cache_slot, NULL, 1, 0))) {
zend_verify_return_error(zf, ce, ret);
}
}
Expand Down
8 changes: 3 additions & 5 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5198,7 +5198,7 @@ ZEND_VM_HOT_HANDLER(63, ZEND_RECV, NUM, UNUSED|CACHE_SLOT)
zval *param = EX_VAR(opline->result.var);

SAVE_OPLINE();
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, NULL, CACHE_ADDR(opline->op2.num)) || EG(exception))) {
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->op2.num)) || EG(exception))) {
HANDLE_EXCEPTION();
}
}
Expand Down Expand Up @@ -5243,10 +5243,8 @@ ZEND_VM_HOT_HANDLER(64, ZEND_RECV_INIT, NUM, CONST, CACHE_SLOT)
}

if (UNEXPECTED((EX(func)->op_array.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) != 0)) {
zval *default_value = RT_CONSTANT(opline, opline->op2);

SAVE_OPLINE();
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, default_value, CACHE_ADDR(opline->extended_value)) || EG(exception))) {
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->extended_value)) || EG(exception))) {
HANDLE_EXCEPTION();
}
}
Expand Down Expand Up @@ -5275,7 +5273,7 @@ ZEND_VM_HANDLER(164, ZEND_RECV_VARIADIC, NUM, UNUSED|CACHE_SLOT)
param = EX_VAR_NUM(EX(func)->op_array.last_var + EX(func)->op_array.T);
if (UNEXPECTED((EX(func)->op_array.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) != 0)) {
do {
zend_verify_variadic_arg_type(EX(func), arg_num, param, NULL, CACHE_ADDR(opline->op2.num));
zend_verify_variadic_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->op2.num));
if (Z_OPT_REFCOUNTED_P(param)) Z_ADDREF_P(param);
ZEND_HASH_FILL_ADD(param);
param++;
Expand Down
8 changes: 3 additions & 5 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3046,10 +3046,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_RECV_INIT_SPEC_CON
}

if (UNEXPECTED((EX(func)->op_array.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) != 0)) {
zval *default_value = RT_CONSTANT(opline, opline->op2);

SAVE_OPLINE();
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, default_value, CACHE_ADDR(opline->extended_value)) || EG(exception))) {
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->extended_value)) || EG(exception))) {
HANDLE_EXCEPTION();
}
}
Expand Down Expand Up @@ -3127,7 +3125,7 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_RECV_SPEC_UNUSED_H
zval *param = EX_VAR(opline->result.var);

SAVE_OPLINE();
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, NULL, CACHE_ADDR(opline->op2.num)) || EG(exception))) {
if (UNEXPECTED(!zend_verify_recv_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->op2.num)) || EG(exception))) {
HANDLE_EXCEPTION();
}
}
Expand Down Expand Up @@ -3155,7 +3153,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_RECV_VARIADIC_SPEC_UNUSED_HAND
param = EX_VAR_NUM(EX(func)->op_array.last_var + EX(func)->op_array.T);
if (UNEXPECTED((EX(func)->op_array.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) != 0)) {
do {
zend_verify_variadic_arg_type(EX(func), arg_num, param, NULL, CACHE_ADDR(opline->op2.num));
zend_verify_variadic_arg_type(EX(func), arg_num, param, CACHE_ADDR(opline->op2.num));
if (Z_OPT_REFCOUNTED_P(param)) Z_ADDREF_P(param);
ZEND_HASH_FILL_ADD(param);
param++;
Expand Down
5 changes: 0 additions & 5 deletions ext/opcache/Optimizer/zend_inference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3104,11 +3104,6 @@ static int zend_update_type_info(const zend_op_array *op_array,
ce = NULL;
if (arg_info) {
tmp = zend_fetch_arg_info_type(script, arg_info, &ce);
if (opline->opcode == ZEND_RECV_INIT &&
Z_TYPE_P(CRT_CONSTANT_EX(op_array, opline, opline->op2, ssa->rt_constants)) == IS_CONSTANT_AST) {
/* The constant may resolve to NULL */
tmp |= MAY_BE_NULL;
}
if (arg_info->pass_by_reference) {
tmp |= MAY_BE_REF;
}
Expand Down
22 changes: 2 additions & 20 deletions ext/opcache/jit/zend_jit_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,23 +1115,6 @@ static zval* ZEND_FASTCALL zend_jit_fetch_global_helper(zend_execute_data *execu
return value;
}

static int is_null_constant(zend_class_entry *scope, zval *default_value)
{
if (Z_CONSTANT_P(default_value)) {
zval constant;

ZVAL_COPY(&constant, default_value);
if (UNEXPECTED(zval_update_constant_ex(&constant, scope) != SUCCESS)) {
return 0;
}
if (Z_TYPE(constant) == IS_NULL) {
return 1;
}
zval_ptr_dtor(&constant);
}
return 0;
}

static zend_bool zend_verify_weak_scalar_type_hint(zend_uchar type_hint, zval *arg)
{
switch (type_hint) {
Expand Down Expand Up @@ -1313,12 +1296,11 @@ static void ZEND_FASTCALL zend_jit_verify_arg_object(zval *arg, zend_op_array *o
}
}

static void ZEND_FASTCALL zend_jit_verify_arg_slow(zval *arg, zend_op_array *op_array, uint32_t arg_num, zend_arg_info *arg_info, void **cache_slot, zval *default_value)
static void ZEND_FASTCALL zend_jit_verify_arg_slow(zval *arg, zend_op_array *op_array, uint32_t arg_num, zend_arg_info *arg_info, void **cache_slot)
{
zend_class_entry *ce = NULL;

if (Z_TYPE_P(arg) == IS_NULL &&
(ZEND_TYPE_ALLOW_NULL(arg_info->type) || (default_value && is_null_constant(op_array->scope, default_value)))) {
if (Z_TYPE_P(arg) == IS_NULL && ZEND_TYPE_ALLOW_NULL(arg_info->type)) {
/* Null passed to nullable type */
return;
}
Expand Down
6 changes: 0 additions & 6 deletions ext/opcache/jit/zend_jit_x86.dasc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9021,16 +9021,13 @@ static int zend_jit_recv(dasm_State **Dst, const zend_op *opline, zend_op_array
| mov CARG3, arg_num
| LOAD_ADDR CARG4, (ptrdiff_t)arg_info
| mov aword A5, r0
| mov aword A6, 0
| EXT_CALL zend_jit_verify_arg_slow, r0
|.elif X64
| mov CARG3, arg_num
| LOAD_ADDR CARG4, (ptrdiff_t)arg_info
| mov CARG5, r0
| xor CARG6, CARG6
| EXT_CALL zend_jit_verify_arg_slow, r0
|.else
| push 0
| push r0
| push (ptrdiff_t)arg_info
| push arg_num
Expand Down Expand Up @@ -9190,16 +9187,13 @@ static int zend_jit_recv_init(dasm_State **Dst, const zend_op *opline, zend_op_a
| mov CARG3, arg_num
| LOAD_ADDR CARG4, (ptrdiff_t)arg_info
| mov aword A5, r0
| ADDR_OP2_2 mov, aword A6, zv, r0
| EXT_CALL zend_jit_verify_arg_slow, r0
|.elif X64
| mov CARG3, arg_num
| LOAD_ADDR CARG4, (ptrdiff_t)arg_info
| mov CARG5, r0
| LOAD_ADDR CARG6, zv
| EXT_CALL zend_jit_verify_arg_slow, r0
|.else
| push zv
| push r0
| push (ptrdiff_t)arg_info
| push arg_num
Expand Down