Skip to content

Delay notice emission until end of opcode #12090

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 2 commits into from
Closed

Delay notice emission until end of opcode #12090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e01a36
Delay notice emission until end of opcode
nikic Apr 23, 2021
0b53735
Add support for INDIRECT delaying
iluuu1994 Aug 29, 2023
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: 2 additions & 0 deletions Zend/tests/bug52041.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
--TEST--
Bug #52041 (Memory leak when writing on uninitialized variable returned from function)
--INI--
opcache.jit=0
--FILE--
<?php
function foo() {
Expand Down
2 changes: 2 additions & 0 deletions Zend/tests/bug78598.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
--TEST--
Bug #78598: Changing array during undef index RW error segfaults
--INI--
opcache.jit=0
--FILE--
<?php

Expand Down
43 changes: 43 additions & 0 deletions Zend/tests/delayed_error_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
Delayed error 001
--INI--
opcache.jit=0
--FILE--
<?php
$array[0][1] .= 'foo';
$array[2][3]++;
$array[3][4]--;
++$array[5][6];
--$array[7][8];
$array[9][10] += 42;
?>
--EXPECTF--
Warning: Undefined variable $array in %s on line %d

Warning: Undefined array key 0 in %s on line %d

Warning: Undefined array key 1 in %s on line %d

Warning: Undefined array key 2 in %s on line %d

Warning: Undefined array key 3 in %s on line %d

Warning: Decrement on type null has no effect, this will change in the next major version of PHP in %s on line %d

Warning: Undefined array key 3 in %s on line %d

Warning: Undefined array key 4 in %s on line %d

Warning: Undefined array key 5 in %s on line %d

Warning: Undefined array key 6 in %s on line %d

Warning: Decrement on type null has no effect, this will change in the next major version of PHP in %s on line %d

Warning: Undefined array key 7 in %s on line %d

Warning: Undefined array key 8 in %s on line %d

Warning: Undefined array key 9 in %s on line %d

Warning: Undefined array key 10 in %s on line %d
10 changes: 8 additions & 2 deletions Zend/tests/undef_index_to_exception.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
--TEST--
Converting undefined index/offset notice to exception
--INI--
opcache.jit=0
--FILE--
<?php

Expand Down Expand Up @@ -37,10 +39,14 @@ try {
?>
--EXPECT--
Undefined array key 0
array(0) {
array(1) {
[0]=>
string(3) "xyz"
}
Undefined array key "key"
array(0) {
array(1) {
[0]=>
string(3) "xyz"
}
Undefined global variable $test
Undefined variable $test
37 changes: 37 additions & 0 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,18 @@ static void zend_init_exception_op(void) /* {{{ */
}
/* }}} */

static void zend_init_delayed_error_op(void) /* {{{ */
{
memset(EG(delayed_error_op), 0, sizeof(EG(delayed_error_op)));
EG(delayed_error_op)[0].opcode = ZEND_HANDLE_DELAYED_ERROR;
ZEND_VM_SET_OPCODE_HANDLER(EG(delayed_error_op));
EG(delayed_error_op)[1].opcode = ZEND_HANDLE_DELAYED_ERROR;
ZEND_VM_SET_OPCODE_HANDLER(EG(delayed_error_op)+1);
EG(delayed_error_op)[2].opcode = ZEND_HANDLE_DELAYED_ERROR;
ZEND_VM_SET_OPCODE_HANDLER(EG(delayed_error_op)+2);
}
/* }}} */

static void zend_init_call_trampoline_op(void) /* {{{ */
{
memset(&EG(call_trampoline_op), 0, sizeof(EG(call_trampoline_op)));
Expand Down Expand Up @@ -781,6 +793,7 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
zend_copy_constants(executor_globals->zend_constants, GLOBAL_CONSTANTS_TABLE);
zend_init_rsrc_plist();
zend_init_exception_op();
zend_init_delayed_error_op();
zend_init_call_trampoline_op();
memset(&executor_globals->trampoline, 0, sizeof(zend_op_array));
executor_globals->capture_warnings_during_sccp = 0;
Expand Down Expand Up @@ -1020,6 +1033,7 @@ void zend_startup(zend_utility_functions *utility_functions) /* {{{ */
#ifndef ZTS
zend_init_rsrc_plist();
zend_init_exception_op();
zend_init_delayed_error_op();
zend_init_call_trampoline_op();
#endif

Expand Down Expand Up @@ -1704,6 +1718,29 @@ ZEND_API void zend_free_recorded_errors(void)
EG(num_errors) = 0;
}

ZEND_API ZEND_COLD void zend_error_delayed(int type, const char *format, ...)
{
ZEND_ASSERT(!(type & E_FATAL_ERRORS) && "Cannot delay fatal error");
zend_error_info *info = emalloc(sizeof(zend_error_info));
info->type = type;
get_filename_lineno(type, &info->filename, &info->lineno);
zend_string_addref(info->filename);

va_list args;
va_start(args, format);
info->message = zend_vstrpprintf(0, format, args);
va_end(args);

zend_hash_next_index_insert_ptr(&EG(delayed_errors), info);

if (EG(current_execute_data)->opline != EG(delayed_error_op)) {
EG(opline_before_exception) = EG(current_execute_data)->opline;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be dangerous to reuse EG(opline_before_exception) for delayed errors. I'm not sure how delayed errors and exceptions may interact. What if we had an exception before the delayed error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need to change the EG(opline_before_exception) = EG(current_execute_data)->opline; if EG(current_execute_data)->opline == EG(exception_op), because all pending errors are handled before exceptions. But there might still be other issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here they are not handled but recorded, I can't be sure if some instruction can't throw exception and then emit delayed warning.

At least add ZEND_ASSERT(!EG(exception)) to catch the possible problem.

EG(current_execute_data)->opline = EG(delayed_error_op);
/* Reset to ZEND_HANDLE_DELAYED_ERROR */
EG(delayed_error_op)[1] = EG(delayed_error_op)[2];
}
}

ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */
{
va_list va;
Expand Down
1 change: 1 addition & 0 deletions Zend/zend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ ZEND_API ZEND_COLD void zend_error_at(int type, zend_string *filename, uint32_t
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(int type, zend_string *filename, uint32_t lineno, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 4, 5);
ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message);
ZEND_API ZEND_COLD void zend_error_zstr_at(int type, zend_string *filename, uint32_t lineno, zend_string *message);
ZEND_API ZEND_COLD void zend_error_delayed(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);

ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
ZEND_API ZEND_COLD void zend_type_error(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
Expand Down
7 changes: 7 additions & 0 deletions Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,13 @@ ZEND_STATIC_ASSERT(ZEND_MM_ALIGNED_SIZE(sizeof(zval)) == sizeof(zval),
# define RT_CONSTANT(opline, node) \
((zval*)(((char*)(opline)) + (int32_t)(node).constant))

/* */
# define RT_CONSTANT_DELAYED(opline, node) \
((zval*)(((char*)RT_CONSTANT_DELAYED_OPLINE(opline)) + (int32_t)(node).constant))

# define RT_CONSTANT_DELAYED_OPLINE(opline) \
((zend_op*)(opline) != &EG(delayed_error_op)[0] ? (zend_op*)(opline) : EG(opline_before_exception))

/* convert constant from compile-time to run-time */
# define ZEND_PASS_TWO_UPDATE_CONSTANT(op_array, opline, node) do { \
(node).constant = \
Expand Down
89 changes: 88 additions & 1 deletion Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,21 @@
typedef int (ZEND_FASTCALL *incdec_t)(zval *);

#define get_zval_ptr(op_type, node, type) _get_zval_ptr(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_delayed(op_type, node, type) _get_zval_ptr_delayed(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_deref(op_type, node, type) _get_zval_ptr_deref(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_deref_delayed(op_type, node, type) _get_zval_ptr_deref_delayed(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_undef(op_type, node, type) _get_zval_ptr_undef(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_undef_delayed(op_type, node, type) _get_zval_ptr_undef_delayed(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_op_data_zval_ptr_r(op_type, node) _get_op_data_zval_ptr_r(op_type, node EXECUTE_DATA_CC OPLINE_CC)
#define get_op_data_zval_ptr_r_delayed(op_type, node) _get_op_data_zval_ptr_r_delayed(op_type, node EXECUTE_DATA_CC OPLINE_CC)
#define get_op_data_zval_ptr_deref_r(op_type, node) _get_op_data_zval_ptr_deref_r(op_type, node EXECUTE_DATA_CC OPLINE_CC)
#define get_op_data_zval_ptr_deref_r_delayed(op_type, node) _get_op_data_zval_ptr_deref_r_delayed(op_type, node EXECUTE_DATA_CC OPLINE_CC)
#define get_zval_ptr_ptr(op_type, node, type) _get_zval_ptr_ptr(op_type, node, type EXECUTE_DATA_CC)
#define get_zval_ptr_ptr_undef(op_type, node, type) _get_zval_ptr_ptr(op_type, node, type EXECUTE_DATA_CC)
#define get_obj_zval_ptr(op_type, node, type) _get_obj_zval_ptr(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_obj_zval_ptr_delayed(op_type, node, type) _get_obj_zval_ptr_delayed(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_obj_zval_ptr_undef(op_type, node, type) _get_obj_zval_ptr_undef(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_obj_zval_ptr_undef_delayed(op_type, node, type) _get_obj_zval_ptr_undef_delayed(op_type, node, type EXECUTE_DATA_CC OPLINE_CC)
#define get_obj_zval_ptr_ptr(op_type, node, type) _get_obj_zval_ptr_ptr(op_type, node, type EXECUTE_DATA_CC)

#define RETURN_VALUE_USED(opline) ((opline)->result_type != IS_UNUSED)
Expand Down Expand Up @@ -425,6 +432,14 @@ static zend_always_inline zval *_get_zval_ptr(int op_type, znode_op node, int ty
}
}

static zend_always_inline zval *_get_zval_ptr_delayed(int op_type, znode_op node, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT_DELAYED(opline, node);
}
return get_zval_ptr(op_type, node, type);
}

static zend_always_inline zval *_get_op_data_zval_ptr_r(int op_type, znode_op node EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type & (IS_TMP_VAR|IS_VAR)) {
Expand All @@ -445,6 +460,14 @@ static zend_always_inline zval *_get_op_data_zval_ptr_r(int op_type, znode_op no
}
}

static zend_always_inline zval *_get_op_data_zval_ptr_r_delayed(int op_type, znode_op node EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT(RT_CONSTANT_DELAYED_OPLINE(opline)+1, (opline+1)->op1);
}
return get_op_data_zval_ptr_r(op_type, node);
}

static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_zval_ptr_deref(int op_type, znode_op node, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type & (IS_TMP_VAR|IS_VAR)) {
Expand All @@ -465,6 +488,14 @@ static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_zval_ptr_deref(int op
}
}

static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_zval_ptr_deref_delayed(int op_type, znode_op node, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT_DELAYED(opline, node);
}
return get_zval_ptr_deref(op_type, node, type);
}

static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_op_data_zval_ptr_deref_r(int op_type, znode_op node EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type & (IS_TMP_VAR|IS_VAR)) {
Expand All @@ -485,6 +516,14 @@ static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_op_data_zval_ptr_dere
}
}

static zend_always_inline ZEND_ATTRIBUTE_UNUSED zval *_get_op_data_zval_ptr_deref_r_delayed(int op_type, znode_op node EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT(RT_CONSTANT_DELAYED_OPLINE(opline)+1, (opline+1)->op1);
}
return get_op_data_zval_ptr_deref_r(op_type, node);
}

static zend_always_inline zval *_get_zval_ptr_undef(int op_type, znode_op node, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type & (IS_TMP_VAR|IS_VAR)) {
Expand All @@ -505,6 +544,14 @@ static zend_always_inline zval *_get_zval_ptr_undef(int op_type, znode_op node,
}
}

static zend_always_inline zval *_get_zval_ptr_undef_delayed(int op_type, znode_op node, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT_DELAYED(opline, node);
}
return get_zval_ptr_undef(op_type, node, type);
}

static zend_always_inline zval *_get_zval_ptr_ptr_var(uint32_t var EXECUTE_DATA_DC)
{
zval *ret = EX_VAR(var);
Expand Down Expand Up @@ -533,6 +580,14 @@ static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr(int op_type, znode_o
return get_zval_ptr(op_type, op, type);
}

static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr_delayed(int op_type, znode_op op, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT(opline, opline->op1);
}
return get_obj_zval_ptr(op_type, op, type);
}

static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr_undef(int op_type, znode_op op, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_UNUSED) {
Expand All @@ -541,6 +596,14 @@ static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr_undef(int op_type, z
return get_zval_ptr_undef(op_type, op, type);
}

static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr_undef_delayed(int op_type, znode_op op, int type EXECUTE_DATA_DC OPLINE_DC)
{
if (op_type == IS_CONST) {
return RT_CONSTANT(opline, opline->op1);
}
return get_obj_zval_ptr_undef(op_type, op, type);
}

static inline ZEND_ATTRIBUTE_UNUSED zval *_get_obj_zval_ptr_ptr(int op_type, znode_op node, int type EXECUTE_DATA_DC)
{
if (op_type == IS_UNUSED) {
Expand Down Expand Up @@ -2226,6 +2289,28 @@ ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_offset_write(HashTable *ht
return zend_hash_index_add_new(ht, lval, &EG(uninitialized_zval));
}

ZEND_API ZEND_COLD void ZEND_FASTCALL zend_undefined_offset_delayed(zend_long lval)
{
zend_error_delayed(E_WARNING, "Undefined array key " ZEND_LONG_FMT, lval);
}

ZEND_API void ZEND_FASTCALL zend_handle_delayed_errors(void)
{
/* Clear EG(delayed_errors), as more errors may be delayed while we are handling these. */
HashTable ht;
memcpy(&ht, &EG(delayed_errors), sizeof(HashTable));
zend_hash_init(&EG(delayed_errors), 0, NULL, NULL, 0);

zend_error_info *info;
ZEND_HASH_FOREACH_PTR(&ht, info) {
zend_error_zstr_at(info->type, info->filename, info->lineno, info->message);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May the first error throw an exception? Then the second. Which exception is going to be caught?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They could be chained, similar to here: https://3v4l.org/um5TH I'm not sure how this currently behaves.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm too :)
Probably the exception thrown from the last error handler is going to be caught first.
Or EG(exception) set by the first handler will prevent the second handler form execution.

zend_string_release(info->filename);
zend_string_release(info->message);
efree(info);
} ZEND_HASH_FOREACH_END();
zend_hash_destroy(&ht);
}

ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_index_write(HashTable *ht, zend_string *offset)
{
zval *retval;
Expand Down Expand Up @@ -2495,7 +2580,8 @@ static zend_always_inline zval *zend_fetch_dimension_address_inner(HashTable *ht
retval = &EG(uninitialized_zval);
break;
case BP_VAR_RW:
retval = zend_undefined_offset_write(ht, hval);
zend_undefined_offset_delayed(hval);
retval = zend_hash_index_add_new(ht, hval, &EG(uninitialized_zval));
break;
}
} else {
Expand Down Expand Up @@ -3236,6 +3322,7 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c

if (prop_op_type == IS_CONST) {
name = Z_STR_P(prop_ptr);
tmp_name = NULL;
} else {
name = zval_get_tmp_string(prop_ptr, &tmp_name);
}
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_invalid_class_constant_type_error(uin
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_object_released_while_assigning_to_property_error(const zend_property_info *info);

ZEND_API ZEND_COLD void ZEND_FASTCALL zend_cannot_add_element(void);
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_undefined_offset_delayed(zend_long lval);

ZEND_API void ZEND_FASTCALL zend_handle_delayed_errors(void);

ZEND_API bool zend_verify_scalar_type_hint(uint32_t type_mask, zval *arg, bool strict, bool is_internal_arg);
ZEND_API ZEND_COLD void zend_verify_arg_error(
Expand Down
4 changes: 4 additions & 0 deletions Zend/zend_execute_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ void init_executor(void) /* {{{ */
zend_objects_store_init(&EG(objects_store), 1024);

EG(full_tables_cleanup) = 0;
EG(delayed_handlers) = 0;
ZEND_ATOMIC_BOOL_INIT(&EG(vm_interrupt), false);
ZEND_ATOMIC_BOOL_INIT(&EG(timed_out), false);

Expand Down Expand Up @@ -201,6 +202,9 @@ void init_executor(void) /* {{{ */

zend_max_execution_timer_init();
zend_fiber_init();

zend_hash_init(&EG(delayed_errors), 0, NULL, NULL, 0);

zend_weakrefs_init();

EG(active) = 1;
Expand Down
5 changes: 5 additions & 0 deletions Zend/zend_globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ struct _zend_executor_globals {

bool full_tables_cleanup;

bool delayed_handlers;

zend_atomic_bool vm_interrupt;
zend_atomic_bool timed_out;

Expand Down Expand Up @@ -246,6 +248,8 @@ struct _zend_executor_globals {
zend_object *exception, *prev_exception;
const zend_op *opline_before_exception;
zend_op exception_op[3];
/* FIXME: Is this safe for VM reentry? We may have to save/restore this value. */
zend_op delayed_error_op[3];

struct _zend_module_entry *current_module;

Expand Down Expand Up @@ -303,6 +307,7 @@ struct _zend_executor_globals {
pid_t pid;
struct sigaction oldact;
#endif
HashTable delayed_errors;

void *reserved[ZEND_MAX_RESERVED_RESOURCES];
};
Expand Down
Loading