Skip to content

Commit

Permalink
Fix various bugs related to DNF types
Browse files Browse the repository at this point in the history
 - GH-11958: DNF types in trait properties do not get bound properly
 - GH-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>
  • Loading branch information
3 people committed Aug 15, 2023
1 parent 0b516ae commit 02a80c5
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 35 deletions.
10 changes: 9 additions & 1 deletion NEWS
Expand Up @@ -14,6 +14,14 @@ PHP NEWS

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

- DOM:
. Fix DOMEntity field getter bugs. (nielsdos)
Expand Down Expand Up @@ -156,7 +164,7 @@ PHP NEWS

- Standard:
. Fix serialization of RC1 objects appearing in object graph twice. (ilutov)

- Streams:
. Fixed bug GH-11735 (Use-after-free when unregistering user stream wrapper
from itself). (ilutov)
Expand Down
@@ -0,0 +1,18 @@
--TEST--
Property types must be invariant
--FILE--
<?php

interface X {}
interface Y {}

class A {
public (X&Y&Z)|L $prop;
}
class B extends A {
public (X&Y)|L $prop;
}

?>
--EXPECTF--
Fatal error: Type of B::$prop must be (X&Y&Z)|L (as in class A) in %s on line %d
@@ -0,0 +1,39 @@
--TEST--
Internal trait used typed property (union type)
--EXTENSIONS--
zend_test
--FILE--
<?php

class C {
use _ZendTestTrait;
}

$o = new C();
var_dump($o);

$prop = new \ReflectionProperty(C::class, 'classUnionProp');
$union = $prop->getType();
$types = $union->getTypes();
var_dump($types, (string)$types[0], (string)$types[1]);

?>
===DONE===
--EXPECT--
object(C)#1 (1) {
["testProp"]=>
NULL
["classUnionProp"]=>
uninitialized(Traversable|Countable)
}
array(2) {
[0]=>
object(ReflectionNamedType)#4 (0) {
}
[1]=>
object(ReflectionNamedType)#5 (0) {
}
}
string(11) "Traversable"
string(9) "Countable"
===DONE===
53 changes: 30 additions & 23 deletions Zend/zend_inheritance.c
Expand Up @@ -57,20 +57,33 @@ static void ZEND_COLD emit_incompatible_method_error(
const zend_function *parent, zend_class_entry *parent_scope,
inheritance_status status);

static void zend_type_copy_ctor(zend_type *type, bool persistent) {
static void zend_type_copy_ctor(zend_type *const type, bool use_arena, bool persistent);

static void zend_type_list_copy_ctor(
zend_type *const parent_type,
bool use_arena,
bool persistent
) {
const zend_type_list *const old_list = ZEND_TYPE_LIST(*parent_type);
size_t size = ZEND_TYPE_LIST_SIZE(old_list->num_types);
zend_type_list *new_list = use_arena
? zend_arena_alloc(&CG(arena), size) : pemalloc(size, persistent);

memcpy(new_list, old_list, size);
ZEND_TYPE_SET_LIST(*parent_type, new_list);
if (use_arena) {
ZEND_TYPE_FULL_MASK(*parent_type) |= _ZEND_TYPE_ARENA_BIT;
}

zend_type *list_type;
ZEND_TYPE_LIST_FOREACH(new_list, list_type) {
zend_type_copy_ctor(list_type, use_arena, persistent);
} ZEND_TYPE_LIST_FOREACH_END();
}

static void zend_type_copy_ctor(zend_type *const type, bool use_arena, bool persistent) {
if (ZEND_TYPE_HAS_LIST(*type)) {
zend_type_list *old_list = ZEND_TYPE_LIST(*type);
size_t size = ZEND_TYPE_LIST_SIZE(old_list->num_types);
zend_type_list *new_list = ZEND_TYPE_USES_ARENA(*type)
? zend_arena_alloc(&CG(arena), size) : pemalloc(size, persistent);
memcpy(new_list, old_list, ZEND_TYPE_LIST_SIZE(old_list->num_types));
ZEND_TYPE_SET_PTR(*type, new_list);

zend_type *list_type;
ZEND_TYPE_LIST_FOREACH(new_list, list_type) {
ZEND_ASSERT(ZEND_TYPE_HAS_NAME(*list_type));
zend_string_addref(ZEND_TYPE_NAME(*list_type));
} ZEND_TYPE_LIST_FOREACH_END();
zend_type_list_copy_ctor(type, use_arena, persistent);
} else if (ZEND_TYPE_HAS_NAME(*type)) {
zend_string_addref(ZEND_TYPE_NAME(*type));
}
Expand Down Expand Up @@ -2401,7 +2414,8 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent
doc_comment = property_info->doc_comment ? zend_string_copy(property_info->doc_comment) : NULL;

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

if (property_info->attributes) {
Expand Down Expand Up @@ -2789,15 +2803,8 @@ static zend_class_entry *zend_lazy_class_load(zend_class_entry *pce)
Z_PTR(p->val) = new_prop_info;
memcpy(new_prop_info, prop_info, sizeof(zend_property_info));
new_prop_info->ce = ce;
if (ZEND_TYPE_HAS_LIST(new_prop_info->type)) {
zend_type_list *new_list;
zend_type_list *list = ZEND_TYPE_LIST(new_prop_info->type);

new_list = zend_arena_alloc(&CG(arena), ZEND_TYPE_LIST_SIZE(list->num_types));
memcpy(new_list, list, ZEND_TYPE_LIST_SIZE(list->num_types));
ZEND_TYPE_SET_PTR(new_prop_info->type, list);
ZEND_TYPE_FULL_MASK(new_prop_info->type) |= _ZEND_TYPE_ARENA_BIT;
}
/* Deep copy the type information */
zend_type_copy_ctor(&new_prop_info->type, /* use_arena */ true, /* persistent */ false);
}
}

Expand Down
12 changes: 2 additions & 10 deletions Zend/zend_opcode.c
Expand Up @@ -110,17 +110,9 @@ ZEND_API void destroy_zend_function(zend_function *function)

ZEND_API void zend_type_release(zend_type type, bool persistent) {
if (ZEND_TYPE_HAS_LIST(type)) {
zend_type *list_type, *sublist_type;
zend_type *list_type;
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(type), list_type) {
if (ZEND_TYPE_HAS_LIST(*list_type)) {
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*list_type), sublist_type) {
if (ZEND_TYPE_HAS_NAME(*sublist_type)) {
zend_string_release(ZEND_TYPE_NAME(*sublist_type));
}
} ZEND_TYPE_LIST_FOREACH_END();
} else if (ZEND_TYPE_HAS_NAME(*list_type)) {
zend_string_release(ZEND_TYPE_NAME(*list_type));
}
zend_type_release(*list_type, persistent);
} ZEND_TYPE_LIST_FOREACH_END();
if (!ZEND_TYPE_USES_ARENA(type)) {
pefree(ZEND_TYPE_LIST(type), persistent);
Expand Down
1 change: 1 addition & 0 deletions ext/zend_test/test.stub.php
Expand Up @@ -55,6 +55,7 @@ public function returnsThrowable(): Exception {}
trait _ZendTestTrait {
/** @var mixed */
public $testProp;
public Traversable|Countable $classUnionProp;

public function testMethod(): bool {}
}
Expand Down
15 changes: 14 additions & 1 deletion ext/zend_test/test_arginfo.h

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

0 comments on commit 02a80c5

Please sign in to comment.