Fix error handling during opcache compilation

Zend/tests/inheritance/deprecation_to_exception_during_inheritance.phpt

@@ -1,5 +1,5 @@
 --TEST--
-Deprecation promoted to exception should result in fatal error during inheritance
+Deprecation promoted to exception during inheritance
 --SKIPIF--
 <?php
 if (getenv('SKIP_PRELOAD')) die('skip Error handler not active during preloading');
@@ -17,7 +17,8 @@ $class = new class extends DateTime {
 
 ?>
 --EXPECTF--
-Fatal error: During inheritance of DateTime: Uncaught Exception: Return type of DateTime@anonymous::getTimezone() should either be compatible with DateTime::getTimezone(): DateTimeZone|false, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in %s:%d
+Fatal error: Uncaught Exception: Return type of DateTime@anonymous::getTimezone() should either be compatible with DateTime::getTimezone(): DateTimeZone|false, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in %s:%d
 Stack trace:
 #0 %s(%d): {closure:%s:%d}(8192, 'Return type of ...', '%s', 8)
-#1 {main} in %s on line %d
+#1 {main}
+  thrown in %s on line %d

Zend/tests/inheritance/gh15907.phpt

@@ -14,5 +14,8 @@ class C implements Serializable {
 
 ?>
 --EXPECTF--
-Fatal error: During inheritance of C, while implementing Serializable: Uncaught Exception: C implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s:%d
-%a
+Fatal error: Uncaught Exception: C implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s:%d
+Stack trace:
+#0 %s(%d): {closure:%s:%d}(8192, 'C implements th...', '/home/arnaud/de...', 7)
+#1 {main}
+  thrown in %s on line %d

Zend/zend.c

@@ -1452,6 +1452,31 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
 		return;
 	}
 
+	/* Emit any delayed error before handling fatal error */
+	if ((type & E_FATAL_ERRORS) && !(type & E_DONT_BAIL)) {
+		if (EG(num_errors)) {
+			uint32_t num_errors = EG(num_errors);
+			zend_error_info **errors = EG(errors);
+			EG(num_errors) = 0;
+			EG(errors) = NULL;
+
+			bool orig_record_errors = EG(record_errors);
+			EG(record_errors) = false;
+
+			/* Disable user error handler before emitting delayed errors, as
+			 * it's unsafe to execute user code after a fatal error. */
+			int orig_user_error_handler_error_reporting = EG(user_error_handler_error_reporting);
+			EG(user_error_handler_error_reporting) = 0;
+
+			zend_emit_recorded_errors_ex(num_errors, errors);
+
+			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;
+		}
+	}
+
 	if (EG(record_errors)) {
 		zend_error_info *info = emalloc(sizeof(zend_error_info));
 		info->type = type;
@@ -1464,6 +1489,11 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
 		EG(num_errors)++;
 		EG(errors) = erealloc(EG(errors), sizeof(zend_error_info*) * EG(num_errors));
 		EG(errors)[EG(num_errors)-1] = info;
+
+		/* Do not process non-fatal recorded error */
+		if (!(type & E_FATAL_ERRORS) || (type & E_DONT_BAIL)) {
+			return;
+		}
 	}
 
 	// Always clear the last backtrace.
@@ -1752,15 +1782,20 @@ ZEND_API void zend_begin_record_errors(void)
 	EG(errors) = NULL;
 }
 
-ZEND_API void zend_emit_recorded_errors(void)
+ZEND_API void zend_emit_recorded_errors_ex(uint32_t num_errors, zend_error_info **errors)
 {
-	EG(record_errors) = false;
-	for (uint32_t i = 0; i < EG(num_errors); i++) {
-		zend_error_info *error = EG(errors)[i];
+	for (uint32_t i = 0; i < num_errors; i++) {
+		zend_error_info *error = errors[i];
 		zend_error_zstr_at(error->type, error->filename, error->lineno, error->message);
 	}
 }
 
+ZEND_API void zend_emit_recorded_errors(void)
+{
+	EG(record_errors) = false;
+	zend_emit_recorded_errors_ex(EG(num_errors), EG(errors));
+}
+
 ZEND_API void zend_free_recorded_errors(void)
 {
 	if (!EG(num_errors)) {

Zend/zend.h

@@ -444,6 +444,7 @@ ZEND_API void zend_replace_error_handling(zend_error_handling_t error_handling,
 ZEND_API void zend_restore_error_handling(zend_error_handling *saved);
 ZEND_API void zend_begin_record_errors(void);
 ZEND_API void zend_emit_recorded_errors(void);
+ZEND_API void zend_emit_recorded_errors_ex(uint32_t num_errors, zend_error_info **errors);
 ZEND_API void zend_free_recorded_errors(void);
 END_EXTERN_C()
 

Zend/zend_compile.c

@@ -1333,7 +1333,6 @@ ZEND_API zend_class_entry *zend_bind_class_in_slot(
 
 	ce = zend_do_link_class(ce, lc_parent_name, Z_STR_P(lcname));
 	if (ce) {
-		ZEND_ASSERT(!EG(exception));
 		zend_observer_class_linked_notify(ce, Z_STR_P(lcname));
 		return ce;
 	}

Zend/zend_globals.h

@@ -295,7 +295,8 @@ struct _zend_executor_globals {
 	size_t fiber_stack_size;
 
 	/* If record_errors is enabled, all emitted diagnostics will be recorded,
-	 * in addition to being processed as usual. */
+	 * 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;

Zend/zend_inheritance.c

@@ -1085,10 +1085,7 @@ static void ZEND_COLD emit_incompatible_method_error(
 				"Return type of %s should either be compatible with %s, "
 				"or the #[\\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice",
 				ZSTR_VAL(child_prototype), ZSTR_VAL(parent_prototype));
-			if (EG(exception)) {
-				zend_exception_uncaught_error(
-					"During inheritance of %s", ZSTR_VAL(parent_scope->name));
-			}
+			ZEND_ASSERT(!EG(exception));
 		}
 	} else {
 		zend_error_at(E_COMPILE_ERROR, func_filename(child), func_lineno(child),
@@ -3759,6 +3756,7 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
 	}
 
 	if (!orig_record_errors) {
+		zend_emit_recorded_errors();
 		zend_free_recorded_errors();
 	}
 	if (traits_and_interfaces) {

Zend/zend_vm_def.h

@@ -7926,7 +7926,7 @@ ZEND_VM_HANDLER(145, ZEND_DECLARE_CLASS_DELAYED, CONST, CONST)
 		if (zv) {
 			SAVE_OPLINE();
 			ce = zend_bind_class_in_slot(zv, lcname, Z_STR_P(RT_CONSTANT(opline, opline->op2)));
-			if (!ce) {
+			if (EG(exception)) {
 				HANDLE_EXCEPTION();
 			}
 		}
@@ -7950,7 +7950,7 @@ ZEND_VM_HANDLER(146, ZEND_DECLARE_ANON_CLASS, ANY, ANY, CACHE_SLOT)
 		if (!(ce->ce_flags & ZEND_ACC_LINKED)) {
 			SAVE_OPLINE();
 			ce = zend_do_link_class(ce, (OP2_TYPE == IS_CONST) ? Z_STR_P(RT_CONSTANT(opline, opline->op2)) : NULL, rtd_key);
-			if (!ce) {
+			if (EG(exception)) {
 				HANDLE_EXCEPTION();
 			}
 		}

ext/opcache/ZendAccelerator.c

@@ -1732,19 +1732,11 @@ static void zend_accel_set_auto_globals(int mask)
 	ZCG(auto_globals_mask) |= mask;
 }
 
-static void replay_warnings(uint32_t num_warnings, zend_error_info **warnings) {
-	for (uint32_t i = 0; i < num_warnings; i++) {
-		zend_error_info *warning = warnings[i];
-		zend_error_zstr_at(warning->type, warning->filename, warning->lineno, warning->message);
-	}
-}
-
 static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handle, int type, zend_op_array **op_array_p)
 {
 	zend_persistent_script *new_persistent_script;
 	uint32_t orig_functions_count, orig_class_count;
 	zend_op_array *orig_active_op_array;
-	zval orig_user_error_handler;
 	zend_op_array *op_array;
 	bool do_bailout = false;
 	accel_time_t timestamp = 0;
@@ -1812,13 +1804,6 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
 	orig_active_op_array = CG(active_op_array);
 	orig_functions_count = EG(function_table)->nNumUsed;
 	orig_class_count = EG(class_table)->nNumUsed;
-	ZVAL_COPY_VALUE(&orig_user_error_handler, &EG(user_error_handler));
-
-	/* Override them with ours */
-	ZVAL_UNDEF(&EG(user_error_handler));
-	if (ZCG(accel_directives).record_warnings) {
-		zend_begin_record_errors();
-	}
 
 	zend_try {
 		orig_compiler_options = CG(compiler_options);
@@ -1848,13 +1833,12 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
 
 	/* Restore originals */
 	CG(active_op_array) = orig_active_op_array;
-	EG(user_error_handler) = orig_user_error_handler;
-	EG(record_errors) = 0;
 
 	if (!op_array) {
 		/* compilation failed */
-		zend_free_recorded_errors();
 		if (do_bailout) {
+			EG(record_errors) = false;
+			zend_free_recorded_errors();
 			zend_bailout();
 		}
 		return NULL;
@@ -1869,10 +1853,6 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
 	zend_accel_move_user_functions(CG(function_table), CG(function_table)->nNumUsed - orig_functions_count, &new_persistent_script->script);
 	zend_accel_move_user_classes(CG(class_table), CG(class_table)->nNumUsed - orig_class_count, &new_persistent_script->script);
 	zend_accel_build_delayed_early_binding_list(new_persistent_script);
-	new_persistent_script->num_warnings = EG(num_errors);
-	new_persistent_script->warnings = EG(errors);
-	EG(num_errors) = 0;
-	EG(errors) = NULL;
 
 	efree(op_array); /* we have valid persistent_script, so it's safe to free op_array */
 
@@ -1954,7 +1934,7 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int
 				}
 			}
 		}
-		replay_warnings(persistent_script->num_warnings, persistent_script->warnings);
+		zend_emit_recorded_errors_ex(persistent_script->num_warnings, persistent_script->warnings);
 
 	    if (persistent_script->ping_auto_globals_mask & ~ZCG(auto_globals_mask)) {
 			zend_accel_set_auto_globals(persistent_script->ping_auto_globals_mask & ~ZCG(auto_globals_mask));
@@ -1963,11 +1943,22 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int
 		return zend_accel_load_script(persistent_script, 1);
 	}
 
+	zend_begin_record_errors();
+
 	persistent_script = opcache_compile_file(file_handle, type, &op_array);
 
 	if (persistent_script) {
+		if (ZCG(accel_directives).record_warnings) {
+			persistent_script->num_warnings = EG(num_errors);
+			persistent_script->warnings = EG(errors);
+		}
+
 		from_memory = false;
 		persistent_script = cache_script_in_file_cache(persistent_script, &from_memory);
+
+		zend_emit_recorded_errors();
+		zend_free_recorded_errors();
+
 		return zend_accel_load_script(persistent_script, from_memory);
 	}
 
@@ -2166,6 +2157,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
 			return accelerator_orig_compile_file(file_handle, type);
 		}
 
+		zend_begin_record_errors();
+
 		SHM_PROTECT();
 		HANDLE_UNBLOCK_INTERRUPTIONS();
 		persistent_script = opcache_compile_file(file_handle, type, &op_array);
@@ -2177,6 +2170,11 @@ 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);
+			}
+
 			/* See GH-17246: we disable GC so that user code cannot be executed during the optimizer run. */
 			bool orig_gc_state = gc_enable(false);
 			persistent_script = cache_script_in_shared_memory(persistent_script, key, &from_shared_memory);
@@ -2189,6 +2187,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
 		if (!persistent_script) {
 			SHM_PROTECT();
 			HANDLE_UNBLOCK_INTERRUPTIONS();
+			zend_emit_recorded_errors();
+			zend_free_recorded_errors();
 			return op_array;
 		}
 		if (from_shared_memory) {
@@ -2202,6 +2202,9 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
 		persistent_script->dynamic_members.last_used = ZCG(request_time);
 		SHM_PROTECT();
 		HANDLE_UNBLOCK_INTERRUPTIONS();
+
+		zend_emit_recorded_errors();
+		zend_free_recorded_errors();
 	} else {
 
 #ifndef ZEND_WIN32
@@ -2244,7 +2247,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
 		SHM_PROTECT();
 		HANDLE_UNBLOCK_INTERRUPTIONS();
 
-		replay_warnings(persistent_script->num_warnings, persistent_script->warnings);
+		zend_emit_recorded_errors_ex(persistent_script->num_warnings, persistent_script->warnings);
 		from_shared_memory = true;
 	}
 
@@ -2311,7 +2314,7 @@ static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce,
 		entry = zend_accel_inheritance_cache_find(entry, ce, parent, traits_and_interfaces, &needs_autoload);
 		if (entry) {
 			if (!needs_autoload) {
-				replay_warnings(entry->num_warnings, entry->warnings);
+				zend_emit_recorded_errors_ex(entry->num_warnings, entry->warnings);
 				if (ZCSG(map_ptr_last) > CG(map_ptr_last)) {
 					zend_map_ptr_extend(ZCSG(map_ptr_last));
 				}
@@ -2465,9 +2468,6 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
 	entry->next = proto->inheritance_cache;
 	proto->inheritance_cache = entry;
 
-	EG(num_errors) = 0;
-	EG(errors) = NULL;
-
 	ZCSG(map_ptr_last) = CG(map_ptr_last);
 
 	zend_shared_alloc_destroy_xlat_table();

ext/opcache/tests/gh17422/001.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-17422 (OPcache bypasses the user-defined error handler for deprecations)
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+--EXTENSIONS--
+opcache
+--FILE--
+<?php
+
+require __DIR__ . "/shutdown.inc";
+
+set_error_handler(static function (int $errno, string $errstr, string $errfile, int $errline) {
+	echo "set_error_handler: {$errstr}", PHP_EOL;
+});
+
+require __DIR__ . "/warning.inc";
+
+warning();
+
+?>
+--EXPECT--
+set_error_handler: "continue" targeting switch is equivalent to "break"
+OK: warning
+array(3) {
+  [0]=>
+  string(7) "001.php"
+  [1]=>
+  string(12) "shutdown.inc"
+  [2]=>
+  string(11) "warning.inc"
+}