Skip to content

Commit 9a1de97

Browse files
morrisonlevimorrisonlevi
committed
fix GH-20469: inheritance cache lookup with nested autoloading
unlinked_instanceof() may see a linked starting class while an ancestor in its parent chain is still unlinked. Keep the guarded traversal instead of delegating to instanceof_function(), which assumes resolved parent pointers throughout the chain. See ext/opcache/tests/gh20469.phpt.
1 parent 3064540 commit 9a1de97

3 files changed

Lines changed: 151 additions & 4 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ PHP NEWS
1717
IntlCalendar::equals(), ::before(), ::after(), and ::isEquivalentTo().
1818
(Weilin Du)
1919

20+
- Opcache:
21+
. Fixed bug GH-20469 (Crash during inheritance cache lookup with nested
22+
autoloading). (Levi Morrison)
23+
2024
- Phar:
2125
. Fixed a bypass of the magic ".phar" directory protection in
2226
Phar::addEmptyDir() for paths starting with "/.phar", while allowing

‎Zend/zend_inheritance.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,23 @@ static zend_class_entry *lookup_class(zend_class_entry *scope, zend_string *name
310310

311311
/* Instanceof that's safe to use on unlinked classes. */
312312
static bool unlinked_instanceof(zend_class_entry *ce1, const zend_class_entry *ce2) {
313+
/* Do not short-circuit to instanceof_function() here:
314+
*
315+
* if (ce1->ce_flags & ZEND_ACC_LINKED) {
316+
* return instanceof_function(ce1, ce2);
317+
* }
318+
*
319+
* See ext/opcache/tests/gh20469.phpt. During inheritance cache lookups,
320+
* autoloading may re-enter class linking in a way where ce1 itself is
321+
* linked, but a class in its parent chain is still unlinked. The normal
322+
* instanceof_function() assumes the whole parent chain is linked, and may
323+
* interpret an unresolved parent_name as a zend_class_entry (and crash).
324+
*/
325+
313326
if (ce1 == ce2) {
314327
return 1;
315328
}
316329

317-
if (ce1->ce_flags & ZEND_ACC_LINKED) {
318-
return instanceof_function(ce1, ce2);
319-
}
320-
321330
if (ce1->parent) {
322331
zend_class_entry *parent_ce;
323332
if (ce1->ce_flags & ZEND_ACC_RESOLVED_PARENT) {

‎ext/opcache/tests/gh20469.phpt‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
--TEST--
2+
GH-20469: Inheritance cache with reentrant autoloading must not crash
3+
--EXTENSIONS--
4+
opcache
5+
--CONFLICTS--
6+
server
7+
--FILE--
8+
<?php
9+
$dir = __DIR__ . '/gh20469';
10+
@mkdir($dir . '/classes', 0777, true);
11+
12+
file_put_contents($dir . '/autoload.php', <<<'PHP'
13+
<?php
14+
spl_autoload_register(function ($class) {
15+
$prefix = 'APP\\';
16+
if (strncmp($class, $prefix, strlen($prefix)) === 0) {
17+
require __DIR__ . '/classes/' . substr($class, strlen($prefix)) . '.php';
18+
}
19+
});
20+
PHP);
21+
22+
/* The dependency cycle is:
23+
* ChildOfParentBeingLinked -> ParentBeingLinked -> CovariantReturnWithTrait
24+
* -> RequiresRootReturnTrait -> ChildOfParentBeingLinked.
25+
*/
26+
file_put_contents($dir . '/test1.php', <<<'PHP'
27+
<?php
28+
require __DIR__ . '/autoload.php';
29+
echo \APP\ChildOfParentBeingLinked::SOME_CONSTANT;
30+
PHP);
31+
32+
file_put_contents($dir . '/test2.php', <<<'PHP'
33+
<?php
34+
require __DIR__ . '/autoload.php';
35+
echo \APP\ParentBeingLinked::SOME_CONSTANT;
36+
PHP);
37+
38+
file_put_contents($dir . '/classes/RootForTraitReturn.php', <<<'PHP'
39+
<?php
40+
namespace APP;
41+
42+
class RootForTraitReturn
43+
{
44+
function createResult(): BaseCovariantReturn
45+
{
46+
}
47+
}
48+
PHP);
49+
50+
file_put_contents($dir . '/classes/ParentBeingLinked.php', <<<'PHP'
51+
<?php
52+
namespace APP;
53+
54+
class ParentBeingLinked extends RootForTraitReturn
55+
{
56+
public const SOME_CONSTANT = 3;
57+
58+
function createResult(): CovariantReturnWithTrait
59+
{
60+
}
61+
}
62+
PHP);
63+
64+
file_put_contents($dir . '/classes/ChildOfParentBeingLinked.php', <<<'PHP'
65+
<?php
66+
namespace APP;
67+
68+
class ChildOfParentBeingLinked extends ParentBeingLinked
69+
{
70+
}
71+
PHP);
72+
73+
file_put_contents($dir . '/classes/BaseCovariantReturn.php', <<<'PHP'
74+
<?php
75+
namespace APP;
76+
77+
abstract class BaseCovariantReturn
78+
{
79+
}
80+
PHP);
81+
82+
file_put_contents($dir . '/classes/RequiresRootReturnTrait.php', <<<'PHP'
83+
<?php
84+
namespace APP;
85+
86+
trait RequiresRootReturnTrait
87+
{
88+
abstract function build(): RootForTraitReturn;
89+
}
90+
PHP);
91+
92+
file_put_contents($dir . '/classes/CovariantReturnWithTrait.php', <<<'PHP'
93+
<?php
94+
namespace APP;
95+
96+
class CovariantReturnWithTrait extends BaseCovariantReturn
97+
{
98+
use RequiresRootReturnTrait;
99+
100+
function build(): ChildOfParentBeingLinked
101+
{
102+
}
103+
}
104+
PHP);
105+
106+
include 'php_cli_server.inc';
107+
$ini = trim((string) getenv('TEST_PHP_EXTRA_ARGS'));
108+
$ini .= ($ini !== '' ? ' ' : '') . '-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.file_update_protection=0';
109+
php_cli_server_start($ini);
110+
111+
echo file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469/test1.php'), "\n";
112+
echo file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469/test2.php'), "\n";
113+
?>
114+
--CLEAN--
115+
<?php
116+
$dir = __DIR__ . '/gh20469';
117+
if (is_dir($dir)) {
118+
$iterator = new RecursiveIteratorIterator(
119+
new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS),
120+
RecursiveIteratorIterator::CHILD_FIRST
121+
);
122+
foreach ($iterator as $file) {
123+
if ($file->isDir()) {
124+
rmdir($file->getPathname());
125+
} else {
126+
unlink($file->getPathname());
127+
}
128+
}
129+
rmdir($dir);
130+
}
131+
?>
132+
--EXPECT--
133+
3
134+
3

0 commit comments

Comments
 (0)