Fix error handling inconsistency with opcache

When opcache is enabled, error handling is altered in the following ways:

 * Errors emitted during compilation bypass the user-defined error handler
 * Exceptions emitted during class linking are turned into fatal errors

Changes here make the behavior consistent regardless of opcache being enabled or
not:

 * Errors emitted during compilation and class linking are always delayed and
   handled after compilation or class linking. During handling, user-defined
   error handlers are not bypassed. Fatal errors emitted during compilation or
   class linking cause any delayed errors to be handled immediately (without
   calling user-defined error handlers, as it would be unsafe).
 * Exceptions thrown by user-defined error handlers when handling class linking
   error are not promoted to fatal errors anymore and do not prevent linking.

Fixes GH-17422.
Closes GH-18541.
Closes GH-17627.

Co-authored-by: Tim Düsterhus <[email protected]>

Author: arnaud-lb

.github/scripts/windows/test_task.bat

@@ -128,6 +128,7 @@ mkdir %PHP_BUILD_DIR%\test_file_cache
 rem generate php.ini
 echo extension_dir=%PHP_BUILD_DIR% > %PHP_BUILD_DIR%\php.ini
 echo opcache.file_cache=%PHP_BUILD_DIR%\test_file_cache >> %PHP_BUILD_DIR%\php.ini
+echo opcache.record_warnings=1 >> %PHP_BUILD_DIR%\php.ini
 rem work-around for some spawned PHP processes requiring OpenSSL and sockets
 echo extension=php_openssl.dll >> %PHP_BUILD_DIR%\php.ini
 echo extension=php_sockets.dll >> %PHP_BUILD_DIR%\php.ini

NEWS

@@ -15,8 +15,11 @@ PHP                                                                        NEWS
 - OPcache:
   . Disallow changing opcache.memory_consumption when SHM is already set up.
     (timwolla)
-  . Fixed GH-15074 (Compiling opcache statically into ZTS PHP fails). (Arnaud)
+  . Fixed bug GH-15074 (Compiling opcache statically into ZTS PHP fails).
+    (Arnaud)
   . Make OPcache non-optional (Arnaud, timwolla)
+  . Fixed bug GH-17422 (OPcache bypasses the user-defined error handler for
+    deprecations). (Arnaud, timwolla)
 
 - OpenSSL:
   . Add $digest_algo parameter to openssl_public_encrypt() and

UPGRADING

@@ -44,6 +44,12 @@ PHP 8.5 UPGRADE NOTES
   . Traits are now bound before the parent class. This is a subtle behavioral
     change, but should more closely match user expectations, demonstrated by
     GH-15753 and GH-16198.
+  . Errors emitted during compilation and class linking are now always delayed
+    and handled after compilation or class linking. Fatal errors emitted during
+    compilation or class linking cause any delayed errors to be handled
+    immediately, without calling user-defined error handlers.
+  . Exceptions thrown by user-defined error handlers when handling class linking
+    errors are not promoted to fatal errors anymore and do not prevent linking.
 
 - DOM:
   . Cloning a DOMNamedNodeMap, DOMNodeList, Dom\NamedNodeMap, Dom\NodeList,

UPGRADING.INTERNALS

@@ -71,6 +71,8 @@ PHP 8.5 INTERNALS UPGRADE NOTES
     * zend_register_string_constant()
     * zend_register_stringl_constant()
   . EG(fake_scope) now is a _const_ zend_class_entry*.
+  . zend_begin_record_errors() or EG(record_errors)=true cause errors to be
+    delayed. Before, errors would be recorded but not delayed.
 
 ========================
 2. Build system changes

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/deprecation_to_exception_during_inheritance_can_be_caught.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Deprecation promoted to exception during inheritance
+--SKIPIF--
+<?php
+if (getenv('SKIP_PRELOAD')) die('skip Error handler not active during preloading');
+?>
+--FILE--
+<?php
+
+set_error_handler(function($code, $message) {
+    throw new Exception($message);
+});
+
+try {
+    class C extends DateTime {
+        public function getTimezone() {}
+        public function getTimestamp() {}
+    };
+} catch (Exception $e) {
+    printf("%s: %s\n", $e::class, $e->getMessage());
+}
+
+var_dump(new C());
+
+?>
+--EXPECTF--
+Exception: Return type of C::getTimezone() should either be compatible with DateTime::getTimezone(): DateTimeZone|false, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
+object(C)#%d (3) {
+  ["date"]=>
+  string(%d) "%s"
+  ["timezone_type"]=>
+  int(3)
+  ["timezone"]=>
+  string(3) "UTC"
+}

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...', '%s', 7)
+#1 {main}
+  thrown in %s on line %d

Zend/zend.c

@@ -1452,6 +1452,29 @@ 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) && 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 +1487,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 +1780,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

@@ -1327,7 +1327,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;
 	}