Fix bug #78226: Don't call __set() on uninitialized typed properties

Assigning to an uninitialized typed property will no longer trigger
a call to __set(). However, calls to __set() are still triggered if
the property is explicitly unset().

This gives us both the behavior people generally expect, and still
allows ORMs to do lazy initialization by unsetting properties.

For PHP 8, we should fine a way to forbid unsetting of declared
properties entirely, and provide a different way to achieve lazy
initialization.

Author: nikic

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 7.4.0RC5
 
+- Core:
+  . Fixed bug #78226 (Unexpected __set behavior with typed properties). (Nikita)
+
 - COM:
   . Fixed bug #78694 (Appending to a variant array causes segfault). (cmb)
 

Zend/tests/type_declarations/typed_properties_magic_set.phpt

@@ -0,0 +1,54 @@
+--TEST--
+__set() should not be invoked when setting an uninitialized typed property
+--FILE--
+<?php
+
+class Test {
+    public int $foo;
+    public function __set($name, $value) {
+        echo "__set ", $name, " = ", $value, "\n";
+    }
+}
+
+$test = new Test;
+$test->foo = 42;
+var_dump($test->foo);
+
+// __set will be called after unset()
+unset($test->foo);
+$test->foo = 42;
+
+// __set will be called after unset() without prior initialization
+$test = new Test;
+unset($test->foo);
+$test->foo = 42;
+
+class Test2 extends Test {
+}
+
+// Check that inherited properties work correctly
+$test = new Test;
+$test->foo = 42;
+var_dump($test->foo);
+unset($test->foo);
+$test->foo = 42;
+
+// Test that cloning works correctly
+$test = clone $test;
+$test->foo = 42;
+$test = clone new Test;
+$test->foo = 42;
+var_dump($test->foo);
+unset($test->foo);
+$test->foo = 42;
+
+?>
+--EXPECT--
+int(42)
+__set foo = 42
+__set foo = 42
+int(42)
+__set foo = 42
+__set foo = 42
+int(42)
+__set foo = 42

Zend/zend_API.c

@@ -1263,13 +1263,13 @@ static zend_always_inline void _object_properties_init(zend_object *object, zend
 
 		if (UNEXPECTED(class_type->type == ZEND_INTERNAL_CLASS)) {
 			do {
-				ZVAL_COPY_OR_DUP(dst, src);
+				ZVAL_COPY_OR_DUP_PROP(dst, src);
 				src++;
 				dst++;
 			} while (src != end);
 		} else {
 			do {
-				ZVAL_COPY(dst, src);
+				ZVAL_COPY_PROP(dst, src);
 				src++;
 				dst++;
 			} while (src != end);
@@ -3725,6 +3725,7 @@ ZEND_API int zend_declare_typed_property(zend_class_entry *ce, zend_string *name
 			}
 		}
 	} else {
+		zval *property_default_ptr;
 		if ((property_info_ptr = zend_hash_find_ptr(&ce->properties_info, name)) != NULL &&
 		    (property_info_ptr->flags & ZEND_ACC_STATIC) == 0) {
 			property_info->offset = property_info_ptr->offset;
@@ -3745,7 +3746,9 @@ ZEND_API int zend_declare_typed_property(zend_class_entry *ce, zend_string *name
 				ce->properties_info_table[ce->default_properties_count - 1] = property_info;
 			}
 		}
-		ZVAL_COPY_VALUE(&ce->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)], property);
+		property_default_ptr = &ce->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)];
+		ZVAL_COPY_VALUE(property_default_ptr, property);
+		Z_PROP_FLAG_P(property_default_ptr) = Z_ISUNDEF_P(property) ? IS_PROP_UNINIT : 0;
 	}
 	if (ce->type & ZEND_INTERNAL_CLASS) {
 		switch(Z_TYPE_P(property)) {

Zend/zend_inheritance.c

@@ -1167,7 +1167,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			do {
 				dst--;
 				src--;
-				ZVAL_COPY_VALUE(dst, src);
+				ZVAL_COPY_VALUE_PROP(dst, src);
 			} while (dst != end);
 			pefree(src, ce->type == ZEND_INTERNAL_CLASS);
 			end = ce->default_properties_table;
@@ -1182,7 +1182,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			do {
 				dst--;
 				src--;
-				ZVAL_COPY_OR_DUP(dst, src);
+				ZVAL_COPY_OR_DUP_PROP(dst, src);
 				if (Z_OPT_TYPE_P(dst) == IS_CONSTANT_AST) {
 					ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
 				}
@@ -1192,7 +1192,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			do {
 				dst--;
 				src--;
-				ZVAL_COPY(dst, src);
+				ZVAL_COPY_PROP(dst, src);
 				if (Z_OPT_TYPE_P(dst) == IS_CONSTANT_AST) {
 					ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
 				}

Zend/zend_object_handlers.c

@@ -836,6 +836,11 @@ ZEND_API zval *zend_std_write_property(zval *object, zval *member, zval *value,
 			zend_assign_to_variable(variable_ptr, value, IS_TMP_VAR, EG(current_execute_data) && ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)));
 			goto exit;
 		}
+		if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) {
+			/* Writes to uninitializde typed properties bypass __set(). */
+			Z_PROP_FLAG_P(variable_ptr) = 0;
+			goto write_std_property;
+		}
 	} else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))) {
 		if (EXPECTED(zobj->properties != NULL)) {
 			if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) {
@@ -1113,6 +1118,8 @@ ZEND_API void zend_std_unset_property(zval *object, zval *member, void **cache_s
 			}
 			goto exit;
 		}
+		/* Reset the IS_PROP_UNINIT flag, if it exists. */
+		Z_PROP_FLAG_P(slot) = 0;
 	} else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))
 	 && EXPECTED(zobj->properties != NULL)) {
 		if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) {

Zend/zend_objects.c

@@ -209,7 +209,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 
 		do {
 			i_zval_ptr_dtor(dst);
-			ZVAL_COPY_VALUE(dst, src);
+			ZVAL_COPY_VALUE_PROP(dst, src);
 			zval_add_ref(dst);
 			if (UNEXPECTED(Z_ISREF_P(dst)) &&
 					(ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(dst)))) {

Zend/zend_types.h

@@ -1262,4 +1262,18 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
 		}												\
 	} while (0)
 
+/* Properties store a flag distinguishing unset and unintialized properties
+ * (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 Z_PROP_FLAG_P(z) Z_EXTRA_P(z)
+#define ZVAL_COPY_VALUE_PROP(z, v) \
+	do { *(z) = *(v); } while (0)
+#define ZVAL_COPY_PROP(z, v) \
+	do { ZVAL_COPY(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+#define ZVAL_COPY_OR_DUP_PROP(z, v) \
+	do { ZVAL_COPY_OR_DUP(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+
+
 #endif /* ZEND_TYPES_H */

ext/opcache/zend_accelerator_util_funcs.c

@@ -270,7 +270,7 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
 		end = src + ce->default_properties_count;
 		ce->default_properties_table = dst;
 		for (; src != end; src++, dst++) {
-			ZVAL_COPY_VALUE(dst, src);
+			ZVAL_COPY_VALUE_PROP(dst, src);
 		}
 	}