Disallow indirect modification on readonly properties within __clone() (#15012)

Indirect modification isn't allowed in __construct() because it allows
references to leak, so it doesn't make much sense to allow it in __clone().

UPGRADING

@@ -33,6 +33,10 @@ PHP 8.4 UPGRADE NOTES
     now 13 bytes longer. Total length is platform-dependent.
   . Encountering recursion during comparison now results in a Error exception,
     rather than a fatal error.
+  . Indirect modification of readonly properties within __clone() is no longer
+    allowed, e.g. $ref = &$this->readonly. This was already forbidden for
+    readonly initialization, and was an oversight in the "readonly
+    reinitialization during cloning" implementation.
 
 - DOM:
   . Added DOMNode::compareDocumentPosition() and DOMNode::DOCUMENT_POSITION_*

Zend/tests/readonly_props/readonly_clone_error5.phpt

@@ -57,20 +57,8 @@ try {
 }
 
 ?>
---EXPECTF--
-object(TestSetOnce)#%d (1) {
-  ["prop"]=>
-  array(1) {
-    [0]=>
-    int(1)
-  }
-}
-object(TestSetOnce)#%d (1) {
-  ["prop"]=>
-  array(1) {
-    [0]=>
-    int(1)
-  }
-}
+--EXPECT--
+Cannot indirectly modify readonly property TestSetOnce::$prop
+Cannot indirectly modify readonly property TestSetOnce::$prop
 Cannot indirectly modify readonly property TestSetTwice::$prop
 Cannot indirectly modify readonly property TestSetTwice::$prop

Zend/tests/readonly_props/readonly_clone_error7.phpt

@@ -0,0 +1,33 @@
+--TEST--
+__clone() cannot 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
+try {
+    var_dump(clone $foo);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(clone $foo);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Cannot indirectly modify readonly property Foo::$bar
+Cannot indirectly modify readonly property Foo::$bar

Zend/tests/readonly_props/readonly_clone_success3.phpt

@@ -1,37 +0,0 @@
---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/zend_execute.c

@@ -3364,8 +3364,6 @@ 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_indirect_modification_error(prop_info);
 							ZVAL_ERROR(result);

Zend/zend_object_handlers.c

@@ -696,8 +696,6 @@ try_again:
 					 * 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_indirect_modification_error(prop_info);
 					retval = &EG(uninitialized_zval);

ext/opcache/jit/zend_jit_ir.c

@@ -13954,15 +13954,6 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 
 			ir_IF_FALSE_cold(if_prop_obj);
 
-			ir_ref extra_addr = ir_ADD_OFFSET(jit_ZVAL_ADDR(jit, prop_addr), offsetof(zval, u2.extra));
-			ir_ref extra = ir_LOAD_U32(extra_addr);
-			ir_ref if_reinitable = ir_IF(ir_AND_U32(extra, ir_CONST_U32(IS_PROP_REINITABLE)));
-			ir_IF_TRUE(if_reinitable);
-			ir_STORE(extra_addr, ir_AND_U32(extra, ir_CONST_U32(~IS_PROP_REINITABLE)));
-			ir_ref reinit_path = ir_END();
-
-			ir_IF_FALSE(if_reinitable);
-
 			jit_SET_EX_OPLINE(jit, opline);
 			ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_readonly_property_indirect_modification_error), prop_info_ref);
 			jit_set_Z_TYPE_INFO(jit, res_addr, _IS_ERROR);
@@ -13970,7 +13961,6 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 
 			if (flags == ZEND_FETCH_DIM_WRITE) {
 				ir_IF_FALSE_cold(if_readonly);
-				ir_MERGE_WITH(reinit_path);
 				jit_SET_EX_OPLINE(jit, opline);
 				ir_CALL_2(IR_VOID, ir_CONST_FC_FUNC(zend_jit_check_array_promotion),
 					prop_ref, prop_info_ref);
@@ -13978,7 +13968,6 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 				ir_IF_FALSE(if_has_prop_info);
 			} else if (flags == ZEND_FETCH_REF) {
 				ir_IF_FALSE_cold(if_readonly);
-				ir_MERGE_WITH(reinit_path);
 				ir_CALL_3(IR_VOID, ir_CONST_FC_FUNC(zend_jit_create_typed_ref),
 					prop_ref,
 					prop_info_ref,
@@ -13986,14 +13975,11 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 				ir_END_list(end_inputs);
 				ir_IF_FALSE(if_has_prop_info);
 			} else {
-				ir_ref list = reinit_path;
-
 				ZEND_ASSERT(flags == 0);
 				ir_IF_FALSE(if_has_prop_info);
-				ir_END_list(list);
+				ir_ref no_prop_info_path = ir_END();
 				ir_IF_FALSE(if_readonly);
-				ir_END_list(list);
-				ir_MERGE_list(list);
+				ir_MERGE_WITH(no_prop_info_path);
 			}
 		}
 	} else {
@@ -14031,19 +14017,12 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 			ir_END_list(end_inputs);
 
 			ir_IF_FALSE_cold(if_prop_obj);
-
-			ir_ref extra_addr = ir_ADD_OFFSET(jit_ZVAL_ADDR(jit, prop_addr), offsetof(zval, u2.extra));
-			ir_ref extra = ir_LOAD_U32(extra_addr);
-			ir_ref if_reinitable = ir_IF(ir_AND_U32(extra, ir_CONST_U32(IS_PROP_REINITABLE)));
-
-			ir_IF_FALSE(if_reinitable);
 			jit_SET_EX_OPLINE(jit, opline);
 			ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_readonly_property_indirect_modification_error), ir_CONST_ADDR(prop_info));
 			jit_set_Z_TYPE_INFO(jit, res_addr, _IS_ERROR);
 			ir_END_list(end_inputs);
 
-			ir_IF_TRUE(if_reinitable);
-			ir_STORE(extra_addr, ir_AND_U32(extra, ir_CONST_U32(~IS_PROP_REINITABLE)));
+			goto result_fetched;
 		}
 
 		if (opline->opcode == ZEND_FETCH_OBJ_W
@@ -14117,6 +14096,7 @@ static int zend_jit_fetch_obj(zend_jit_ctx         *jit,
 		}
 	}
 
+result_fetched:
 	if (op1_avoid_refcounting) {
 		SET_STACK_REG(JIT_G(current_frame)->stack, EX_VAR_TO_NUM(opline->op1.var), ZREG_NONE);
 	}