Address review feedback: embed zend_err_buf struct directly in EG.

- Change EG(errors) from pointer to embedded struct (arnaud-lb)
- Replace flexible array member with plain pointer (arnaud-lb)
- Use EG(errors).field syntax instead of EG(errors->field) (ndossche)
- Remove persistent allocation, use erealloc for the inner array
- Remove zend_init_errors_buf/ZEND_ERR_BUF_SIZE, zeroing suffices
- Free EG(errors).errors buffer in zend_free_recorded_errors
- Save/restore full EG(errors) state in error handler paths
- Revert unrelated int->uint32_t change (arnaud-lb)

Author: devnexen

Zend/zend.c

@@ -80,8 +80,6 @@ static char *zend_version_info;
 static uint32_t zend_version_info_length;
 #define ZEND_CORE_VERSION_INFO	"Zend Engine v" ZEND_VERSION ", Copyright (c) Zend Technologies\n"
 #define PRINT_ZVAL_INDENT 4
-#define ZEND_ERR_BUF_SIZE(capacity) \
-	(XtOffsetOf(struct zend_err_buf, buf) + sizeof(zend_error_info *) * (capacity))
 
 /* true multithread-shared globals */
 ZEND_API zend_class_entry *zend_standard_class_def = NULL;
@@ -645,12 +643,6 @@ static FILE *zend_fopen_wrapper(zend_string *filename, zend_string **opened_path
 }
 /* }}} */
 
-static void zend_init_errors_buf(zend_executor_globals *eg)
-{
-	eg->errors = pemalloc(ZEND_ERR_BUF_SIZE(2), true);
-	eg->errors->capacity = 2;
-	eg->errors->size = 0;
-}
 
 #ifdef ZTS
 static bool short_tags_default = true;
@@ -842,7 +834,7 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
 #endif
 	executor_globals->flags = EG_FLAGS_INITIAL;
 	executor_globals->record_errors = false;
-	zend_init_errors_buf(executor_globals);
+	memset(&executor_globals->errors, 0, sizeof(executor_globals->errors));
 	executor_globals->filename_override = NULL;
 	executor_globals->lineno_override = -1;
 #ifdef ZEND_CHECK_STACK_LIMIT
@@ -877,8 +869,6 @@ static void executor_globals_dtor(zend_executor_globals *executor_globals) /* {{
 		zend_hash_destroy(executor_globals->zend_constants);
 		free(executor_globals->zend_constants);
 	}
-
-	pefree(executor_globals->errors, true);
 }
 /* }}} */
 
@@ -1075,7 +1065,6 @@ void zend_startup(zend_utility_functions *utility_functions) /* {{{ */
 	zend_init_rsrc_plist();
 	zend_init_exception_op();
 	zend_init_call_trampoline_op();
-	zend_init_errors_buf(&executor_globals);
 #endif
 
 	zend_ini_startup();
@@ -1158,7 +1147,6 @@ zend_result zend_post_startup(void) /* {{{ */
 	free(EG(zend_constants));
 	EG(zend_constants) = NULL;
 
-	pefree(executor_globals->errors, true);
 	executor_globals_ctor(executor_globals);
 	global_persistent_list = &EG(persistent_list);
 	zend_copy_ini_directives();
@@ -1236,7 +1224,6 @@ void zend_shutdown(void) /* {{{ */
 		pefree(CG(internal_run_time_cache), 1);
 		CG(internal_run_time_cache) = NULL;
 	}
-	pefree(EG(errors), true);
 #endif
 	zend_map_ptr_static_last = 0;
 	zend_map_ptr_static_size = 0;
@@ -1461,6 +1448,7 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
 	bool orig_record_errors;
 	uint32_t orig_num_errors = 0;
 	uint32_t orig_cap_errors = 0;
+	zend_error_info **orig_errors = NULL;
 	zend_result res;
 
 	/* If we're executing a function during SCCP, count any warnings that may be emitted,
@@ -1472,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(errors->size)) {
-		uint32_t num_errors = EG(errors->size);
-		uint32_t cap_errors = EG(errors->capacity);
-		zend_error_info **errors = EG(errors->buf);
-		EG(errors->size) = 0;
+	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;
+		EG(errors).size = 0;
 
 		bool orig_record_errors = EG(record_errors);
 		EG(record_errors) = false;
@@ -1490,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(errors->size) = num_errors;
-		EG(errors->capacity) = cap_errors;
+		EG(errors).size = num_errors;
+		EG(errors).capacity = cap_errors;
+		EG(errors).errors = errors;
 	}
 
 	if (EG(record_errors)) {
@@ -1500,13 +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);
-		EG(errors->size)++;
-		if (EG(errors->size) >= EG(errors->capacity)) {
-			uint32_t capacity = EG(errors->capacity) + (EG(errors->capacity) >> 1);
-			EG(errors) = safe_perealloc(EG(errors), sizeof(zend_error_info *), capacity, XtOffsetOf(struct zend_err_buf, buf), true);
-			EG(errors->capacity) = capacity;
+		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->buf)[EG(errors->size)-1] = info;
+		EG(errors).errors[EG(errors).size - 1] = info;
 
 		/* Do not process non-fatal recorded error */
 		if (!(type & E_FATAL_ERRORS) || (type & E_DONT_BAIL)) {
@@ -1591,16 +1580,18 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
 			orig_record_errors = EG(record_errors);
 			EG(record_errors) = false;
 
-			orig_num_errors = EG(errors->size);
-			orig_cap_errors = EG(errors->capacity);
-			EG(errors->size) = 0;
+			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(errors->capacity) = orig_cap_errors;
-			EG(errors->size) = orig_num_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) {
@@ -1795,7 +1786,7 @@ ZEND_API void zend_begin_record_errors(void)
 {
 	ZEND_ASSERT(!EG(record_errors) && "Error recording already enabled");
 	EG(record_errors) = true;
-	EG(errors->size) = 0;
+	EG(errors).size = 0;
 }
 
 ZEND_API void zend_emit_recorded_errors_ex(uint32_t num_errors, zend_error_info **errors)
@@ -1809,22 +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(errors->size), EG(errors->buf));
+	zend_emit_recorded_errors_ex(EG(errors).size, EG(errors).errors);
 }
 
 ZEND_API void zend_free_recorded_errors(void)
 {
-	if (!EG(errors->size)) {
-		return;
-	}
-
-	for (uint32_t i = 0; i < EG(errors->size); i++) {
-		zend_error_info *info = EG(errors->buf)[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_size(info, sizeof(zend_error_info));
 	}
-	EG(errors->size) = 0;
+	if (EG(errors).errors) {
+		efree(EG(errors).errors);
+	}
+	memset(&EG(errors), 0, sizeof(EG(errors)));
 }
 
 ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */
@@ -2032,12 +2022,12 @@ ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count
 }
 /* }}} */
 
-#define COMPILED_STRING_DESCRIPTION_FORMAT "%s(%u) : %s"
+#define COMPILED_STRING_DESCRIPTION_FORMAT "%s(%d) : %s"
 
 ZEND_API char *zend_make_compiled_string_description(const char *name) /* {{{ */
 {
 	const char *cur_filename;
-	uint32_t cur_lineno;
+	int cur_lineno;
 	char *compiled_string_description;
 
 	if (zend_is_compiling()) {

Zend/zend_execute_API.c

@@ -192,7 +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(errors->size) = 0;
+	memset(&EG(errors), 0, sizeof(EG(errors)));
 
 	EG(filename_override) = NULL;
 	EG(lineno_override) = -1;

Zend/zend_globals.h

@@ -85,7 +85,7 @@ typedef enum {
 struct zend_err_buf {
 	uint32_t size;
 	uint32_t capacity;
-	zend_error_info *buf[1];
+	zend_error_info **errors;
 };
 
 struct _zend_compiler_globals {
@@ -305,7 +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;
-	struct zend_err_buf *errors;
+	struct zend_err_buf errors;
 
 	/* Override filename or line number of thrown errors and exceptions */
 	zend_string *filename_override;

ext/opcache/ZendAccelerator.c

@@ -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(errors->size);
-			persistent_script->warnings = EG(errors->buf);
+			persistent_script->num_warnings = EG(errors).size;
+			persistent_script->warnings = EG(errors).errors;
 		}
 
 		from_memory = false;
@@ -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(errors->size);
-				persistent_script->warnings = EG(errors->buf);
+				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. */
@@ -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(errors->size), EG(errors->buf));
+	zend_persist_warnings_calc(EG(errors).size, EG(errors).errors);
 	size = dummy.size;
 
 	zend_shared_alloc_clear_xlat_table();
@@ -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(errors->size);
-	entry->warnings = zend_persist_warnings(EG(errors->size), EG(errors->buf));
+	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;
 
@@ -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(errors->size) > 0);
-				zend_hash_update_ptr(&errors, key, EG(errors->buf)[EG(errors->size)-1]);
-				EG(errors->size)--;
+				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;