Skip to content

Commit 1146bdb

Browse files
committed
Fixed bug #78989
Always operate on copies of the functions, so we don't reference temporary trait methods that have gone out of scope. This could be more efficient, but doing an allocated copy only when strictly necessary turned out to be somewhat tricky.
1 parent e197f65 commit 1146bdb

File tree

6 files changed

+107
-8
lines changed

6 files changed

+107
-8
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ PHP NEWS
77
. Fixed bug #79155 (Property nullability lost when using multiple property
88
definition). (Nikita)
99
. Fixed bug #78323 (Code 0 is returned on invalid options). (Ivan Mikheykin)
10+
. Fixed bug #78989 (Delayed variance check involving trait segfaults).
11+
(Nikita)
1012

1113
- CURL:
1214
. Fixed bug #79078 (Hypothetical use-after-free in curl_multi_add_handle()).
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Use of parent inside a class that has / has no parent (success cases)
3+
--FILE--
4+
<?php
5+
6+
// Legal: A2::parent == P2
7+
class P2 {}
8+
class A2 extends P2 {
9+
public function method(parent $x) {}
10+
}
11+
class B2 extends A2 {
12+
public function method(P2 $x) {}
13+
}
14+
15+
// Legal: B3::parent == A3 is subclass of A3::parent == P3 in covariant position
16+
class P3 {}
17+
class A3 extends P3 {
18+
public function method($x): parent {}
19+
}
20+
class B3 extends A3 {
21+
public function method($x): parent {}
22+
}
23+
24+
?>
25+
===DONE===
26+
--EXPECT--
27+
===DONE===
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Early binding should be prevented if property types cannot be checked
3+
--FILE--
4+
<?php
5+
6+
class X {}
7+
class_alias('X', 'Y');
8+
9+
class A {
10+
public X $prop;
11+
}
12+
class B extends A {
13+
public Y $prop;
14+
}
15+
16+
?>
17+
===DONE===
18+
--EXPECT--
19+
===DONE===
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Trait delayed variance check fails
3+
--FILE--
4+
<?php
5+
6+
// Taken from bug #78989.
7+
8+
class X {
9+
function method($a): A {}
10+
}
11+
trait T {
12+
function method($r): B {}
13+
}
14+
class U extends X {
15+
use T;
16+
}
17+
18+
?>
19+
--EXPECTF--
20+
Fatal error: Could not check compatibility between T::method($r): B and X::method($a): A, because class B is not available in %s on line %d
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Trait delayed variance check succeeds
3+
--FILE--
4+
<?php
5+
6+
// Taken from bug #79179.
7+
8+
spl_autoload_register(function() {
9+
interface InterfaceB extends InterfaceA {}
10+
});
11+
12+
interface InterfaceA {}
13+
14+
trait SomeTrait {
15+
abstract public function func(): ?InterfaceA;
16+
}
17+
18+
class Foo {
19+
public function func(): ?InterfaceB {}
20+
}
21+
22+
class Bar extends Foo {
23+
use SomeTrait;
24+
}
25+
26+
?>
27+
===DONE===
28+
--EXPECT--
29+
===DONE===

Zend/zend_inheritance.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,8 +2237,10 @@ typedef struct {
22372237
union {
22382238
zend_class_entry *dependency_ce;
22392239
struct {
2240-
const zend_function *parent_fn;
2241-
const zend_function *child_fn;
2240+
/* Traits may use temporary on-stack functions during inheritance checks,
2241+
* so use copies of functions here as well. */
2242+
zend_function parent_fn;
2243+
zend_function child_fn;
22422244
zend_bool always_error;
22432245
};
22442246
struct {
@@ -2292,8 +2294,8 @@ static void add_compatibility_obligation(
22922294
HashTable *obligations = get_or_init_obligations_for_class(ce);
22932295
variance_obligation *obligation = emalloc(sizeof(variance_obligation));
22942296
obligation->type = OBLIGATION_COMPATIBILITY;
2295-
obligation->child_fn = child_fn;
2296-
obligation->parent_fn = parent_fn;
2297+
obligation->child_fn = *child_fn;
2298+
obligation->parent_fn = *parent_fn;
22972299
obligation->always_error = always_error;
22982300
zend_hash_next_index_insert_ptr(obligations, obligation);
22992301
}
@@ -2324,15 +2326,15 @@ static int check_variance_obligation(zval *zv) {
23242326
} else if (obligation->type == OBLIGATION_COMPATIBILITY) {
23252327
zend_string *unresolved_class;
23262328
inheritance_status status = zend_do_perform_implementation_check(
2327-
&unresolved_class, obligation->child_fn, obligation->parent_fn);
2329+
&unresolved_class, &obligation->child_fn, &obligation->parent_fn);
23282330

23292331
if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
23302332
if (EXPECTED(status == INHERITANCE_UNRESOLVED)) {
23312333
return ZEND_HASH_APPLY_KEEP;
23322334
}
23332335
ZEND_ASSERT(status == INHERITANCE_ERROR);
23342336
emit_incompatible_method_error_or_warning(
2335-
obligation->child_fn, obligation->parent_fn, status, unresolved_class,
2337+
&obligation->child_fn, &obligation->parent_fn, status, unresolved_class,
23362338
obligation->always_error);
23372339
}
23382340
/* Either the compatibility check was successful or only threw a warning. */
@@ -2402,10 +2404,10 @@ static void report_variance_errors(zend_class_entry *ce) {
24022404
if (obligation->type == OBLIGATION_COMPATIBILITY) {
24032405
/* Just used to fetch the unresolved_class in this case. */
24042406
status = zend_do_perform_implementation_check(
2405-
&unresolved_class, obligation->child_fn, obligation->parent_fn);
2407+
&unresolved_class, &obligation->child_fn, &obligation->parent_fn);
24062408
ZEND_ASSERT(status == INHERITANCE_UNRESOLVED);
24072409
emit_incompatible_method_error_or_warning(
2408-
obligation->child_fn, obligation->parent_fn,
2410+
&obligation->child_fn, &obligation->parent_fn,
24092411
status, unresolved_class, obligation->always_error);
24102412
} else if (obligation->type == OBLIGATION_PROPERTY_COMPATIBILITY) {
24112413
emit_incompatible_property_error(obligation->child_prop, obligation->parent_prop);

0 commit comments

Comments
 (0)