Skip to content

Commit 82b29ed

Browse files
committed
Fix EG(record_errors) leak when compilation bails out
Since 7b3e68f, a bailout in compile_file() or in the opcache optimizer leaves EG(record_errors) true. The next compilation fails the assertion "Error recording already enabled", and PHP does not show later warnings. Add a zend_catch block to these three regions, as opcache_compile_file() does. Add zend_test.fatal_error_in_pass to do a test of the optimizer.
1 parent beee544 commit 82b29ed

10 files changed

Lines changed: 117 additions & 3 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ PHP NEWS
2424
the end of their mapping. (Ilia Alshanetsky)
2525
. Fixed exception thrown by destructor during GC in a Fiber not being rethrown
2626
into the frame that triggered the GC. (Nicolas Grekas)
27+
. Fixed warnings being silently dropped after a fatal error during
28+
compilation or optimization. (Pierre Tondereau)
2729

2830
- CLI
2931
. Fix GH-22567 (Windows ZTS CLI SAPI should refresh its TSRMLS cache during
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
class A {
3+
function f() {}
4+
function f() {}
5+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--TEST--
2+
Error recording is disabled after a fatal error during compilation
3+
--FILE--
4+
<?php
5+
register_shutdown_function(function () {
6+
trigger_error('From shutdown', E_USER_WARNING);
7+
});
8+
require __DIR__ . '/record_errors_compile_bailout.inc';
9+
?>
10+
--EXPECTF--
11+
Fatal error: Cannot redeclare A::f() in %srecord_errors_compile_bailout.inc on line %d
12+
13+
Warning: From shutdown in %s on line %d

‎Zend/zend_language_scanner.l‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,15 @@ ZEND_API zend_op_array *compile_file(zend_file_handle *file_handle, int type)
657657
zend_begin_record_errors();
658658
}
659659

660-
op_array = zend_compile(ZEND_USER_FUNCTION);
660+
zend_try {
661+
op_array = zend_compile(ZEND_USER_FUNCTION);
662+
} zend_catch {
663+
if (!orig_record_errors) {
664+
EG(record_errors) = false;
665+
zend_free_recorded_errors();
666+
}
667+
zend_bailout();
668+
} zend_end_try();
661669

662670
if (!orig_record_errors) {
663671
zend_emit_recorded_errors();

‎ext/opcache/ZendAccelerator.c‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,7 +1956,13 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int
19561956
}
19571957

19581958
from_memory = false;
1959-
persistent_script = cache_script_in_file_cache(persistent_script, &from_memory);
1959+
zend_try {
1960+
persistent_script = cache_script_in_file_cache(persistent_script, &from_memory);
1961+
} zend_catch {
1962+
EG(record_errors) = false;
1963+
zend_free_recorded_errors();
1964+
zend_bailout();
1965+
} zend_end_try();
19601966

19611967
zend_emit_recorded_errors();
19621968
zend_free_recorded_errors();
@@ -2182,7 +2188,16 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
21822188

21832189
/* See GH-17246: we disable GC so that user code cannot be executed during the optimizer run. */
21842190
bool orig_gc_state = gc_enable(false);
2185-
persistent_script = cache_script_in_shared_memory(persistent_script, key, &from_shared_memory);
2191+
zend_try {
2192+
persistent_script = cache_script_in_shared_memory(persistent_script, key, &from_shared_memory);
2193+
} zend_catch {
2194+
gc_enable(orig_gc_state);
2195+
SHM_PROTECT();
2196+
HANDLE_UNBLOCK_INTERRUPTIONS();
2197+
EG(record_errors) = false;
2198+
zend_free_recorded_errors();
2199+
zend_bailout();
2200+
} zend_end_try();
21862201
gc_enable(orig_gc_state);
21872202
}
21882203

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
<?php
2+
function f() {}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
Error recording is disabled after a fatal error during the optimizer with opcache.file_cache_only
3+
--EXTENSIONS--
4+
opcache
5+
zend_test
6+
--SKIPIF--
7+
<?php
8+
if (getenv("SKIP_REPEAT")) die("skip Not compatible with repeat");
9+
if (getenv("SKIP_PRELOAD")) die("skip Not compatible with preload");
10+
?>
11+
--INI--
12+
opcache.enable=1
13+
opcache.enable_cli=1
14+
zend_test.register_passes=1
15+
opcache.file_cache="{TMP}"
16+
opcache.file_cache_only=1
17+
--FILE--
18+
<?php
19+
register_shutdown_function(function () {
20+
trigger_error('From shutdown', E_USER_WARNING);
21+
});
22+
ini_set('zend_test.fatal_error_in_pass', '1');
23+
require __DIR__ . '/bailout.inc';
24+
?>
25+
--EXPECTF--
26+
%Apass1
27+
28+
Fatal error: Fatal error in pass1 in %s on line %d
29+
30+
Warning: From shutdown in %s on line %d
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
Error recording is disabled after a fatal error during the optimizer
3+
--EXTENSIONS--
4+
opcache
5+
zend_test
6+
--SKIPIF--
7+
<?php
8+
if (getenv("SKIP_REPEAT")) die("skip Not compatible with repeat");
9+
if (getenv("SKIP_PRELOAD")) die("skip Not compatible with preload");
10+
?>
11+
--INI--
12+
opcache.enable=1
13+
opcache.enable_cli=1
14+
zend_test.register_passes=1
15+
opcache.file_cache=
16+
opcache.file_cache_only=0
17+
--FILE--
18+
<?php
19+
register_shutdown_function(function () {
20+
var_dump(gc_enabled());
21+
trigger_error('From shutdown', E_USER_WARNING);
22+
});
23+
ini_set('zend_test.fatal_error_in_pass', '1');
24+
require __DIR__ . '/bailout.inc';
25+
?>
26+
--EXPECTF--
27+
pass1
28+
pass2
29+
pass1
30+
31+
Fatal error: Fatal error in pass1 in %s on line %d
32+
bool(true)
33+
34+
Warning: From shutdown in %s on line %d

‎ext/zend_test/php_test.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
5555
HashTable *global_weakmap;
5656
int replace_zend_execute_ex;
5757
int register_passes;
58+
bool fatal_error_in_pass;
5859
bool print_stderr_mshutdown;
5960
zend_long limit_copy_file_range;
6061
int observe_opline_in_zendmm;

‎ext/zend_test/test.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ static ZEND_FUNCTION(zend_test_void_return)
120120
static void pass1(zend_script *script, void *context)
121121
{
122122
php_printf("pass1\n");
123+
if (ZT_G(fatal_error_in_pass)) {
124+
zend_error_noreturn(E_ERROR, "Fatal error in pass1");
125+
}
123126
}
124127

125128
static void pass2(zend_script *script, void *context)
@@ -1328,6 +1331,7 @@ static ZEND_METHOD(_ZendTestMagicCallForward, __call)
13281331
PHP_INI_BEGIN()
13291332
STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals)
13301333
STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals)
1334+
STD_PHP_INI_BOOLEAN("zend_test.fatal_error_in_pass", "0", PHP_INI_ALL, OnUpdateBool, fatal_error_in_pass, zend_zend_test_globals, zend_test_globals)
13311335
STD_PHP_INI_BOOLEAN("zend_test.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals)
13321336
#ifdef HAVE_COPY_FILE_RANGE
13331337
STD_PHP_INI_ENTRY("zend_test.limit_copy_file_range", "-1", PHP_INI_ALL, OnUpdateLong, limit_copy_file_range, zend_zend_test_globals, zend_test_globals)

0 commit comments

Comments
 (0)