Skip to content

Fix error handling during opcache compilation #18541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions .github/scripts/windows/test_task.bat
Original file line number Diff line number Diff line change
Expand Up @@ -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
if "%OPCACHE%" equ "1" echo zend_extension=php_opcache.dll >> %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
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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"
}
7 changes: 5 additions & 2 deletions Zend/tests/inheritance/gh15907.phpt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one fails when OPcache is not loaded. It still says the “During inheritance of C” bit. Thus:

We could apply the same behavior when opcache is disabled.

This would make sense to me.

Original file line number Diff line number Diff line change
Expand Up @@ -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
41 changes: 37 additions & 4 deletions Zend/zend.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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)) {
Expand Down
1 change: 1 addition & 0 deletions Zend/zend.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
1 change: 0 additions & 1 deletion Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion Zend/zend_globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
38 changes: 23 additions & 15 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -3546,8 +3543,6 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
}
#endif

bool orig_record_errors = EG(record_errors);

if (ce->ce_flags & ZEND_ACC_IMMUTABLE && is_cacheable) {
if (zend_inheritance_cache_get && zend_inheritance_cache_add) {
zend_class_entry *ret = zend_inheritance_cache_get(ce, parent, traits_and_interfaces);
Expand All @@ -3559,16 +3554,21 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
Z_CE_P(zv) = ret;
return ret;
}

/* Make sure warnings (such as deprecations) thrown during inheritance
* will be recorded in the inheritance cache. */
zend_begin_record_errors();
} else {
is_cacheable = 0;
}
proto = ce;
}

/* Delay and record warnings (such as deprecations) thrown during
* inheritance, so they will be recorded in the inheritance cache.
* Warnings must be delayed in all cases so that we get a consistent
* behavior regardless of cacheability. */
bool orig_record_errors = EG(record_errors);
if (!orig_record_errors) {
zend_begin_record_errors();
}

zend_try {
if (ce->ce_flags & ZEND_ACC_IMMUTABLE) {
/* Lazy class loading */
Expand Down Expand Up @@ -3759,6 +3759,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) {
Expand Down Expand Up @@ -3919,10 +3920,12 @@ ZEND_API zend_class_entry *zend_try_early_bind(zend_class_entry *ce, zend_class_
orig_linking_class = CG(current_linking_class);
CG(current_linking_class) = is_cacheable ? ce : NULL;

bool orig_record_errors = EG(record_errors);

zend_try{
CG(zend_lineno) = ce->info.user.line_start;

if (is_cacheable) {
if (!orig_record_errors) {
zend_begin_record_errors();
}

Expand All @@ -3944,13 +3947,13 @@ ZEND_API zend_class_entry *zend_try_early_bind(zend_class_entry *ce, zend_class_

CG(current_linking_class) = orig_linking_class;
} zend_catch {
EG(record_errors) = false;
zend_free_recorded_errors();
if (!orig_record_errors) {
EG(record_errors) = false;
zend_free_recorded_errors();
}
zend_bailout();
} zend_end_try();

EG(record_errors) = false;

if (is_cacheable) {
HashTable *ht = (HashTable*)ce->inheritance_cache;
zend_class_entry *new_ce;
Expand All @@ -3968,6 +3971,11 @@ ZEND_API zend_class_entry *zend_try_early_bind(zend_class_entry *ce, zend_class_
}
}

if (!orig_record_errors) {
zend_emit_recorded_errors();
zend_free_recorded_errors();
}

if (ZSTR_HAS_CE_CACHE(ce->name)) {
ZSTR_SET_CE_CACHE(ce->name, ce);
}
Expand Down
10 changes: 10 additions & 0 deletions Zend/zend_language_scanner.l
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,17 @@ ZEND_API zend_op_array *compile_file(zend_file_handle *file_handle, int type)
}
}
} else {
bool orig_record_errors = EG(record_errors);
if (!orig_record_errors) {
zend_begin_record_errors();
}

op_array = zend_compile(ZEND_USER_FUNCTION);

if (!orig_record_errors) {
zend_emit_recorded_errors();
zend_free_recorded_errors();
}
}

zend_restore_lexical_state(&original_lex_state);
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand All @@ -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();
}
}
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_vm_execute.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading