Handle promoted readonly reassignment in readonly failure paths

Author: nicolas-grekas

Zend/Optimizer/compact_literals.c

@@ -514,15 +514,18 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 				case ZEND_POST_INC_OBJ:
 				case ZEND_POST_DEC_OBJ:
 					if (opline->op2_type == IS_CONST) {
+						uint32_t obj_flags = (opline->opcode == ZEND_ASSIGN_OBJ || opline->opcode == ZEND_ASSIGN_OBJ_REF)
+							? ZEND_ASSIGN_OBJ_FLAGS
+							: ZEND_FETCH_OBJ_FLAGS;
 						// op2 property
 						if (opline->op1_type == IS_UNUSED &&
 						    property_slot[opline->op2.constant] >= 0) {
-							opline->extended_value = property_slot[opline->op2.constant] | (opline->extended_value & ZEND_FETCH_OBJ_FLAGS);
+							opline->extended_value = property_slot[opline->op2.constant] | (opline->extended_value & obj_flags);
 						} else {
-							opline->extended_value = cache_size | (opline->extended_value & ZEND_FETCH_OBJ_FLAGS);
+							opline->extended_value = cache_size | (opline->extended_value & obj_flags);
 							cache_size += 3 * sizeof(void *);
 							if (opline->op1_type == IS_UNUSED) {
-								property_slot[opline->op2.constant] = opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS;
+								property_slot[opline->op2.constant] = opline->extended_value & ~obj_flags;
 							}
 						}
 					}

Zend/tests/readonly_props/cpp_reassign_grandchild_redefine.phpt

@@ -36,4 +36,4 @@ new D();
 ?>
 --EXPECT--
 Error: Cannot modify readonly property D::$x
-string(2) "C2"
+string(2) "C2"

Zend/zend_API.c

@@ -1744,7 +1744,7 @@ ZEND_API void object_properties_load(zend_object *object, const HashTable *prope
 				zval *slot = OBJ_PROP(object, property_info->offset);
 				if (UNEXPECTED((property_info->flags & ZEND_ACC_READONLY) && !Z_ISUNDEF_P(slot))) {
 					if (Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE) {
-						Z_PROP_FLAG_P(slot) &= ~(IS_PROP_REINITABLE | IS_PROP_CTOR_REINITABLE);
+						Z_PROP_FLAG_P(slot) &= ~IS_PROP_REINITABLE;
 					} else {
 						zend_readonly_property_modification_error(property_info);
 						return;
@@ -4422,9 +4422,6 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
 
 		if (access_type & ZEND_ACC_READONLY) {
 			ce->ce_flags |= ZEND_ACC_HAS_READONLY_PROPS;
-			if (access_type & ZEND_ACC_PROMOTED) {
-				ce->ce_flags |= ZEND_ACC_HAS_PROMOTED_READONLY_PROPS;
-			}
 		}
 	}
 

Zend/zend_compile.c

@@ -8310,6 +8310,9 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
 		zend_op *opline = zend_emit_op(NULL,
 			is_ref ? ZEND_ASSIGN_OBJ_REF : ZEND_ASSIGN_OBJ, NULL, &name_node);
 		opline->extended_value = zend_alloc_cache_slots(3);
+		if ((flags & ZEND_ACC_READONLY) != 0) {
+			opline->extended_value |= ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT;
+		}
 		zend_emit_op_data(&value_node);
 	}
 }

Zend/zend_compile.h

@@ -338,8 +338,6 @@ typedef struct _zend_oparray_context {
 /* loaded from file cache to process memory               |     |     |     */
 #define ZEND_ACC_FILE_CACHED             (1 << 27) /*  X  |     |     |     */
 /*                                                        |     |     |     */
-#define ZEND_ACC_HAS_PROMOTED_READONLY_PROPS (1u << 31) /* X |  |     |     */
-
 /* Class cannot be serialized or unserialized             |     |     |     */
 #define ZEND_ACC_NOT_SERIALIZABLE        (1 << 29) /*  X  |     |     |     */
 /*                                                        |     |     |     */
@@ -1099,6 +1097,9 @@ ZEND_API zend_string *zend_type_to_string(zend_type type);
 #define ZEND_FETCH_DIM_WRITE	2
 #define ZEND_FETCH_OBJ_FLAGS	3
 
+#define ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT (1u << 31)
+#define ZEND_ASSIGN_OBJ_FLAGS (ZEND_FETCH_OBJ_FLAGS | ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT)
+
 /* Used to mark what kind of operation a writing FETCH_DIM is used in,
  * to produce a more precise error on incorrect string offset use. */
 #define ZEND_FETCH_DIM_REF 1

Zend/zend_execute.c

@@ -1071,10 +1071,14 @@ ZEND_API bool zend_never_inline zend_verify_property_type(const zend_property_in
 static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_info *info, zval *property_val, zval *value, zend_refcounted **garbage_ptr EXECUTE_DATA_DC)
 {
 	zval tmp;
+	zend_readonly_write_kind readonly_write_kind = ZEND_READONLY_WRITE_FORBIDDEN;
 
 	if (UNEXPECTED(info->flags & (ZEND_ACC_READONLY|ZEND_ACC_PPP_SET_MASK))) {
+		if (info->flags & ZEND_ACC_READONLY) {
+			readonly_write_kind = zend_get_readonly_write_kind(property_val, info);
+		}
 		if ((info->flags & ZEND_ACC_READONLY)
-		 && (!zend_readonly_property_is_reinitable_for_context(property_val, info)
+		 && (readonly_write_kind == ZEND_READONLY_WRITE_FORBIDDEN
 		     || zend_is_foreign_cpp_overwrite(property_val, info))) {
 			zend_readonly_property_modification_error(info);
 			return &EG(uninitialized_zval);
@@ -1093,7 +1097,11 @@ static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_inf
 		return &EG(uninitialized_zval);
 	}
 
-	Z_PROP_FLAG_P(property_val) &= ~(IS_PROP_REINITABLE | IS_PROP_CTOR_REINITABLE);
+	if (readonly_write_kind == ZEND_READONLY_WRITE_REINITABLE) {
+		Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
+	} else if (readonly_write_kind == ZEND_READONLY_WRITE_CTOR_REASSIGNED) {
+		Z_PROP_FLAG_P(property_val) |= IS_PROP_CTOR_REASSIGNED;
+	}
 
 	return zend_assign_to_variable_ex(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES(), garbage_ptr);
 }
@@ -3656,7 +3664,7 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c
 static zend_always_inline void zend_assign_to_property_reference(zval *container, uint32_t container_op_type, zval *prop_ptr, uint32_t prop_op_type, zval *value_ptr OPLINE_DC EXECUTE_DATA_DC)
 {
 	zval variable, *variable_ptr = &variable;
-	void **cache_addr = (prop_op_type == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_RETURNS_FUNCTION) : NULL;
+	void **cache_addr = (prop_op_type == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~(ZEND_RETURNS_FUNCTION | ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT)) : NULL;
 	zend_refcounted *garbage = NULL;
 	zend_property_info *prop_info = NULL;
 
@@ -5899,53 +5907,6 @@ static zend_always_inline zend_execute_data *_zend_vm_stack_push_call_frame(uint
 /* This callback disables optimization of "vm_stack_data" variable in VM */
 ZEND_API void (ZEND_FASTCALL *zend_touch_vm_stack_data)(void *vm_stack_data) = NULL;
 
-static zend_always_inline bool zend_has_more_derived_promoted_override(
-	const zend_class_entry *runtime_ce, const zend_class_entry *ctor_scope, zend_string *property_name)
-{
-	for (const zend_class_entry *ce = runtime_ce; ce != NULL && ce != ctor_scope; ce = ce->parent) {
-		zend_property_info *prop_info = zend_hash_find_ptr(&ce->properties_info, property_name);
-		if (prop_info != NULL && (prop_info->flags & ZEND_ACC_PROMOTED)) {
-			return true;
-		}
-	}
-	return false;
-}
-
-/* Outlined slow path: clear IS_PROP_REINITABLE from promoted readonly properties
- * of the exiting constructor's scope.  Skips properties that a child class has
- * redefined with its own CPP (the child owns that reassignment window). */
-static zend_never_inline void zend_ctor_clear_promoted_readonly_reinitable_slow(zend_execute_data *ex)
-{
-	zend_object *obj = Z_OBJ(ex->This);
-	zend_class_entry *ctor_scope = ex->func->common.scope;
-	zend_property_info *prop_info;
-
-	ZEND_HASH_MAP_FOREACH_PTR(&ctor_scope->properties_info, prop_info) {
-		if ((prop_info->flags & (ZEND_ACC_READONLY | ZEND_ACC_PROMOTED)) == (ZEND_ACC_READONLY | ZEND_ACC_PROMOTED)
-		 && IS_VALID_PROPERTY_OFFSET(prop_info->offset)) {
-			/* When the object is an instance of a descendant class, do not clear the
-			 * window if a more-derived class owns the property through its own CPP,
-			 * even if the runtime class itself redeclared it without CPP. */
-			if (obj->ce != ctor_scope
-			 && zend_has_more_derived_promoted_override(obj->ce, ctor_scope, prop_info->name)) {
-				continue;
-			}
-			Z_PROP_FLAG_P(OBJ_PROP(obj, prop_info->offset)) &= ~(IS_PROP_REINITABLE | IS_PROP_CTOR_REINITABLE);
-		}
-	} ZEND_HASH_FOREACH_END();
-}
-
-/* Clear IS_PROP_REINITABLE from all promoted readonly properties of the exiting
- * constructor's scope. Called for both 'new Foo()' and 'parent::__construct()'. */
-ZEND_API void ZEND_FASTCALL zend_ctor_clear_promoted_readonly_reinitable(zend_execute_data *ex, uint32_t call_info)
-{
-	if ((call_info & ZEND_CALL_HAS_THIS)
-	 && (ex->func->common.fn_flags & ZEND_ACC_CTOR)
-	 && ZEND_CLASS_HAS_PROMOTED_READONLY_PROPS(ex->func->common.scope)) {
-		zend_ctor_clear_promoted_readonly_reinitable_slow(ex);
-	}
-}
-
 #include "zend_vm_execute.h"
 
 ZEND_API zend_result zend_set_user_opcode_handler(zend_uchar opcode, user_opcode_handler_t handler)

Zend/zend_execute.h

@@ -583,7 +583,6 @@ ZEND_API zend_result ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *cal
 
 #define ZEND_CLASS_HAS_TYPE_HINTS(ce) ((bool)(ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS))
 #define ZEND_CLASS_HAS_READONLY_PROPS(ce) ((bool)(ce->ce_flags & ZEND_ACC_HAS_READONLY_PROPS))
-#define ZEND_CLASS_HAS_PROMOTED_READONLY_PROPS(ce) ((bool)(ce->ce_flags & ZEND_ACC_HAS_PROMOTED_READONLY_PROPS))
 
 static zend_always_inline bool zend_scope_is_derived_from(
 	const zend_class_entry *scope, const zend_class_entry *ancestor)
@@ -650,19 +649,53 @@ static zend_always_inline bool zend_has_active_ctor_with_promoted_property(
 	return false;
 }
 
-static zend_always_inline bool zend_readonly_property_is_reinitable_for_context(
-	const zval *property_val, const zend_property_info *prop_info)
+static zend_always_inline bool zend_is_promoted_readonly_ctor_init_assign(
+	const zend_execute_data *ex, const zend_property_info *prop_info)
 {
-	if (!(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE)) {
+	if (ex == NULL
+	 || ex->opline == NULL
+	 || !((ex->opline->opcode == ZEND_ASSIGN_OBJ || ex->opline->opcode == ZEND_ASSIGN_OBJ_REF)
+	  && (ex->opline->extended_value & ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT))) {
 		return false;
 	}
-	if (!(Z_PROP_FLAG_P(property_val) & IS_PROP_CTOR_REINITABLE)) {
-		return true;
+	if (!(ZEND_CALL_INFO(ex) & ZEND_CALL_HAS_THIS)) {
+		return false;
+	}
+	return zend_has_active_ctor_with_promoted_property(ex, Z_OBJ(ex->This), prop_info->name);
+}
+
+static zend_always_inline zend_object *zend_property_owner_from_slot(
+	const zval *property_val, const zend_property_info *prop_info)
+{
+	return (zend_object *) ((char *) property_val - prop_info->offset);
+}
+
+typedef enum _zend_readonly_write_kind {
+	ZEND_READONLY_WRITE_FORBIDDEN = 0, /* Write disallowed, or no special flag update needed */
+	ZEND_READONLY_WRITE_REINITABLE,    /* Clone reinit window: clear IS_PROP_REINITABLE after write */
+	ZEND_READONLY_WRITE_CTOR_REASSIGNED, /* CPP ctor reassignment: set IS_PROP_CTOR_REASSIGNED after write */
+} zend_readonly_write_kind;
+
+static zend_always_inline zend_readonly_write_kind zend_get_readonly_write_kind(
+	const zval *property_val, const zend_property_info *prop_info)
+{
+	if (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE) {
+		return ZEND_READONLY_WRITE_REINITABLE;
+	}
+	if (Z_PROP_FLAG_P(property_val) & IS_PROP_CTOR_REASSIGNED) {
+		return ZEND_READONLY_WRITE_FORBIDDEN;
 	}
 	zend_execute_data *ex = EG(current_execute_data);
-	return ex
-		&& (ZEND_CALL_INFO(ex) & ZEND_CALL_HAS_THIS)
-		&& zend_has_active_ctor_with_promoted_property(ex, Z_OBJ(ex->This), prop_info->name);
+	if (ex == NULL
+	 || !(ZEND_CALL_INFO(ex) & ZEND_CALL_HAS_THIS)
+	 || !zend_has_active_ctor_with_promoted_property(ex, Z_OBJ(ex->This), prop_info->name)
+	 || zend_property_owner_from_slot(property_val, prop_info) != Z_OBJ(ex->This)
+	 || (ex->opline != NULL
+	     && (ex->opline->opcode == ZEND_ASSIGN_OBJ || ex->opline->opcode == ZEND_ASSIGN_OBJ_REF)
+	     && (ex->opline->extended_value & ZEND_ASSIGN_OBJ_PROMOTED_READONLY_INIT))) {
+		return ZEND_READONLY_WRITE_FORBIDDEN;
+	}
+	return ZEND_READONLY_WRITE_CTOR_REASSIGNED;
 }
 
 /* Check if a foreign constructor is attempting a CPP initial assignment on an
@@ -684,7 +717,6 @@ static zend_always_inline bool zend_is_foreign_cpp_overwrite(
 		return true;
 	}
 	if (!(prop_info->flags & ZEND_ACC_PROMOTED)
-	 && (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE)
 	 && zend_has_active_derived_ctor_with_promoted_property(ex, Z_OBJ(ex->This), prop_info)) {
 		return true;
 	}
@@ -697,7 +729,6 @@ ZEND_COLD void zend_verify_class_constant_type_error(const zend_class_constant *
 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_COLD void zend_magic_get_property_type_inconsistency_error(const zend_property_info *info, const zval *property);
-ZEND_API void ZEND_FASTCALL zend_ctor_clear_promoted_readonly_reinitable(zend_execute_data *ex, uint32_t call_info);
 
 #define ZEND_REF_ADD_TYPE_SOURCE(ref, source) \
 	zend_ref_add_type_source(&ZEND_REF_TYPE_SOURCES(ref), source)

Zend/zend_object_handlers.c

@@ -1049,12 +1049,14 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 	uintptr_t property_offset;
 	const zend_property_info *prop_info = NULL;
 	uint32_t *guard = NULL;
+	zend_readonly_write_kind readonly_write_kind = ZEND_READONLY_WRITE_FORBIDDEN;
 	ZEND_ASSERT(!Z_ISREF_P(value));
 
 	property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__set != NULL), cache_slot, &prop_info);
 
 	if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
 try_again:
+		readonly_write_kind = ZEND_READONLY_WRITE_FORBIDDEN;
 		variable_ptr = OBJ_PROP(zobj, property_offset);
 
 		if (prop_info && UNEXPECTED(prop_info->flags & (ZEND_ACC_READONLY|ZEND_ACC_PPP_SET_MASK))) {
@@ -1066,9 +1068,12 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 				error = (*guard) & IN_SET;
 			}
 			if (error) {
+				if ((prop_info->flags & ZEND_ACC_READONLY) && Z_TYPE_P(variable_ptr) != IS_UNDEF) {
+					readonly_write_kind = zend_get_readonly_write_kind(variable_ptr, prop_info);
+				}
 				if ((prop_info->flags & ZEND_ACC_READONLY)
 				 && Z_TYPE_P(variable_ptr) != IS_UNDEF
-				 && (!zend_readonly_property_is_reinitable_for_context(variable_ptr, prop_info)
+				 && (readonly_write_kind == ZEND_READONLY_WRITE_FORBIDDEN
 				     || zend_is_foreign_cpp_overwrite(variable_ptr, prop_info))) {
 					zend_readonly_property_modification_error(prop_info);
 					variable_ptr = &EG(error_zval);
@@ -1103,42 +1108,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 					variable_ptr = &EG(error_zval);
 					goto exit;
 				}
-				/* For readonly properties initialized for the first time via CPP, set
-				 * IS_PROP_REINITABLE to allow one reassignment in the constructor body.
-				 * The flag is cleared by zend_leave_helper when the constructor exits.
-				 *
-				 * Classical case: the property is promoted in the declaring class and the
-				 * executing constructor belongs to that class (scope == prop_info->ce).
-				 *
-				 * Extended case: a child class redeclared the property without CPP, so
-				 * prop_info->ce is the child but the property isn't promoted there. CPP
-				 * "ownership" still belongs to the ancestor whose constructor has CPP for
-				 * this property name, so its body is allowed to reassign once. The clearing
-				 * loop in zend_leave_helper iterates the exiting ctor's own promoted props,
-				 * which share the same object slot, so cleanup happens automatically. */
-				bool reinitable = false;
-				if ((prop_info->flags & ZEND_ACC_READONLY)
-				 && (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_UNINIT)
-				 && EG(current_execute_data)
-				 && (EG(current_execute_data)->func->common.fn_flags & ZEND_ACC_CTOR)) {
-					zend_class_entry *ctor_scope = EG(current_execute_data)->func->common.scope;
-					if (prop_info->flags & ZEND_ACC_PROMOTED) {
-						reinitable = (ctor_scope == prop_info->ce);
-					} else if (ctor_scope != prop_info->ce) {
-						/* Child redeclared without CPP: check if the executing ctor's class
-						 * has a CPP declaration for this property name. */
-						zend_property_info *scope_prop = zend_hash_find_ptr(
-							&ctor_scope->properties_info, prop_info->name);
-						reinitable = scope_prop != NULL
-							&& (scope_prop->flags & (ZEND_ACC_READONLY|ZEND_ACC_PROMOTED))
-							   == (ZEND_ACC_READONLY|ZEND_ACC_PROMOTED);
-					}
-				}
-				if (reinitable) {
-					Z_PROP_FLAG_P(variable_ptr) = IS_PROP_REINITABLE | IS_PROP_CTOR_REINITABLE;
-				} else {
-					Z_PROP_FLAG_P(variable_ptr) &= ~(IS_PROP_UNINIT|IS_PROP_REINITABLE|IS_PROP_CTOR_REINITABLE);
-				}
+				Z_PROP_FLAG_P(variable_ptr) &= ~(IS_PROP_UNINIT|IS_PROP_REINITABLE);
 				value = &tmp;
 			}
 
@@ -1148,6 +1118,12 @@ found:;
 			variable_ptr = zend_assign_to_variable_ex(
 				variable_ptr, value, IS_TMP_VAR, property_uses_strict_types(), &garbage);
 
+			if (readonly_write_kind == ZEND_READONLY_WRITE_REINITABLE) {
+				Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE;
+			} else if (readonly_write_kind == ZEND_READONLY_WRITE_CTOR_REASSIGNED) {
+				Z_PROP_FLAG_P(variable_ptr) |= IS_PROP_CTOR_REASSIGNED;
+			}
+
 			if (garbage) {
 				if (GC_DELREF(garbage) == 0) {
 					zend_execute_data *execute_data = EG(current_execute_data);
@@ -1556,7 +1532,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
 			if (error) {
 				if ((prop_info->flags & ZEND_ACC_READONLY)
 				 && Z_TYPE_P(slot) != IS_UNDEF
-				 && !zend_readonly_property_is_reinitable_for_context(slot, prop_info)) {
+				 && !(Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE)) {
 					zend_readonly_property_unset_error(prop_info->ce, name);
 					return;
 				}

Zend/zend_object_handlers.h

@@ -277,12 +277,6 @@ ZEND_API HashTable *rebuild_object_properties_internal(zend_object *zobj);
 ZEND_API ZEND_COLD zend_never_inline void zend_bad_method_call(const zend_function *fbc, const zend_string *method_name, const zend_class_entry *scope);
 ZEND_API ZEND_COLD zend_never_inline void zend_abstract_method_call(const zend_function *fbc);
 
-/* Check if a readonly property has the REINITABLE flag (set by clone or CPP initialization) */
-static zend_always_inline bool zend_readonly_property_is_reinitable(const zval *property_val)
-{
-	return (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE) != 0;
-}
-
 static zend_always_inline HashTable *zend_std_get_properties_ex(zend_object *object)
 {
 	if (UNEXPECTED(zend_lazy_object_must_init(object))) {