Fix GH-24361: Restore GC after opcache_compile_file errors

NEWS

@@ -50,6 +50,8 @@ PHP                                                                        NEWS
   . Fixed use-after-free in JIT when the error handler releases a string while
     an "Undefined variable" warning is raised for its offset in ?? or nested
     isset()/empty(). (Jorg Sowa)
+  . Fixed bug GH-24361 (opcache_compile_file() disables cycle collection after
+    a compile error). (akasakariko)
 
 - PDO_Firebird:
   . Fixed stack buffer overflow with long connection user names, passwords,

ext/opcache/ZendAccelerator.c

@@ -3241,6 +3241,8 @@ static int accel_startup(zend_extension *extension)
 	/* Prevent unloading */
 	extension->handle = 0;
 
+	zend_observer_error_register(zend_accel_record_fatal_error);
+
 	return SUCCESS;
 }
 

ext/opcache/tests/gh24361.inc

@@ -0,0 +1,2 @@
+<?php
+break 2;

ext/opcache/tests/gh24361.phpt

@@ -0,0 +1,53 @@
+--TEST--
+GH-24361 (opcache_compile_file() disables cycle collection after a compile error)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.file_update_protection=0
+--FILE--
+<?php
+var_dump(gc_status()['protected']);
+var_dump(@opcache_compile_file(__DIR__ . '/gh24361.inc'));
+var_dump(gc_status()['protected']);
+
+$node = new stdClass;
+$node->self = $node;
+unset($node);
+var_dump(gc_collect_cycles());
+
+for ($i = 0; $i < 100000; $i++) {
+    $node = new stdClass;
+    $node->self = $node;
+}
+unset($node);
+var_dump(gc_status()['runs'] > 1);
+gc_collect_cycles();
+var_dump(gc_status()['collected']);
+
+ini_set('opcache.enable', '0');
+gc_disable();
+var_dump(@opcache_compile_file(__DIR__ . '/gh24361.inc'));
+var_dump(gc_status()['protected']);
+var_dump(gc_enabled());
+$node = new stdClass;
+$node->self = $node;
+unset($node);
+var_dump(gc_collect_cycles());
+?>
+--EXPECTF--
+bool(false)
+
+Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2
+bool(false)
+bool(false)
+int(1)
+bool(true)
+int(100001)
+
+Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2
+bool(false)
+bool(false)
+bool(false)
+int(1)

ext/opcache/tests/gh24361_error_handler.phpt

@@ -0,0 +1,34 @@
+--TEST--
+GH-24361 (GC is restored before reporting the opcache_compile_file() warning)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.file_update_protection=0
+--FILE--
+<?php
+set_error_handler(function ($severity, $message) {
+    var_dump($severity === E_WARNING);
+    var_dump(gc_status()['protected']);
+    $node = new stdClass;
+    $node->self = $node;
+    unset($node);
+    var_dump(gc_collect_cycles());
+    throw new RuntimeException('compile warning');
+});
+
+try {
+    opcache_compile_file(__DIR__ . '/gh24361.inc');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+var_dump(gc_status()['protected']);
+?>
+--EXPECTF--
+Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2
+bool(true)
+bool(false)
+int(1)
+RuntimeException: compile warning
+bool(false)

ext/opcache/tests/gh24361_memory_limit.phpt

@@ -0,0 +1,41 @@
+--TEST--
+GH-24361 (memory exhaustion while opening a script keeps GC protected)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.file_update_protection=0
+memory_limit=16M
+--SKIPIF--
+<?php
+if (getenv('USE_ZEND_ALLOC') === '0') {
+    die('skip Zend MM disabled');
+}
+?>
+--FILE--
+<?php
+class MemoryLimitStream {
+    public $context;
+
+    public function stream_open($path, $mode, $options, &$opened_path) {
+        $memory = [];
+        for ($i = 0; $i < 64; $i++) {
+            $memory[] = str_repeat('x', 1024 * 1024);
+        }
+    }
+}
+stream_wrapper_register('memory-limit', MemoryLimitStream::class);
+register_shutdown_function(static function () {
+    var_dump(gc_status()['protected']);
+});
+
+var_dump(gc_status()['protected']);
+opcache_compile_file('memory-limit://test');
+echo "Unreachable\n";
+?>
+--EXPECTF--
+bool(false)
+
+Fatal error: Allowed memory size of %d bytes exhausted %s in %s on line %d
+bool(true)

ext/opcache/tests/gh24361_nested_fatal.phpt

@@ -0,0 +1,35 @@
+--TEST--
+GH-24361 (bailouts from user code and existing GC protection are preserved)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.file_update_protection=0
+--FILE--
+<?php
+class CompileErrorStream {
+    public $context;
+
+    public function stream_open($path, $mode, $options, &$opened_path) {
+        eval('break 2;');
+    }
+}
+stream_wrapper_register('compile-error', CompileErrorStream::class);
+
+var_dump(gc_status()['protected']);
+var_dump(@opcache_compile_file('compile-error://test'));
+var_dump(gc_status()['protected']);
+var_dump(@opcache_compile_file(__DIR__ . '/gh24361.inc'));
+var_dump(gc_status()['protected']);
+?>
+--EXPECTF--
+bool(false)
+
+Fatal error: 'break' not in the 'loop' or 'switch' context in %s on line 1
+bool(false)
+bool(true)
+
+Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2
+bool(false)
+bool(true)

ext/opcache/tests/gh24361_repeated_errors.phpt

@@ -0,0 +1,39 @@
+--TEST--
+GH-24361 (GC recovery does not rely on the last reported error)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.file_update_protection=0
+ignore_repeated_errors=1
+ignore_repeated_source=1
+display_errors=0
+log_errors=0
+--FILE--
+<?php
+class RepeatedErrorStream {
+    public $context;
+
+    public function stream_open($path, $mode, $options, &$opened_path) {
+        trigger_error("'break' not in the 'loop' or 'switch' context", E_USER_ERROR);
+    }
+}
+stream_wrapper_register('repeated-error', RepeatedErrorStream::class);
+set_error_handler(static fn() => true, E_WARNING | E_DEPRECATED);
+for ($i = 0; $i < 2; $i++) {
+    var_dump(opcache_compile_file(__DIR__ . '/gh24361.inc'));
+    var_dump(gc_status()['protected']);
+}
+var_dump(opcache_compile_file('repeated-error://test'));
+var_dump(gc_status()['protected']);
+var_dump(error_get_last()['type'] === E_COMPILE_ERROR);
+?>
+--EXPECT--
+bool(false)
+bool(false)
+bool(false)
+bool(false)
+bool(false)
+bool(true)
+bool(true)

ext/opcache/zend_accelerator_module.c

@@ -27,6 +27,7 @@
 #include "zend_closures.h"
 #include "zend_shared_alloc.h"
 #include "zend_accelerator_blacklist.h"
+#include "zend_accelerator_module.h"
 #include "php_ini.h"
 #include "SAPI.h"
 #include "zend_virtual_cwd.h"
@@ -59,6 +60,22 @@ static zif_handler orig_file_exists = NULL;
 static zif_handler orig_is_file = NULL;
 static zif_handler orig_is_readable = NULL;
 
+ZEND_TLS int accel_last_fatal_error_type;
+ZEND_TLS zend_execute_data *accel_last_fatal_error_execute_data;
+ZEND_TLS bool accel_last_fatal_error_gc_protected;
+
+void zend_accel_record_fatal_error(
+		int type, zend_string *error_filename, uint32_t error_lineno, zend_string *message)
+{
+	if (type & E_FATAL_ERRORS) {
+		/* Record the origin before zend_bailout() clears the current execution frame.
+		 * Unlike last_error_type, notifications also cover ignored repeated errors. */
+		accel_last_fatal_error_type = type;
+		accel_last_fatal_error_execute_data = EG(current_execute_data);
+		accel_last_fatal_error_gc_protected = gc_protected();
+	}
+}
+
 static int validate_api_restriction(void)
 {
 	if (ZCG(accel_directives).restrict_api && *ZCG(accel_directives).restrict_api) {
@@ -985,10 +1002,18 @@ ZEND_FUNCTION(opcache_compile_file)
 		 * state. */
 		op_array = persistent_compile_file(&handle, ZEND_INCLUDE);
 	} else {
+		accel_last_fatal_error_type = 0;
 		zend_try {
 			op_array = persistent_compile_file(&handle, ZEND_INCLUDE);
 		} zend_catch {
 			EG(current_execute_data) = orig_execute_data;
+			/* Only recover GC after a compile error in this frame, not a fatal error
+			 * that unwound user code called by a stream wrapper or another hook. */
+			if (accel_last_fatal_error_type == E_COMPILE_ERROR
+					&& accel_last_fatal_error_execute_data == orig_execute_data
+					&& !ZEND_HAS_OOM()) {
+				gc_protect(accel_last_fatal_error_gc_protected);
+			}
 			zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " could not compile file %s", ZSTR_VAL(handle.filename));
 		} zend_end_try();
 	}

ext/opcache/zend_accelerator_module.h

@@ -26,4 +26,7 @@ int start_accel_module(void);
 
 void zend_accel_override_file_functions(void);
 
+void zend_accel_record_fatal_error(
+		int type, zend_string *error_filename, uint32_t error_lineno, zend_string *message);
+
 #endif /* _ZEND_ACCELERATOR_MODULE_H */