diff --git a/Zend/tests/property_hooks/dump.phpt b/Zend/tests/property_hooks/dump.phpt index d7cd57183d63f..84db6cb3175b0 100644 --- a/Zend/tests/property_hooks/dump.phpt +++ b/Zend/tests/property_hooks/dump.phpt @@ -34,7 +34,7 @@ class Test { } class Child extends Test { - public $addedHooks { + public $addedHooks = 'addedHooks' { get { return strtoupper(parent::$addedHooks::get()); } } private $changed = 'changed Child' { diff --git a/Zend/tests/property_hooks/generator_hook_002.phpt b/Zend/tests/property_hooks/generator_hook_002.phpt index 0eeb99c8cf9b4..8855dc092fa1c 100644 --- a/Zend/tests/property_hooks/generator_hook_002.phpt +++ b/Zend/tests/property_hooks/generator_hook_002.phpt @@ -10,6 +10,7 @@ class A { class B extends A { public $prop { get { + yield parent::$prop::get(); yield parent::$prop::get() + 1; yield parent::$prop::get() + 2; yield parent::$prop::get() + 3; @@ -24,6 +25,7 @@ foreach ($b->prop as $value) { ?> --EXPECT-- -int(43) -int(44) -int(45) +NULL +int(1) +int(2) +int(3) diff --git a/Zend/tests/property_hooks/gh17376.phpt b/Zend/tests/property_hooks/gh17376.phpt new file mode 100644 index 0000000000000..d0896ff4599e4 --- /dev/null +++ b/Zend/tests/property_hooks/gh17376.phpt @@ -0,0 +1,119 @@ +--TEST-- +GH-17376: Child classes may add hooks to plain properties +--INI-- +# Avoid triggering for main, trigger for test so we get a side-trace for property hooks +opcache.jit_hot_func=2 +--FILE-- +prop; + } + set { + echo __METHOD__, "\n"; + $this->prop = $value; + } + } +} + +function test(A $a) { + echo "read\n"; + var_dump($a->prop); + echo "write\n"; + $a->prop = 42; + echo "read-write\n"; + $a->prop += 43; + echo "pre-inc\n"; + ++$a->prop; + echo "pre-dec\n"; + --$a->prop; + echo "post-inc\n"; + $a->prop++; + echo "post-dec\n"; + $a->prop--; +} + +echo "A\n"; +test(new A); + +echo "\nA\n"; +test(new A); + +echo "\nB\n"; +test(new B); + +echo "\nB\n"; +test(new B); + +?> +--EXPECT-- +A +read +int(1) +write +read-write +pre-inc +pre-dec +post-inc +post-dec + +A +read +int(1) +write +read-write +pre-inc +pre-dec +post-inc +post-dec + +B +read +B::$prop::get +int(1) +write +B::$prop::set +read-write +B::$prop::get +B::$prop::set +pre-inc +B::$prop::get +B::$prop::set +pre-dec +B::$prop::get +B::$prop::set +post-inc +B::$prop::get +B::$prop::set +post-dec +B::$prop::get +B::$prop::set + +B +read +B::$prop::get +int(1) +write +B::$prop::set +read-write +B::$prop::get +B::$prop::set +pre-inc +B::$prop::get +B::$prop::set +pre-dec +B::$prop::get +B::$prop::set +post-inc +B::$prop::get +B::$prop::set +post-dec +B::$prop::get +B::$prop::set diff --git a/Zend/tests/property_hooks/parent_get_plain.phpt b/Zend/tests/property_hooks/parent_get_plain.phpt index f461b39a8a372..c3dc72d51fee8 100644 --- a/Zend/tests/property_hooks/parent_get_plain.phpt +++ b/Zend/tests/property_hooks/parent_get_plain.phpt @@ -8,7 +8,7 @@ class P { } class C extends P { - public $prop { + public $prop = 42 { get => parent::$prop::get(); } } diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 1eaf3ef686e79..e0632d0ffa34d 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -463,7 +463,10 @@ typedef struct _zend_property_info { #define OBJ_PROP_TO_OFFSET(num) \ ((uint32_t)(XtOffsetOf(zend_object, properties_table) + sizeof(zval) * (num))) #define OBJ_PROP_TO_NUM(offset) \ - ((offset - OBJ_PROP_TO_OFFSET(0)) / sizeof(zval)) + (((offset) - OBJ_PROP_TO_OFFSET(0)) / sizeof(zval)) + +#define Z_PROP_TABLE_OFFSET(prop_info) \ + OBJ_PROP_TO_NUM(!((prop_info)->prototype->flags & ZEND_ACC_VIRTUAL) ? (prop_info)->prototype->offset : (prop_info)->offset) typedef struct _zend_class_constant { zval value; /* flags are stored in u2 */ diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 9a1314682cee6..7f68d4b77df78 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1478,17 +1478,38 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::$%s must be %s (as in class %s)%s", ZSTR_VAL(ce->name), ZSTR_VAL(key), zend_visibility_string(parent_info->flags), ZSTR_VAL(parent_info->ce->name), (parent_info->flags&ZEND_ACC_PUBLIC) ? "" : " or weaker"); } if (!(child_info->flags & ZEND_ACC_STATIC) && !(parent_info->flags & ZEND_ACC_VIRTUAL)) { + /* If we added props to the child property, we use the childs slot for + * storage to keep the parent slot set to null. This automatically picks + * the slow path in the JIT. */ + bool use_child_prop = !parent_info->hooks && child_info->hooks; + + if (use_child_prop && child_info->offset == ZEND_VIRTUAL_PROPERTY_OFFSET) { + child_info->offset = OBJ_PROP_TO_OFFSET(ce->default_properties_count); + ce->default_properties_count++; + ce->default_properties_table = perealloc(ce->default_properties_table, sizeof(zval) * ce->default_properties_count, ce->type == ZEND_INTERNAL_CLASS); + zval *property_default_ptr = &ce->default_properties_table[OBJ_PROP_TO_NUM(child_info->offset)]; + ZVAL_UNDEF(property_default_ptr); + Z_PROP_FLAG_P(property_default_ptr) = IS_PROP_UNINIT; + } + if (child_info->offset != ZEND_VIRTUAL_PROPERTY_OFFSET) { int parent_num = OBJ_PROP_TO_NUM(parent_info->offset); - int child_num = OBJ_PROP_TO_NUM(child_info->offset); /* Don't keep default properties in GC (they may be freed by opcache) */ zval_ptr_dtor_nogc(&(ce->default_properties_table[parent_num])); - ce->default_properties_table[parent_num] = ce->default_properties_table[child_num]; - ZVAL_UNDEF(&ce->default_properties_table[child_num]); + + if (use_child_prop) { + ZVAL_UNDEF(&ce->default_properties_table[parent_num]); + } else { + int child_num = OBJ_PROP_TO_NUM(child_info->offset); + ce->default_properties_table[parent_num] = ce->default_properties_table[child_num]; + ZVAL_UNDEF(&ce->default_properties_table[child_num]); + } } - child_info->offset = parent_info->offset; + if (!use_child_prop) { + child_info->offset = parent_info->offset; + } child_info->flags &= ~ZEND_ACC_VIRTUAL; } @@ -1663,7 +1684,7 @@ void zend_build_properties_info_table(zend_class_entry *ce) ZEND_HASH_MAP_FOREACH_PTR(&ce->properties_info, prop) { if (prop->ce == ce && (prop->flags & ZEND_ACC_STATIC) == 0 && !(prop->flags & ZEND_ACC_VIRTUAL)) { - table[OBJ_PROP_TO_NUM(prop->offset)] = prop; + table[Z_PROP_TABLE_OFFSET(prop)] = prop; } } ZEND_HASH_FOREACH_END(); } @@ -1677,8 +1698,12 @@ ZEND_API void zend_verify_hooked_property(zend_class_entry *ce, zend_property_in /* We specified a default value (otherwise offset would be -1), but the virtual flag wasn't * removed during inheritance. */ if ((prop_info->flags & ZEND_ACC_VIRTUAL) && prop_info->offset != ZEND_VIRTUAL_PROPERTY_OFFSET) { - zend_error_noreturn(E_COMPILE_ERROR, - "Cannot specify default value for virtual hooked property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(prop_name)); + if (Z_TYPE(ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)]) == IS_UNDEF) { + prop_info->offset = ZEND_VIRTUAL_PROPERTY_OFFSET; + } else { + zend_error_noreturn(E_COMPILE_ERROR, + "Cannot specify default value for virtual hooked property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(prop_name)); + } } /* If the property turns backed during inheritance and no type and default value are set, we want * the default value to be null. */ diff --git a/Zend/zend_lazy_objects.c b/Zend/zend_lazy_objects.c index e6dd6144d4b5c..41ba3e1cd288d 100644 --- a/Zend/zend_lazy_objects.c +++ b/Zend/zend_lazy_objects.c @@ -806,7 +806,11 @@ zend_property_info *zend_lazy_object_get_property_info_for_slot(zend_object *obj zend_property_info **table = obj->ce->properties_info_table; intptr_t prop_num = slot - obj->properties_table; if (prop_num >= 0 && prop_num < obj->ce->default_properties_count) { - return table[prop_num]; + if (table[prop_num]) { + return table[prop_num]; + } else { + return zend_get_property_info_for_slot_slow(obj, slot); + } } if (!zend_lazy_object_initialized(obj)) { diff --git a/Zend/zend_objects_API.c b/Zend/zend_objects_API.c index 8a6b714c8b3fd..1ba250bec6439 100644 --- a/Zend/zend_objects_API.c +++ b/Zend/zend_objects_API.c @@ -200,3 +200,15 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object) /* {{{ * } } /* }}} */ + +ZEND_API ZEND_COLD zend_property_info *zend_get_property_info_for_slot_slow(zend_object *obj, zval *slot) +{ + uintptr_t offset = (uintptr_t)slot - (uintptr_t)obj->properties_table; + zend_property_info *prop_info; + ZEND_HASH_MAP_FOREACH_PTR(&obj->ce->properties_info, prop_info) { + if (prop_info->offset == offset) { + return prop_info; + } + } ZEND_HASH_FOREACH_END(); + return NULL; +} diff --git a/Zend/zend_objects_API.h b/Zend/zend_objects_API.h index 02326f2b31460..242bf212ba9c6 100644 --- a/Zend/zend_objects_API.h +++ b/Zend/zend_objects_API.h @@ -96,6 +96,8 @@ static zend_always_inline void *zend_object_alloc(size_t obj_size, zend_class_en return obj; } +ZEND_API ZEND_COLD zend_property_info *zend_get_property_info_for_slot_slow(zend_object *obj, zval *slot); + /* Use when 'slot' was obtained directly from obj->properties_table, or when * 'obj' can not be lazy. Otherwise, use zend_get_property_info_for_slot(). */ static inline zend_property_info *zend_get_property_info_for_slot_self(zend_object *obj, zval *slot) @@ -103,7 +105,11 @@ static inline zend_property_info *zend_get_property_info_for_slot_self(zend_obje zend_property_info **table = obj->ce->properties_info_table; intptr_t prop_num = slot - obj->properties_table; ZEND_ASSERT(prop_num >= 0 && prop_num < obj->ce->default_properties_count); - return table[prop_num]; + if (table[prop_num]) { + return table[prop_num]; + } else { + return zend_get_property_info_for_slot_slow(obj, slot); + } } static inline zend_property_info *zend_get_property_info_for_slot(zend_object *obj, zval *slot) @@ -114,7 +120,11 @@ static inline zend_property_info *zend_get_property_info_for_slot(zend_object *o zend_property_info **table = obj->ce->properties_info_table; intptr_t prop_num = slot - obj->properties_table; ZEND_ASSERT(prop_num >= 0 && prop_num < obj->ce->default_properties_count); - return table[prop_num]; + if (table[prop_num]) { + return table[prop_num]; + } else { + return zend_get_property_info_for_slot_slow(obj, slot); + } } /* Helper for cases where we're only interested in property info of typed properties. */ diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 110d84d3a343c..d7daad76f17c2 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -14377,7 +14377,7 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit, ref = ir_CONST_ADDR(prop_info); } else { int prop_info_offset = - (((prop_info->offset - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); + (((Z_PROP_TABLE_OFFSET(prop_info) - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); ref = ir_LOAD_A(ir_ADD_OFFSET(obj_ref, offsetof(zend_object, ce))); ref = ir_LOAD_A(ir_ADD_OFFSET(ref, offsetof(zend_class_entry, properties_info_table))); @@ -14778,7 +14778,7 @@ static int zend_jit_assign_obj(zend_jit_ctx *jit, ref = ir_CONST_ADDR(prop_info); } else { int prop_info_offset = - (((prop_info->offset - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); + (((Z_PROP_TABLE_OFFSET(prop_info) - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); ref = ir_LOAD_A(ir_ADD_OFFSET(obj_ref, offsetof(zend_object, ce))); ref = ir_LOAD_A(ir_ADD_OFFSET(ref, offsetof(zend_class_entry, properties_info_table))); @@ -15134,7 +15134,7 @@ static int zend_jit_assign_obj_op(zend_jit_ctx *jit, ref = ir_CONST_ADDR(prop_info); } else { int prop_info_offset = - (((prop_info->offset - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); + (((Z_PROP_TABLE_OFFSET(prop_info) - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); ref = ir_LOAD_A(ir_ADD_OFFSET(obj_ref, offsetof(zend_object, ce))); ref = ir_LOAD_A(ir_ADD_OFFSET(ref, offsetof(zend_class_entry, properties_info_table))); @@ -15524,7 +15524,7 @@ static int zend_jit_incdec_obj(zend_jit_ctx *jit, ref = ir_CONST_ADDR(prop_info); } else { int prop_info_offset = - (((prop_info->offset - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); + (((Z_PROP_TABLE_OFFSET(prop_info) - (sizeof(zend_object) - sizeof(zval))) / sizeof(zval)) * sizeof(void*)); ref = ir_LOAD_A(ir_ADD_OFFSET(obj_ref, offsetof(zend_object, ce))); ref = ir_LOAD_A(ir_ADD_OFFSET(ref, offsetof(zend_class_entry, properties_info_table)));