Allow readonly properties to be reinitialized once during cloning (#10389)

RFC: https://wiki.php.net/rfc/readonly_amendments

Zend/tests/readonly_props/readonly_clone_error1.phpt

@@ -0,0 +1,36 @@
+--TEST--
+Readonly property cannot be reset twice during cloning
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar
+    ) {}
+
+    public function __clone()
+    {
+        $this->bar = 2;
+        var_dump($this);
+        $this->bar = 3;
+    }
+}
+
+$foo = new Foo(1);
+
+try {
+    clone $foo;
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+echo "done";
+
+?>
+--EXPECT--
+object(Foo)#2 (1) {
+  ["bar"]=>
+  int(2)
+}
+Cannot modify readonly property Foo::$bar
+done

Zend/tests/readonly_props/readonly_clone_error2.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Readonly property cannot be reset after cloning when there is no custom clone handler
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar,
+        public readonly int $baz
+    ) {}
+
+    public function wrongCloneOld()
+    {
+        $instance = clone $this;
+        $this->bar++;
+    }
+
+    public function wrongCloneNew()
+    {
+        $instance = clone $this;
+        $instance->baz++;
+    }
+}
+
+$foo = new Foo(1, 1);
+
+try {
+    $foo->wrongCloneOld();
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+try {
+    $foo->wrongCloneNew();
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+Cannot modify readonly property Foo::$bar
+Cannot modify readonly property Foo::$baz

Zend/tests/readonly_props/readonly_clone_error3.phpt

@@ -0,0 +1,44 @@
+--TEST--
+Readonly property cannot be reset after cloning when there is a custom clone handler
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar,
+        public readonly int $baz
+    ) {}
+
+    public function __clone() {}
+
+    public function wrongCloneOld()
+    {
+        $instance = clone $this;
+        $this->bar++;
+    }
+
+    public function wrongCloneNew()
+    {
+        $instance = clone $this;
+        $instance->baz++;
+    }
+}
+
+$foo = new Foo(1, 1);
+
+try {
+    $foo->wrongCloneOld();
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+try {
+    $foo->wrongCloneNew();
+} catch (Error $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+Cannot modify readonly property Foo::$bar
+Cannot modify readonly property Foo::$baz

Zend/tests/readonly_props/readonly_clone_success1.phpt

@@ -0,0 +1,39 @@
+--TEST--
+Readonly property can be reset once during cloning
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar
+    ) {}
+
+    public function __clone()
+    {
+        $this->bar++;
+    }
+}
+
+$foo = new Foo(1);
+
+var_dump(clone $foo);
+
+$foo2 = clone $foo;
+var_dump($foo2);
+
+var_dump(clone $foo2);
+
+?>
+--EXPECTF--
+object(Foo)#%d (%d) {
+  ["bar"]=>
+  int(2)
+}
+object(Foo)#%d (%d) {
+  ["bar"]=>
+  int(2)
+}
+object(Foo)#%d (%d) {
+  ["bar"]=>
+  int(3)
+}

Zend/tests/readonly_props/readonly_clone_success2.phpt

@@ -0,0 +1,46 @@
+--TEST--
+Test that __clone() unset and reassign properties
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly stdClass $bar,
+    ) {}
+
+    public function __clone()
+    {
+        unset($this->bar);
+        var_dump($this);
+        $this->bar = new stdClass();
+    }
+}
+
+$foo = new Foo(new stdClass());
+var_dump($foo);
+$foo2 = clone $foo;
+
+var_dump($foo);
+var_dump($foo2);
+
+?>
+--EXPECTF--
+object(Foo)#1 (%d) {
+  ["bar"]=>
+  object(stdClass)#2 (%d) {
+  }
+}
+object(Foo)#3 (%d) {
+  ["bar"]=>
+  uninitialized(stdClass)
+}
+object(Foo)#1 (%d) {
+  ["bar"]=>
+  object(stdClass)#2 (%d) {
+  }
+}
+object(Foo)#3 (%d) {
+  ["bar"]=>
+  object(stdClass)#4 (%d) {
+  }
+}

Zend/tests/readonly_props/readonly_clone_success3.phpt

@@ -0,0 +1,37 @@
+--TEST--
+__clone() can indirectly modify unlocked readonly properties
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly array $bar
+    ) {}
+
+    public function __clone()
+    {
+        $this->bar['bar'] = 'bar';
+    }
+}
+
+$foo = new Foo([]);
+// First call fills the cache slot
+var_dump(clone $foo);
+var_dump(clone $foo);
+
+?>
+--EXPECTF--
+object(Foo)#2 (%d) {
+  ["bar"]=>
+  array(1) {
+    ["bar"]=>
+    string(3) "bar"
+  }
+}
+object(Foo)#2 (%d) {
+  ["bar"]=>
+  array(1) {
+    ["bar"]=>
+    string(3) "bar"
+  }
+}

Zend/tests/readonly_props/readonly_clone_success4.phpt

@@ -0,0 +1,39 @@
+--TEST--
+Readonly property can be reset once during cloning even after a type error
+--FILE--
+<?php
+
+class Foo {
+    public function __construct(
+        public readonly int $bar
+    ) {}
+
+    public function __clone()
+    {
+        try {
+            $this->bar = "foo";
+        } catch (Error $e) {
+            echo $e->getMessage() . "\n";
+        }
+
+        $this->bar = 2;
+    }
+}
+
+$foo = new Foo(1);
+
+var_dump(clone $foo);
+var_dump(clone $foo);
+
+?>
+--EXPECTF--
+Cannot assign string to property Foo::$bar of type int
+object(Foo)#%d (%d) {
+  ["bar"]=>
+  int(2)
+}
+Cannot assign string to property Foo::$bar of type int
+object(Foo)#%d (%d) {
+  ["bar"]=>
+  int(2)
+}

Zend/tests/readonly_props/readonly_coercion_type_error.phpt

@@ -0,0 +1,23 @@
+--TEST--
+Test failing readonly assignment with coercion
+--FILE--
+<?php
+
+class Foo {
+    public readonly string $bar;
+
+    public function __construct() {
+        $this->bar = 'bar';
+        try {
+            $this->bar = 42;
+        } catch (Error $e) {
+            echo $e->getMessage(), "\n";
+        }
+    }
+}
+
+new Foo();
+
+?>
+--EXPECTF--
+Cannot modify readonly property Foo::$bar

Zend/zend_API.c

@@ -4224,6 +4224,10 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
 		ce->ce_flags |= ZEND_ACC_HAS_TYPE_HINTS;
 	}
 
+	if (access_type & ZEND_ACC_READONLY) {
+		ce->ce_flags |= ZEND_ACC_HAS_READONLY_PROPS;
+	}
+
 	if (ce->type == ZEND_INTERNAL_CLASS) {
 		property_info = pemalloc(sizeof(zend_property_info), 1);
 	} else {

Zend/zend_compile.h

@@ -241,7 +241,7 @@ typedef struct _zend_oparray_context {
 /* or IS_CONSTANT_VISITED_MARK                            |     |     |     */
 #define ZEND_CLASS_CONST_IS_CASE         (1 << 6)  /*     |     |     |  X  */
 /*                                                        |     |     |     */
-/* Class Flags (unused: 21,30,31)                         |     |     |     */
+/* Class Flags (unused: 30,31)                            |     |     |     */
 /* ===========                                            |     |     |     */
 /*                                                        |     |     |     */
 /* Special class types                                    |     |     |     */
@@ -288,6 +288,8 @@ typedef struct _zend_oparray_context {
 /*                                                        |     |     |     */
 /* Class is linked apart from variance obligations.       |     |     |     */
 #define ZEND_ACC_NEARLY_LINKED           (1 << 20) /*  X  |     |     |     */
+/* Class has readonly props                               |     |     |     */
+#define ZEND_ACC_HAS_READONLY_PROPS      (1 << 21) /*  X  |     |     |     */
 /*                                                        |     |     |     */
 /* stored in opcache (may be partially)                   |     |     |     */
 #define ZEND_ACC_CACHED                  (1 << 22) /*  X  |     |     |     */

Zend/zend_enum.c

@@ -41,9 +41,17 @@ zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case
 	zend_object *zobj = zend_objects_new(ce);
 	ZVAL_OBJ(result, zobj);
 
-	ZVAL_STR_COPY(OBJ_PROP_NUM(zobj, 0), case_name);
+	zval *zname = OBJ_PROP_NUM(zobj, 0);
+	ZVAL_STR_COPY(zname, case_name);
+	/* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */
+	Z_PROP_FLAG_P(zname) = 0;
+
 	if (backing_value_zv != NULL) {
-		ZVAL_COPY(OBJ_PROP_NUM(zobj, 1), backing_value_zv);
+		zval *prop = OBJ_PROP_NUM(zobj, 1);
+
+		ZVAL_COPY(prop, backing_value_zv);
+		/* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */
+		Z_PROP_FLAG_P(prop) = 0;
 	}
 
 	return zobj;
@@ -179,7 +187,7 @@ void zend_enum_add_interfaces(zend_class_entry *ce)
 
 	if (ce->enum_backing_type != IS_UNDEF) {
 		ce->interface_names[num_interfaces_before + 1].name = zend_string_copy(zend_ce_backed_enum->name);
-		ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0);	
+		ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0);
 	}
 
 	ce->default_object_handlers = &zend_enum_object_handlers;

Zend/zend_execute.c

@@ -1009,11 +1009,6 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf
 {
 	zval tmp;
 
-	if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) {
-		zend_readonly_property_modification_error(info);
-		return &EG(uninitialized_zval);
-	}
-
 	ZVAL_DEREF(value);
 	ZVAL_COPY(&tmp, value);
 
@@ -1022,6 +1017,16 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf
 		return &EG(uninitialized_zval);
 	}
 
+	if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) {
+		if (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE) {
+			Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
+		} else {
+			zval_ptr_dtor(&tmp);
+			zend_readonly_property_modification_error(info);
+			return &EG(uninitialized_zval);
+		}
+	}
+
 	return zend_assign_to_variable(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES());
 }
 
@@ -3161,6 +3166,8 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c
 						ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET);
 						if (Z_TYPE_P(ptr) == IS_OBJECT) {
 							ZVAL_COPY(result, ptr);
+						} else if (Z_PROP_FLAG_P(ptr) & IS_PROP_REINITABLE) {
+							Z_PROP_FLAG_P(ptr) &= ~IS_PROP_REINITABLE;
 						} else {
 							zend_readonly_property_modification_error(prop_info);
 							ZVAL_ERROR(result);

Zend/zend_execute.h

@@ -463,6 +463,7 @@ ZEND_API int ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *call);
 	} while (0)
 
 #define ZEND_CLASS_HAS_TYPE_HINTS(ce) ((ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS) == ZEND_ACC_HAS_TYPE_HINTS)
+#define ZEND_CLASS_HAS_READONLY_PROPS(ce) ((ce->ce_flags & ZEND_ACC_HAS_READONLY_PROPS) == ZEND_ACC_HAS_READONLY_PROPS)
 
 ZEND_API bool zend_verify_property_type(const zend_property_info *info, zval *property, bool strict);
 ZEND_COLD void zend_verify_property_type_error(const zend_property_info *info, const zval *property);

Zend/zend_inheritance.c

@@ -1629,7 +1629,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			ce->ce_flags |= ZEND_ACC_EXPLICIT_ABSTRACT_CLASS;
 		}
 	}
-	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES);
+	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_HAS_READONLY_PROPS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES);
 }
 /* }}} */
 

Zend/zend_object_handlers.c

@@ -615,6 +615,8 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
 					 * to make sure no actual modification is possible. */
 					ZVAL_COPY(rv, retval);
 					retval = rv;
+				} else if (Z_PROP_FLAG_P(retval) & IS_PROP_REINITABLE) {
+					Z_PROP_FLAG_P(retval) &= ~IS_PROP_REINITABLE;
 				} else {
 					zend_readonly_property_modification_error(prop_info);
 					retval = &EG(uninitialized_zval);
@@ -633,7 +635,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
 				}
 			}
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT)) {
 			/* Skip __get() for uninitialized typed properties */
 			goto uninit_error;
 		}
@@ -810,13 +812,6 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 			Z_TRY_ADDREF_P(value);
 
 			if (UNEXPECTED(prop_info)) {
-				if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
-					Z_TRY_DELREF_P(value);
-					zend_readonly_property_modification_error(prop_info);
-					variable_ptr = &EG(error_zval);
-					goto exit;
-				}
-
 				ZVAL_COPY_VALUE(&tmp, value);
 				// Increase refcount to prevent object from being released in __toString()
 				GC_ADDREF(zobj);
@@ -833,6 +828,16 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 					variable_ptr = &EG(error_zval);
 					goto exit;
 				}
+				if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
+					if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE) {
+						Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE;
+					} else {
+						zval_ptr_dtor(&tmp);
+						zend_readonly_property_modification_error(prop_info);
+						variable_ptr = &EG(error_zval);
+						goto exit;
+					}
+				}
 				value = &tmp;
 			}
 
@@ -841,7 +846,7 @@ found:
 				variable_ptr, value, IS_TMP_VAR, property_uses_strict_types());
 			goto exit;
 		}
-		if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) {
+		if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_UNINIT) {
 			/* Writes to uninitialized typed properties bypass __set(). */
 			goto write_std_property;
 		}
@@ -1069,7 +1074,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
 		if (UNEXPECTED(Z_TYPE_P(retval) == IS_UNDEF)) {
 			if (EXPECTED(!zobj->ce->__get) ||
 			    UNEXPECTED((*zend_get_property_guard(zobj, name)) & IN_GET) ||
-			    UNEXPECTED(prop_info && Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) {
+			    UNEXPECTED(prop_info && (Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT))) {
 				if (UNEXPECTED(type == BP_VAR_RW || type == BP_VAR_R)) {
 					if (UNEXPECTED(prop_info)) {
 						zend_throw_error(NULL,
@@ -1146,8 +1151,12 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
 
 		if (Z_TYPE_P(slot) != IS_UNDEF) {
 			if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY))) {
-				zend_readonly_property_unset_error(prop_info->ce, name);
-				return;
+				if (Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE) {
+					Z_PROP_FLAG_P(slot) &= ~IS_PROP_REINITABLE;
+				} else {
+					zend_readonly_property_unset_error(prop_info->ce, name);
+					return;
+				}
 			}
 			if (UNEXPECTED(Z_ISREF_P(slot)) &&
 					(ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(slot)))) {
@@ -1164,7 +1173,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
 			}
 			return;
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(slot) & IS_PROP_UNINIT)) {
 			if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY)
 					&& !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) {
 				return;
@@ -1779,7 +1788,7 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
 		if (Z_TYPE_P(value) != IS_UNDEF) {
 			goto found;
 		}
-		if (UNEXPECTED(Z_PROP_FLAG_P(value) == IS_PROP_UNINIT)) {
+		if (UNEXPECTED(Z_PROP_FLAG_P(value) & IS_PROP_UNINIT)) {
 			/* Skip __isset() for uninitialized typed properties */
 			result = 0;
 			goto exit;

Zend/zend_objects.c

@@ -192,6 +192,8 @@ ZEND_API zend_object* ZEND_FASTCALL zend_objects_new(zend_class_entry *ce)
 
 ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, zend_object *old_object)
 {
+	bool has_clone_method = old_object->ce->clone != NULL;
+
 	if (old_object->ce->default_properties_count) {
 		zval *src = old_object->properties_table;
 		zval *dst = new_object->properties_table;
@@ -201,6 +203,11 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 			i_zval_ptr_dtor(dst);
 			ZVAL_COPY_VALUE_PROP(dst, src);
 			zval_add_ref(dst);
+			if (has_clone_method) {
+				/* Unconditionally add the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */
+				Z_PROP_FLAG_P(dst) |= IS_PROP_REINITABLE;
+			}
+
 			if (UNEXPECTED(Z_ISREF_P(dst)) &&
 					(ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(dst)))) {
 				zend_property_info *prop_info = zend_get_property_info_for_slot(new_object, dst);
@@ -211,7 +218,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 			src++;
 			dst++;
 		} while (src != end);
-	} else if (old_object->properties && !old_object->ce->clone) {
+	} else if (old_object->properties && !has_clone_method) {
 		/* fast copy */
 		if (EXPECTED(old_object->handlers == &std_object_handlers)) {
 			if (EXPECTED(!(GC_FLAGS(old_object->properties) & IS_ARRAY_IMMUTABLE))) {
@@ -245,6 +252,10 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 				ZVAL_COPY_VALUE(&new_prop, prop);
 				zval_add_ref(&new_prop);
 			}
+			if (has_clone_method) {
+				/* Unconditionally add the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */
+				Z_PROP_FLAG_P(&new_prop) |= IS_PROP_REINITABLE;
+			}
 			if (EXPECTED(key)) {
 				_zend_hash_append(new_object->properties, key, &new_prop);
 			} else {
@@ -253,9 +264,18 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 		} ZEND_HASH_FOREACH_END();
 	}
 
-	if (old_object->ce->clone) {
+	if (has_clone_method) {
 		GC_ADDREF(new_object);
 		zend_call_known_instance_method_with_0_params(new_object->ce->clone, new_object, NULL);
+
+		if (ZEND_CLASS_HAS_READONLY_PROPS(new_object->ce)) {
+			for (uint32_t i = 0; i < new_object->ce->default_properties_count; i++) {
+				zval* prop = OBJ_PROP_NUM(new_object, i);
+				/* Unconditionally remove the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */
+				Z_PROP_FLAG_P(prop) &= ~IS_PROP_REINITABLE;
+			}
+		}
+
 		OBJ_RELEASE(new_object);
 	}
 }

Zend/zend_types.h

@@ -1273,7 +1273,8 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
  * (both use IS_UNDEF type) in the Z_EXTRA space. As such we also need to copy
  * the Z_EXTRA space when copying property default values etc. We define separate
  * macros for this purpose, so this workaround is easier to remove in the future. */
-#define IS_PROP_UNINIT 1
+#define IS_PROP_UNINIT (1<<0)
+#define IS_PROP_REINITABLE (1<<1)  /* It has impact only on readonly properties */
 #define Z_PROP_FLAG_P(z) Z_EXTRA_P(z)
 #define ZVAL_COPY_VALUE_PROP(z, v) \
 	do { *(z) = *(v); } while (0)