Skip to content

Zend: Preallocate error buffer with capacity tracking #20565

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

Merged
devnexen merged 4 commits into from
Mar 30, 2026
Merged

Zend: Preallocate error buffer with capacity tracking #20565

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cef4d30
Zend: Preallocate error buffer with capacity tracking
devnexen Nov 23, 2025
155e86f
review
devnexen Jan 24, 2026
49c8d34
Address review feedback: embed zend_err_buf struct directly in EG.
devnexen Mar 30, 2026
cbddab7
Address review feedback: use struct copy for save/restore, cleanup.
devnexen Mar 30, 2026
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
74 changes: 38 additions & 36 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,7 @@ static FILE *zend_fopen_wrapper(zend_string *filename, zend_string **opened_path
}
/* }}} */


Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Mar 30, 2026

Choose a reason for hiding this comment

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

Unexpected white space change

#ifdef ZTS
static bool short_tags_default = true;
static uint32_t compiler_options_default = ZEND_COMPILE_DEFAULT;
Expand Down Expand Up @@ -833,8 +834,7 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
#endif
executor_globals->flags = EG_FLAGS_INITIAL;
executor_globals->record_errors = false;
executor_globals->num_errors = 0;
executor_globals->errors = NULL;
memset(&executor_globals->errors, 0, sizeof(executor_globals->errors));
executor_globals->filename_override = NULL;
executor_globals->lineno_override = -1;
#ifdef ZEND_CHECK_STACK_LIMIT
Expand Down Expand Up @@ -1446,8 +1446,9 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
zend_stack delayed_oplines_stack;
int type = orig_type & E_ALL;
bool orig_record_errors;
uint32_t orig_num_errors;
zend_error_info **orig_errors;
uint32_t orig_num_errors = 0;
uint32_t orig_cap_errors = 0;
zend_error_info **orig_errors = NULL;
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Mar 30, 2026

Choose a reason for hiding this comment

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

Here it maybe possible to replace these 3 variables by a single zend_err_buf variable

zend_result res;

/* If we're executing a function during SCCP, count any warnings that may be emitted,
Expand All @@ -1459,11 +1460,11 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
}

/* Emit any delayed error before handling fatal error */
if ((type & E_FATAL_ERRORS) && !(type & E_DONT_BAIL) && EG(num_errors)) {
uint32_t num_errors = EG(num_errors);
zend_error_info **errors = EG(errors);
EG(num_errors) = 0;
EG(errors) = NULL;
if ((type & E_FATAL_ERRORS) && !(type & E_DONT_BAIL) && EG(errors).size) {
uint32_t num_errors = EG(errors).size;
uint32_t cap_errors = EG(errors).capacity;
zend_error_info **errors = EG(errors).errors;
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Mar 30, 2026

Choose a reason for hiding this comment

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

Ditto

EG(errors).size = 0;

bool orig_record_errors = EG(record_errors);
EG(record_errors) = false;
Expand All @@ -1477,8 +1478,9 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(

EG(user_error_handler_error_reporting) = orig_user_error_handler_error_reporting;
EG(record_errors) = orig_record_errors;
EG(num_errors) = num_errors;
EG(errors) = errors;
EG(errors).size = num_errors;
EG(errors).capacity = cap_errors;
EG(errors).errors = errors;
}

if (EG(record_errors)) {
Expand All @@ -1487,12 +1489,13 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
info->lineno = error_lineno;
info->filename = zend_string_copy(error_filename);
info->message = zend_string_copy(message);

/* This is very inefficient for a large number of errors.
* Use pow2 realloc if it becomes a problem. */
EG(num_errors)++;
EG(errors) = erealloc(EG(errors), sizeof(zend_error_info*) * EG(num_errors));
EG(errors)[EG(num_errors)-1] = info;
EG(errors).size++;
if (EG(errors).size > EG(errors).capacity) {
uint32_t capacity = EG(errors).capacity ? EG(errors).capacity + (EG(errors).capacity >> 1) : 2;
EG(errors).errors = erealloc(EG(errors).errors, sizeof(zend_error_info *) * capacity);
EG(errors).capacity = capacity;
}
EG(errors).errors[EG(errors).size - 1] = info;

/* Do not process non-fatal recorded error */
if (!(type & E_FATAL_ERRORS) || (type & E_DONT_BAIL)) {
Expand Down Expand Up @@ -1575,17 +1578,20 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
}

orig_record_errors = EG(record_errors);
orig_num_errors = EG(num_errors);
orig_errors = EG(errors);
EG(record_errors) = false;
EG(num_errors) = 0;
EG(errors) = NULL;

orig_num_errors = EG(errors).size;
orig_cap_errors = EG(errors).capacity;
orig_errors = EG(errors).errors;
memset(&EG(errors), 0, sizeof(EG(errors)));

res = call_user_function(CG(function_table), NULL, &orig_user_error_handler, &retval, 4, params);

EG(record_errors) = orig_record_errors;
EG(num_errors) = orig_num_errors;
EG(errors) = orig_errors;

EG(errors).capacity = orig_cap_errors;
EG(errors).size = orig_num_errors;
EG(errors).errors = orig_errors;

if (res == SUCCESS) {
if (Z_TYPE(retval) != IS_UNDEF) {
Expand Down Expand Up @@ -1780,8 +1786,7 @@ ZEND_API void zend_begin_record_errors(void)
{
ZEND_ASSERT(!EG(record_errors) && "Error recording already enabled");
EG(record_errors) = true;
EG(num_errors) = 0;
EG(errors) = NULL;
EG(errors).size = 0;
}

ZEND_API void zend_emit_recorded_errors_ex(uint32_t num_errors, zend_error_info **errors)
Expand All @@ -1795,24 +1800,21 @@ ZEND_API void zend_emit_recorded_errors_ex(uint32_t num_errors, zend_error_info
ZEND_API void zend_emit_recorded_errors(void)
{
EG(record_errors) = false;
zend_emit_recorded_errors_ex(EG(num_errors), EG(errors));
zend_emit_recorded_errors_ex(EG(errors).size, EG(errors).errors);
}

ZEND_API void zend_free_recorded_errors(void)
{
if (!EG(num_errors)) {
return;
}
Comment on lines -1803 to -1805
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Mar 30, 2026

Choose a reason for hiding this comment

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

We could keep this check to avoid the memset() and other checks when there are not errors


for (uint32_t i = 0; i < EG(num_errors); i++) {
zend_error_info *info = EG(errors)[i];
for (uint32_t i = 0; i < EG(errors).size; i++) {
zend_error_info *info = EG(errors).errors[i];
zend_string_release(info->filename);
zend_string_release(info->message);
efree(info);
efree_size(info, sizeof(zend_error_info));
}
if (EG(errors).errors) {
efree(EG(errors).errors);
}
efree(EG(errors));
EG(errors) = NULL;
EG(num_errors) = 0;
memset(&EG(errors), 0, sizeof(EG(errors)));
}

ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */
Expand Down
3 changes: 1 addition & 2 deletions Zend/zend_execute_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ void init_executor(void) /* {{{ */
EG(get_gc_buffer).start = EG(get_gc_buffer).end = EG(get_gc_buffer).cur = NULL;

EG(record_errors) = false;
EG(num_errors) = 0;
EG(errors) = NULL;
memset(&EG(errors), 0, sizeof(EG(errors)));

EG(filename_override) = NULL;
EG(lineno_override) = -1;
Expand Down
10 changes: 8 additions & 2 deletions Zend/zend_globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,20 @@ typedef struct _zend_vm_stack *zend_vm_stack;
typedef struct _zend_ini_entry zend_ini_entry;
typedef struct _zend_fiber_context zend_fiber_context;
typedef struct _zend_fiber zend_fiber;
typedef struct _zend_error_info zend_error_info;

typedef enum {
ZEND_MEMOIZE_NONE,
ZEND_MEMOIZE_COMPILE,
ZEND_MEMOIZE_FETCH,
} zend_memoize_mode;

struct zend_err_buf {
uint32_t size;
uint32_t capacity;
zend_error_info **errors;
};
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Mar 30, 2026

Choose a reason for hiding this comment

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

Nit: we rarely use structs directly without a typedef:

Suggested change
struct zend_err_buf {
uint32_t size;
uint32_t capacity;
zend_error_info **errors;
};
typedef struct zend_err_buf {
uint32_t size;
uint32_t capacity;
zend_error_info **errors;
} zend_err_buf;

(Then use zend_err_buf intead of struct zend_err_buf everywhere)


struct _zend_compiler_globals {
zend_stack loop_var_stack;

Expand Down Expand Up @@ -298,8 +305,7 @@ struct _zend_executor_globals {
* and their processing is delayed until zend_emit_recorded_errors()
* is called or a fatal diagnostic is emitted. */
bool record_errors;
uint32_t num_errors;
zend_error_info **errors;
struct zend_err_buf errors;

/* Override filename or line number of thrown errors and exceptions */
zend_string *filename_override;
Expand Down
20 changes: 10 additions & 10 deletions ext/opcache/ZendAccelerator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1968,8 +1968,8 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int

if (persistent_script) {
if (ZCG(accel_directives).record_warnings) {
persistent_script->num_warnings = EG(num_errors);
persistent_script->warnings = EG(errors);
persistent_script->num_warnings = EG(errors).size;
persistent_script->warnings = EG(errors).errors;
}

from_memory = false;
Expand Down Expand Up @@ -2193,8 +2193,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
from_shared_memory = false;
if (persistent_script) {
if (ZCG(accel_directives).record_warnings) {
persistent_script->num_warnings = EG(num_errors);
persistent_script->warnings = EG(errors);
persistent_script->num_warnings = EG(errors).size;
persistent_script->warnings = EG(errors).errors;
}

/* See GH-17246: we disable GC so that user code cannot be executed during the optimizer run. */
Expand Down Expand Up @@ -2421,7 +2421,7 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
}
ZCG(current_persistent_script) = &dummy;
zend_persist_class_entry_calc(ce);
zend_persist_warnings_calc(EG(num_errors), EG(errors));
zend_persist_warnings_calc(EG(errors).size, EG(errors).errors);
size = dummy.size;

zend_shared_alloc_clear_xlat_table();
Expand Down Expand Up @@ -2491,8 +2491,8 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
JIT_G(on) = jit_on_old;
#endif

entry->num_warnings = EG(num_errors);
entry->warnings = zend_persist_warnings(EG(num_errors), EG(errors));
entry->num_warnings = EG(errors).size;
entry->warnings = zend_persist_warnings(EG(errors).size, EG(errors).errors);
entry->next = proto->inheritance_cache;
proto->inheritance_cache = entry;

Expand Down Expand Up @@ -4123,9 +4123,9 @@ static void preload_link(void)
/* Remember the last error. */
zend_error_cb = orig_error_cb;
EG(record_errors) = false;
ZEND_ASSERT(EG(num_errors) > 0);
zend_hash_update_ptr(&errors, key, EG(errors)[EG(num_errors)-1]);
EG(num_errors)--;
ZEND_ASSERT(EG(errors).size > 0);
zend_hash_update_ptr(&errors, key, EG(errors).errors[EG(errors).size-1]);
EG(errors).size--;
} zend_end_try();
CG(in_compilation) = false;
CG(compiled_filename) = NULL;
Expand Down
Loading