Skip to content

Commit

Permalink
Clear recorded errors before executing shutdown functions
Browse files Browse the repository at this point in the history
Recorded errors may be attached to the wrong cached script when a fatal error
occurs during recording. This happens because the fatal error will cause a
bailout, which may prevent the recorded errors from being freed. If an other
script is compiled after bailout, or if a class is linked after bailout, the
recorded errors will be attached to it.

This change fixes this by freeing recorded errors before executing shutdown
functions.

Fixes GH-8063
  • Loading branch information
arnaud-lb committed Apr 22, 2022
1 parent 15ee285 commit f20e11c
Show file tree
Hide file tree
Showing 9 changed files with 242 additions and 94 deletions.
207 changes: 113 additions & 94 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -2775,101 +2775,113 @@ 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) {
if (is_cacheable) {
if (zend_inheritance_cache_get && zend_inheritance_cache_add) {
zend_class_entry *ret = zend_inheritance_cache_get(ce, parent, traits_and_interfaces);
if (ret) {
if (traits_and_interfaces) {
free_alloca(traits_and_interfaces, use_heap);
}
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ret;
return ret;
}

/* Make sure warnings (such as deprecations) thrown during inheritance
* will be recoreded in the inheritance cache. */
zend_begin_record_errors();
} else {
is_cacheable = 0;
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);
if (ret) {
if (traits_and_interfaces) {
free_alloca(traits_and_interfaces, use_heap);
}
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ret;
return ret;
}
proto = ce;

/* Make sure warnings (such as deprecations) thrown during inheritance
* will be recorded in the inheritance cache. */
zend_begin_record_errors();
} else {
is_cacheable = 0;
}
/* Lazy class loading */
ce = zend_lazy_class_load(ce);
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ce;
} else if (ce->ce_flags & ZEND_ACC_FILE_CACHED) {
/* Lazy class loading */
ce = zend_lazy_class_load(ce);
ce->ce_flags &= ~ZEND_ACC_FILE_CACHED;
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ce;
proto = ce;
}

if (CG(unlinked_uses)) {
zend_hash_index_del(CG(unlinked_uses), (zend_long)(zend_uintptr_t) ce);
}
zend_try {
if (ce->ce_flags & ZEND_ACC_IMMUTABLE) {
/* Lazy class loading */
ce = zend_lazy_class_load(ce);
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ce;
} else if (ce->ce_flags & ZEND_ACC_FILE_CACHED) {
/* Lazy class loading */
ce = zend_lazy_class_load(ce);
ce->ce_flags &= ~ZEND_ACC_FILE_CACHED;
zv = zend_hash_find_known_hash(CG(class_table), key);
Z_CE_P(zv) = ce;
}

orig_linking_class = CG(current_linking_class);
CG(current_linking_class) = is_cacheable ? ce : NULL;
if (CG(unlinked_uses)) {
zend_hash_index_del(CG(unlinked_uses), (zend_long)(zend_uintptr_t) ce);
}

if (ce->ce_flags & ZEND_ACC_ENUM) {
/* Only register builtin enum methods during inheritance to avoid persisting them in
* opcache. */
zend_enum_register_funcs(ce);
}
orig_linking_class = CG(current_linking_class);
CG(current_linking_class) = is_cacheable ? ce : NULL;

if (parent) {
if (!(parent->ce_flags & ZEND_ACC_LINKED)) {
add_dependency_obligation(ce, parent);
if (ce->ce_flags & ZEND_ACC_ENUM) {
/* Only register builtin enum methods during inheritance to avoid persisting them in
* opcache. */
zend_enum_register_funcs(ce);
}
zend_do_inheritance(ce, parent);
}
if (ce->num_traits) {
zend_do_bind_traits(ce, traits_and_interfaces);
}
if (ce->num_interfaces) {
/* Also copy the parent interfaces here, so we don't need to reallocate later. */
uint32_t num_parent_interfaces = parent ? parent->num_interfaces : 0;
zend_class_entry **interfaces = emalloc(
sizeof(zend_class_entry *) * (ce->num_interfaces + num_parent_interfaces));

if (num_parent_interfaces) {
memcpy(interfaces, parent->interfaces,
sizeof(zend_class_entry *) * num_parent_interfaces);
if (parent) {
if (!(parent->ce_flags & ZEND_ACC_LINKED)) {
add_dependency_obligation(ce, parent);
}
zend_do_inheritance(ce, parent);
}
if (ce->num_traits) {
zend_do_bind_traits(ce, traits_and_interfaces);
}
memcpy(interfaces + num_parent_interfaces, traits_and_interfaces + ce->num_traits,
sizeof(zend_class_entry *) * ce->num_interfaces);
if (ce->num_interfaces) {
/* Also copy the parent interfaces here, so we don't need to reallocate later. */
uint32_t num_parent_interfaces = parent ? parent->num_interfaces : 0;
zend_class_entry **interfaces = emalloc(
sizeof(zend_class_entry *) * (ce->num_interfaces + num_parent_interfaces));

zend_do_implement_interfaces(ce, interfaces);
} else if (parent && parent->num_interfaces) {
zend_do_inherit_interfaces(ce, parent);
}
if (!(ce->ce_flags & (ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT))
&& (ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS))
) {
zend_verify_abstract_class(ce);
}
if (ce->ce_flags & ZEND_ACC_ENUM) {
zend_verify_enum(ce);
}
if (num_parent_interfaces) {
memcpy(interfaces, parent->interfaces,
sizeof(zend_class_entry *) * num_parent_interfaces);
}
memcpy(interfaces + num_parent_interfaces, traits_and_interfaces + ce->num_traits,
sizeof(zend_class_entry *) * ce->num_interfaces);

/* Normally Stringable is added during compilation. However, if it is imported from a trait,
* we need to explicilty add the interface here. */
if (ce->__tostring && !(ce->ce_flags & ZEND_ACC_TRAIT)
zend_do_implement_interfaces(ce, interfaces);
} else if (parent && parent->num_interfaces) {
zend_do_inherit_interfaces(ce, parent);
}
if (!(ce->ce_flags & (ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT))
&& (ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS))
) {
zend_verify_abstract_class(ce);
}
if (ce->ce_flags & ZEND_ACC_ENUM) {
zend_verify_enum(ce);
}

/* Normally Stringable is added during compilation. However, if it is imported from a trait,
* we need to explicilty add the interface here. */
if (ce->__tostring && !(ce->ce_flags & ZEND_ACC_TRAIT)
&& !zend_class_implements_interface(ce, zend_ce_stringable)) {
ZEND_ASSERT(ce->__tostring->common.fn_flags & ZEND_ACC_TRAIT_CLONE);
ce->ce_flags |= ZEND_ACC_RESOLVED_INTERFACES;
ce->num_interfaces++;
ce->interfaces = perealloc(ce->interfaces,
sizeof(zend_class_entry *) * ce->num_interfaces, ce->type == ZEND_INTERNAL_CLASS);
ce->interfaces[ce->num_interfaces - 1] = zend_ce_stringable;
do_interface_implementation(ce, zend_ce_stringable);
}
ZEND_ASSERT(ce->__tostring->common.fn_flags & ZEND_ACC_TRAIT_CLONE);
ce->ce_flags |= ZEND_ACC_RESOLVED_INTERFACES;
ce->num_interfaces++;
ce->interfaces = perealloc(ce->interfaces,
sizeof(zend_class_entry *) * ce->num_interfaces, ce->type == ZEND_INTERNAL_CLASS);
ce->interfaces[ce->num_interfaces - 1] = zend_ce_stringable;
do_interface_implementation(ce, zend_ce_stringable);
}

zend_build_properties_info_table(ce);
} zend_catch {
/* Do not leak recorded errors to the next linked class. */
if (!orig_record_errors) {
EG(record_errors) = false;
zend_free_recorded_errors();
}
zend_bailout();
} zend_end_try();

zend_build_properties_info_table(ce);
EG(record_errors) = orig_record_errors;

if (!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE)) {
Expand Down Expand Up @@ -3038,22 +3050,29 @@ zend_class_entry *zend_try_early_bind(zend_class_entry *ce, zend_class_entry *pa
orig_linking_class = CG(current_linking_class);
CG(current_linking_class) = is_cacheable ? ce : NULL;

if (is_cacheable) {
zend_begin_record_errors();
}
zend_try{
if (is_cacheable) {
zend_begin_record_errors();
}

zend_do_inheritance_ex(ce, parent_ce, status == INHERITANCE_SUCCESS);
if (parent_ce && parent_ce->num_interfaces) {
zend_do_inherit_interfaces(ce, parent_ce);
}
zend_build_properties_info_table(ce);
if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) {
zend_verify_abstract_class(ce);
}
ZEND_ASSERT(!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE));
ce->ce_flags |= ZEND_ACC_LINKED;
zend_do_inheritance_ex(ce, parent_ce, status == INHERITANCE_SUCCESS);
if (parent_ce && parent_ce->num_interfaces) {
zend_do_inherit_interfaces(ce, parent_ce);
}
zend_build_properties_info_table(ce);
if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) {
zend_verify_abstract_class(ce);
}
ZEND_ASSERT(!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE));
ce->ce_flags |= ZEND_ACC_LINKED;

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

CG(current_linking_class) = orig_linking_class;
EG(record_errors) = false;

if (is_cacheable) {
Expand Down
31 changes: 31 additions & 0 deletions ext/opcache/tests/gh8063-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Bug GH-8063 (Opcache breaks autoloading after E_COMPILE_ERROR) 001
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.record_warnings=0
--EXTENSIONS--
opcache
--FILE--
<?php

spl_autoload_register(function ($class) {
printf("Autoloading %s\n", $class);
include __DIR__.DIRECTORY_SEPARATOR.'gh8063'.DIRECTORY_SEPARATOR.$class.'.inc';
});

register_shutdown_function(function () {
new Bar();
new Baz();
print "Finished\n";
});

new BadClass();
--EXPECTF--
Autoloading BadClass
Autoloading Foo

Fatal error: Declaration of BadClass::dummy() must be compatible with Foo::dummy(): void in %sBadClass.inc on line 5
Autoloading Bar
Autoloading Baz
Finished
31 changes: 31 additions & 0 deletions ext/opcache/tests/gh8063-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Bug GH-8063 (Opcache breaks autoloading after E_COMPILE_ERROR) 002
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.record_warnings=1
--EXTENSIONS--
opcache
--FILE--
<?php

spl_autoload_register(function ($class) {
printf("Autoloading %s\n", $class);
include __DIR__.DIRECTORY_SEPARATOR.'gh8063'.DIRECTORY_SEPARATOR.$class.'.inc';
});

register_shutdown_function(function () {
new Bar();
new Baz();
print "Finished\n";
});

new BadClass();
--EXPECTF--
Autoloading BadClass
Autoloading Foo

Fatal error: Declaration of BadClass::dummy() must be compatible with Foo::dummy(): void in %sBadClass.inc on line 5
Autoloading Bar
Autoloading Baz
Finished
30 changes: 30 additions & 0 deletions ext/opcache/tests/gh8063-003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
Bug GH-8063 (Opcache breaks autoloading after E_COMPILE_ERROR) 003
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.record_warnings=0
--EXTENSIONS--
opcache
--FILE--
<?php

spl_autoload_register(function ($class) {
printf("Autoloading %s\n", $class);
include __DIR__.DIRECTORY_SEPARATOR.'gh8063'.DIRECTORY_SEPARATOR.$class.'.inc';
});

register_shutdown_function(function () {
new Bar();
new Baz();
print "Finished\n";
});

new BadClass2();
--EXPECTF--
Autoloading BadClass2

Fatal error: Declaration of BadClass2::dummy() must be compatible with Foo2::dummy(): void in %sBadClass2.inc on line %d
Autoloading Bar
Autoloading Baz
Finished
8 changes: 8 additions & 0 deletions ext/opcache/tests/gh8063/BadClass.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

class BadClass extends Foo
{
function dummy()
{
}
}
15 changes: 15 additions & 0 deletions ext/opcache/tests/gh8063/BadClass2.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

class Foo2
{
function dummy(): void
{
}
}

class BadClass2 extends Foo2
{
function dummy()
{
}
}
3 changes: 3 additions & 0 deletions ext/opcache/tests/gh8063/Bar.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

class Bar {}
3 changes: 3 additions & 0 deletions ext/opcache/tests/gh8063/Baz.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

class Baz {}
8 changes: 8 additions & 0 deletions ext/opcache/tests/gh8063/Foo.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

class Foo
{
function dummy(): void
{
}
}

0 comments on commit f20e11c

Please sign in to comment.