Fix circumvented added hooks in JIT

The following code poses a problem in the JIT:

```php
class A {
    public $prop = 1;
}

class B extends A {
    public $prop = 1 {
        get => parent::$prop::get() * 2;
    }
}

function test(A $a) {
    var_dump($a->prop);
}

test(new B);
```

The JIT would assume A::$prop in test() could be accessed directly
through OBJ_PROP_NUM(). However, since child classes can add new hooks
to existing properties, this assumption no longer holds.

To avoid introducing more JIT checks, a hooked property that overrides a
unhooked property now results in a separate zval slot that is used
instead of the parent slot. This causes the JIT to pick the slow path
due to an IS_UNDEF value in the parent slot.

zend_class_entry.properties_info_table poses a problem in that
zend_get_property_info_for_slot() and friends will be called using the
child slot, which does not store its property info, since the parent
slot already does. In this case, zend_get_property_info_for_slot() now
provides a fallback that will iterate all property infos to find the
correct one.

This also uncovered a bug (see Zend/tests/property_hooks/dump.phpt)
where the default value of a parent property would accidentally be
inherited by the child property.

Fixes GH-17376
Closes GH-17870

Author: iluuu1994

NEWS

@@ -7,6 +7,8 @@ PHP                                                                        NEWS
     (ilutov)
   . Fixed accidentally inherited default value in overridden virtual properties.
     (ilutov)
+  . Fixed bug GH-17376 (Broken JIT polymorphism for property hooks added to
+    child class). (ilutov)
 
 27 Feb 2025, PHP 8.4.5
 

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--
+<?php
+
+class A {
+    public $prop = 1;
+}
+
+class B extends A {
+    public $prop = 1 {
+        get {
+            echo __METHOD__, "\n";
+            return $this->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

Zend/zend_compile.h

@@ -463,7 +463,7 @@ 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))
 
 typedef struct _zend_class_constant {
 	zval value; /* flags are stored in u2 */

Zend/zend_inheritance.c

@@ -1478,14 +1478,32 @@ 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 hooks to the child property, we use the child's slot for
+				 * storage to keep the parent slot set to IS_UNDEF. 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;
+				}
+
 				int parent_num = OBJ_PROP_TO_NUM(parent_info->offset);
 				if (child_info->offset != ZEND_VIRTUAL_PROPERTY_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]);
+					}
 				} else {
 					/* Default value was removed in child, remove it from parent too. */
 					if (ZEND_TYPE_IS_SET(child_info->type)) {
@@ -1495,7 +1513,9 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke
 					}
 				}
 
-				child_info->offset = parent_info->offset;
+				if (!use_child_prop) {
+					child_info->offset = parent_info->offset;
+				}
 				child_info->flags &= ~ZEND_ACC_VIRTUAL;
 			}
 
@@ -1670,7 +1690,8 @@ 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;
+			uint32_t prop_table_offset = OBJ_PROP_TO_NUM(!(prop->prototype->flags & ZEND_ACC_VIRTUAL) ? prop->prototype->offset : prop->offset);
+			table[prop_table_offset] = prop;
 		}
 	} ZEND_HASH_FOREACH_END();
 }
@@ -1684,8 +1705,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. */

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)) {

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;
+}

Zend/zend_objects_API.h

@@ -96,14 +96,20 @@ 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)
 {
 	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. */