Skip to content

Commit 02a80c5

Browse files
Girgiasiluuu1994ju1ius
committed
Fix various bugs related to DNF types
 - phpGH-11958: DNF types in trait properties do not get bound properly - phpGH-11883: Memory leak in zend_type_release() for non-arena allocated DNF types - Internal trait bound to userland class would not be arena allocated - Property DNF types were not properly deep copied during lazy loading Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com> Co-authored-by: ju1ius <jules.bernable@gmail.com>
1 parent 0b516ae commit 02a80c5

File tree

7 files changed

+113
-35
lines changed

7 files changed

+113
-35
lines changed

‎NEWS‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ PHP NEWS
1414

1515
- Core:
1616
. Fixed strerror_r detection at configuration time. (Kévin Dunglas)
17+
. Fixed trait typed properties using a DNF type not being correctly bound.
18+
(Girgias)
19+
. Fixed trait property types not being arena allocated if copied from
20+
an internal trait. (Girgias)
21+
. Fixed deep copy of property DNF type during lazy class load.
22+
(Girgias, ilutov)
23+
. Fixed memory freeing of DNF types for non arena allocated types.
24+
(Girgias, ju1ius)
1725

1826
- DOM:
1927
. Fix DOMEntity field getter bugs. (nielsdos)
@@ -156,7 +164,7 @@ PHP NEWS
156164

157165
- Standard:
158166
. Fix serialization of RC1 objects appearing in object graph twice. (ilutov)
159-
167+
160168
- Streams:
161169
. Fixed bug GH-11735 (Use-after-free when unregistering user stream wrapper
162170
from itself). (ilutov)

‎Zend/tests/type_declarations/dnf_types/variance/invalid_invariance1_var.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Property types must be invariant
3+
--FILE--
4+
<?php
5+
6+
interface X {}
7+
interface Y {}
8+
9+
class A {
10+
public (X&Y&Z)|L $prop;
11+
}
12+
class B extends A {
13+
public (X&Y)|L $prop;
14+
}
15+
16+
?>
17+
--EXPECTF--
18+
Fatal error: Type of B::$prop must be (X&Y&Z)|L (as in class A) in %s on line %d

‎Zend/tests/type_declarations/union_types/internal_trait_use_typed_union.phpt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Internal trait used typed property (union type)
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
8+
class C {
9+
use _ZendTestTrait;
10+
}
11+
12+
$o = new C();
13+
var_dump($o);
14+
15+
$prop = new \ReflectionProperty(C::class, 'classUnionProp');
16+
$union = $prop->getType();
17+
$types = $union->getTypes();
18+
var_dump($types, (string)$types[0], (string)$types[1]);
19+
20+
?>
21+
===DONE===
22+
--EXPECT--
23+
object(C)#1 (1) {
24+
["testProp"]=>
25+
NULL
26+
["classUnionProp"]=>
27+
uninitialized(Traversable|Countable)
28+
}
29+
array(2) {
30+
[0]=>
31+
object(ReflectionNamedType)#4 (0) {
32+
}
33+
[1]=>
34+
object(ReflectionNamedType)#5 (0) {
35+
}
36+
}
37+
string(11) "Traversable"
38+
string(9) "Countable"
39+
===DONE===

‎Zend/zend_inheritance.c‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,33 @@ static void ZEND_COLD emit_incompatible_method_error(
5757
const zend_function *parent, zend_class_entry *parent_scope,
5858
inheritance_status status);
5959

60-
static void zend_type_copy_ctor(zend_type *type, bool persistent) {
60+
static void zend_type_copy_ctor(zend_type *const type, bool use_arena, bool persistent);
61+
62+
static void zend_type_list_copy_ctor(
63+
zend_type *const parent_type,
64+
bool use_arena,
65+
bool persistent
66+
) {
67+
const zend_type_list *const old_list = ZEND_TYPE_LIST(*parent_type);
68+
size_t size = ZEND_TYPE_LIST_SIZE(old_list->num_types);
69+
zend_type_list *new_list = use_arena
70+
? zend_arena_alloc(&CG(arena), size) : pemalloc(size, persistent);
71+
72+
memcpy(new_list, old_list, size);
73+
ZEND_TYPE_SET_LIST(*parent_type, new_list);
74+
if (use_arena) {
75+
ZEND_TYPE_FULL_MASK(*parent_type) |= _ZEND_TYPE_ARENA_BIT;
76+
}
77+
78+
zend_type *list_type;
79+
ZEND_TYPE_LIST_FOREACH(new_list, list_type) {
80+
zend_type_copy_ctor(list_type, use_arena, persistent);
81+
} ZEND_TYPE_LIST_FOREACH_END();
82+
}
83+
84+
static void zend_type_copy_ctor(zend_type *const type, bool use_arena, bool persistent) {
6185
if (ZEND_TYPE_HAS_LIST(*type)) {
62-
zend_type_list *old_list = ZEND_TYPE_LIST(*type);
63-
size_t size = ZEND_TYPE_LIST_SIZE(old_list->num_types);
64-
zend_type_list *new_list = ZEND_TYPE_USES_ARENA(*type)
65-
? zend_arena_alloc(&CG(arena), size) : pemalloc(size, persistent);
66-
memcpy(new_list, old_list, ZEND_TYPE_LIST_SIZE(old_list->num_types));
67-
ZEND_TYPE_SET_PTR(*type, new_list);
68-
69-
zend_type *list_type;
70-
ZEND_TYPE_LIST_FOREACH(new_list, list_type) {
71-
ZEND_ASSERT(ZEND_TYPE_HAS_NAME(*list_type));
72-
zend_string_addref(ZEND_TYPE_NAME(*list_type));
73-
} ZEND_TYPE_LIST_FOREACH_END();
86+
zend_type_list_copy_ctor(type, use_arena, persistent);
7487
} else if (ZEND_TYPE_HAS_NAME(*type)) {
7588
zend_string_addref(ZEND_TYPE_NAME(*type));
7689
}
@@ -2401,7 +2414,8 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent
24012414
doc_comment = property_info->doc_comment ? zend_string_copy(property_info->doc_comment) : NULL;
24022415

24032416
zend_type type = property_info->type;
2404-
zend_type_copy_ctor(&type, /* persistent */ 0);
2417+
/* Assumption: only userland classes can use traits, as such the type must be arena allocated */
2418+
zend_type_copy_ctor(&type, /* use arena */ true, /* persistent */ false);
24052419
new_prop = zend_declare_typed_property(ce, prop_name, prop_value, flags, doc_comment, type);
24062420

24072421
if (property_info->attributes) {
@@ -2789,15 +2803,8 @@ static zend_class_entry *zend_lazy_class_load(zend_class_entry *pce)
27892803
Z_PTR(p->val) = new_prop_info;
27902804
memcpy(new_prop_info, prop_info, sizeof(zend_property_info));
27912805
new_prop_info->ce = ce;
2792-
if (ZEND_TYPE_HAS_LIST(new_prop_info->type)) {
2793-
zend_type_list *new_list;
2794-
zend_type_list *list = ZEND_TYPE_LIST(new_prop_info->type);
2795-
2796-
new_list = zend_arena_alloc(&CG(arena), ZEND_TYPE_LIST_SIZE(list->num_types));
2797-
memcpy(new_list, list, ZEND_TYPE_LIST_SIZE(list->num_types));
2798-
ZEND_TYPE_SET_PTR(new_prop_info->type, list);
2799-
ZEND_TYPE_FULL_MASK(new_prop_info->type) |= _ZEND_TYPE_ARENA_BIT;
2800-
}
2806+
/* Deep copy the type information */
2807+
zend_type_copy_ctor(&new_prop_info->type, /* use_arena */ true, /* persistent */ false);
28012808
}
28022809
}
28032810

‎Zend/zend_opcode.c‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,9 @@ ZEND_API void destroy_zend_function(zend_function *function)
110110

111111
ZEND_API void zend_type_release(zend_type type, bool persistent) {
112112
if (ZEND_TYPE_HAS_LIST(type)) {
113-
zend_type *list_type, *sublist_type;
113+
zend_type *list_type;
114114
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(type), list_type) {
115-
if (ZEND_TYPE_HAS_LIST(*list_type)) {
116-
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*list_type), sublist_type) {
117-
if (ZEND_TYPE_HAS_NAME(*sublist_type)) {
118-
zend_string_release(ZEND_TYPE_NAME(*sublist_type));
119-
}
120-
} ZEND_TYPE_LIST_FOREACH_END();
121-
} else if (ZEND_TYPE_HAS_NAME(*list_type)) {
122-
zend_string_release(ZEND_TYPE_NAME(*list_type));
123-
}
115+
zend_type_release(*list_type, persistent);
124116
} ZEND_TYPE_LIST_FOREACH_END();
125117
if (!ZEND_TYPE_USES_ARENA(type)) {
126118
pefree(ZEND_TYPE_LIST(type), persistent);

‎ext/zend_test/test.stub.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public function returnsThrowable(): Exception {}
5555
trait _ZendTestTrait {
5656
/** @var mixed */
5757
public $testProp;
58+
public Traversable|Countable $classUnionProp;
5859

5960
public function testMethod(): bool {}
6061
}

‎ext/zend_test/test_arginfo.h‎

Lines changed: 14 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)