Fix lazy proxy calling magic methods twice

Fixes GH-18038
Closes GH-18039

NEWS

@@ -5,6 +5,7 @@ PHP                                                                        NEWS
 - Core:
   . Fixed bug GH-17711 and GH-18022 (Infinite recursion on deprecated attribute
     evaluation). (ilutov)
+  . Fixed bug GH-18038 (Lazy proxy calls magic methods twice). (Arnaud)
 
 - Standard:
   . Fixed bug GH-18145 (php8ts crashes in php_clear_stat_cache()).

Zend/tests/lazy_objects/gh18038-001.phpt

@@ -0,0 +1,29 @@
+--TEST--
+GH-18038 001: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __set($name, $value) {
+        var_dump(__METHOD__);
+        $this->$name = $value * 2;
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+$obj->prop = 1;
+var_dump($obj->prop);
+
+?>
+--EXPECT--
+string(8) "C::__set"
+init
+int(2)

Zend/tests/lazy_objects/gh18038-002.phpt

@@ -0,0 +1,38 @@
+--TEST--
+GH-18038 002: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class RealInstance {
+    public $_;
+    public function __set($name, $value) {
+        global $obj;
+        var_dump(get_class($this)."::".__FUNCTION__);
+        $obj->$name = $value * 2;
+        unset($this->$name);
+        $this->$name = $value * 2;
+    }
+}
+
+#[AllowDynamicProperties]
+class Proxy extends RealInstance {
+}
+
+$rc = new ReflectionClass(Proxy::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new RealInstance;
+});
+
+$real = $rc->initializeLazyObject($obj);
+$real->prop = 1;
+var_dump($obj->prop);
+
+?>
+--EXPECT--
+init
+string(19) "RealInstance::__set"
+string(12) "Proxy::__set"
+int(2)

Zend/tests/lazy_objects/gh18038-003.phpt

@@ -0,0 +1,30 @@
+--TEST--
+GH-18038 003: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __get($name) {
+        var_dump(__METHOD__);
+        return $this->$name;
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+var_dump($obj->prop);
+
+?>
+--EXPECTF--
+string(8) "C::__get"
+init
+
+Warning: Undefined property: C::$prop in %s on line %d
+NULL

Zend/tests/lazy_objects/gh18038-004.phpt

@@ -0,0 +1,45 @@
+--TEST--
+GH-18038 004: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class RealInstance {
+    public $_;
+    public function __get($name) {
+        global $obj;
+        var_dump(get_class($this)."::".__FUNCTION__);
+        var_dump($obj->$name);
+        return $this->$name;
+    }
+}
+
+#[AllowDynamicProperties]
+class Proxy extends RealInstance {
+    public function __get($name) {
+        var_dump(get_class($this)."::".__FUNCTION__);
+        return $this->$name;
+    }
+}
+
+$rc = new ReflectionClass(Proxy::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new RealInstance;
+});
+
+$real = $rc->initializeLazyObject($obj);
+var_dump($real->prop);
+
+?>
+--EXPECTF--
+init
+string(19) "RealInstance::__get"
+string(12) "Proxy::__get"
+
+Warning: Undefined property: RealInstance::$prop in %s on line %d
+NULL
+
+Warning: Undefined property: RealInstance::$prop in %s on line %d
+NULL

Zend/tests/lazy_objects/gh18038-005.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-18038 005: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __isset($name) {
+        var_dump(__METHOD__);
+        return isset($this->$name['']);
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+var_dump(isset($obj->prop['']));
+
+?>
+--EXPECT--
+string(10) "C::__isset"
+init
+bool(false)

Zend/tests/lazy_objects/gh18038-006.phpt

@@ -0,0 +1,37 @@
+--TEST--
+GH-18038 006: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __isset($name) {
+        var_dump(__METHOD__);
+        return isset($this->$name['']);
+    }
+    public function __get($name) {
+        var_dump(__METHOD__);
+        return $this->$name[''];
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+var_dump(isset($obj->prop['']));
+
+?>
+--EXPECTF--
+string(10) "C::__isset"
+string(8) "C::__get"
+init
+
+Warning: Undefined property: C::$prop in %s on line %d
+
+Warning: Trying to access array offset on null in %s on line %d
+bool(false)

Zend/tests/lazy_objects/gh18038-007.phpt

@@ -0,0 +1,41 @@
+--TEST--
+GH-18038 007: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class RealInstance {
+    public $_;
+    public function __isset($name) {
+        global $obj;
+        var_dump(get_class($this)."::".__FUNCTION__);
+        var_dump(isset($obj->$name['']));
+        return isset($this->$name['']);
+    }
+}
+
+#[AllowDynamicProperties]
+class Proxy extends RealInstance {
+    public function __isset($name) {
+        var_dump(get_class($this)."::".__FUNCTION__);
+        return isset($this->$name['']);
+    }
+}
+
+$rc = new ReflectionClass(Proxy::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new RealInstance;
+});
+
+$real = $rc->initializeLazyObject($obj);
+var_dump(isset($real->prop['']));
+
+?>
+--EXPECT--
+init
+string(21) "RealInstance::__isset"
+string(14) "Proxy::__isset"
+bool(false)
+bool(false)

Zend/tests/lazy_objects/gh18038-008.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-18038 008: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __isset($name) {
+        var_dump(__METHOD__);
+        return isset($this->$name);
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+var_dump(isset($obj->prop));
+
+?>
+--EXPECT--
+string(10) "C::__isset"
+init
+bool(false)

Zend/tests/lazy_objects/gh18038-009.phpt

@@ -0,0 +1,41 @@
+--TEST--
+GH-18038 009: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class RealInstance {
+    public $_;
+    public function __isset($name) {
+        global $obj;
+        var_dump(get_class($this)."::".__FUNCTION__);
+        var_dump(isset($obj->$name));
+        return isset($this->$name);
+    }
+}
+
+#[AllowDynamicProperties]
+class Proxy extends RealInstance {
+    public function __isset($name) {
+        var_dump(get_class($this)."::".__FUNCTION__);
+        return isset($this->$name);
+    }
+}
+
+$rc = new ReflectionClass(Proxy::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new RealInstance;
+});
+
+$real = $rc->initializeLazyObject($obj);
+var_dump(isset($real->prop));
+
+?>
+--EXPECT--
+init
+string(21) "RealInstance::__isset"
+string(14) "Proxy::__isset"
+bool(false)
+bool(false)

Zend/tests/lazy_objects/gh18038-010.phpt

@@ -0,0 +1,35 @@
+--TEST--
+GH-18038 010: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __unset($name) {
+        var_dump(__METHOD__);
+        unset($this->$name);
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new C;
+});
+
+unset($obj->prop);
+var_dump($obj);
+
+?>
+--EXPECTF--
+string(10) "C::__unset"
+init
+lazy proxy object(C)#%d (1) {
+  ["instance"]=>
+  object(C)#%d (1) {
+    ["_"]=>
+    NULL
+  }
+}

Zend/tests/lazy_objects/gh18038-011.phpt

@@ -0,0 +1,45 @@
+--TEST--
+GH-18038 011: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class RealInstance {
+    public $_;
+    public function __unset($name) {
+        global $obj;
+        var_dump(get_class($this)."::".__FUNCTION__);
+        unset($this->$name);
+    }
+}
+
+#[AllowDynamicProperties]
+class Proxy extends RealInstance {
+    public function __isset($name) {
+        var_dump(get_class($this)."::".__FUNCTION__);
+        unset($this->$name);
+    }
+}
+
+$rc = new ReflectionClass(Proxy::class);
+
+$obj = $rc->newLazyProxy(function () {
+    echo "init\n";
+    return new RealInstance;
+});
+
+$real = $rc->initializeLazyObject($obj);
+unset($real->prop);
+var_dump($obj);
+
+?>
+--EXPECTF--
+init
+string(21) "RealInstance::__unset"
+lazy proxy object(Proxy)#%d (1) {
+  ["instance"]=>
+  object(RealInstance)#%d (1) {
+    ["_"]=>
+    NULL
+  }
+}

Zend/tests/lazy_objects/gh18038-012.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-18038 012: Lazy proxy calls magic methods twice
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class C {
+    public $_;
+    public function __set($name, $value) {
+        var_dump(__METHOD__);
+        $this->$name = $value * 2;
+    }
+}
+
+$rc = new ReflectionClass(C::class);
+
+$obj = $rc->newLazyGhost(function () {
+    echo "init\n";
+});
+
+$obj->prop = 1;
+var_dump($obj->prop);
+
+?>
+--EXPECT--
+string(8) "C::__set"
+init
+int(2)

Zend/zend_object_handlers.c

@@ -946,6 +946,18 @@ uninit_error:
 				goto exit;
 			}
 
+			if (UNEXPECTED(guard)) {
+				uint32_t guard_type = (type == BP_VAR_IS) && zobj->ce->__isset
+					? IN_ISSET : IN_GET;
+				guard = zend_get_property_guard(zobj, name);
+				if (!((*guard) & guard_type)) {
+					(*guard) |= guard_type;
+					retval = zend_std_read_property(zobj, name, type, cache_slot, rv);
+					(*guard) &= ~guard_type;
+					return retval;
+				}
+			}
+
 			return zend_std_read_property(zobj, name, type, cache_slot, rv);
 		}
 	}
@@ -970,6 +982,43 @@ static zend_always_inline bool property_uses_strict_types(void) {
 		&& ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data));
 }
 
+static zval *forward_write_to_lazy_object(zend_object *zobj,
+		zend_string *name, zval *value, void **cache_slot, bool guarded)
+{
+	zval *variable_ptr;
+
+	/* backup value as it may change during initialization */
+	zval backup;
+	ZVAL_COPY(&backup, value);
+
+	zend_object *instance = zend_lazy_object_init(zobj);
+	if (UNEXPECTED(!instance)) {
+		zval_ptr_dtor(&backup);
+		return &EG(error_zval);
+	}
+
+	if (UNEXPECTED(guarded)) {
+		uint32_t *guard = zend_get_property_guard(instance, name);
+		if (!((*guard) & IN_SET)) {
+			(*guard) |= IN_SET;
+			variable_ptr = zend_std_write_property(instance, name, &backup, cache_slot);
+			(*guard) &= ~IN_SET;
+			goto exit;
+		}
+	}
+
+	variable_ptr = zend_std_write_property(instance, name, &backup, cache_slot);
+
+exit:
+	zval_ptr_dtor(&backup);
+
+	if (variable_ptr == &backup) {
+		variable_ptr = value;
+	}
+
+	return variable_ptr;
+}
+
 ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zval *value, void **cache_slot) /* {{{ */
 {
 	zval *variable_ptr, tmp;
@@ -1151,7 +1200,8 @@ found:;
 			variable_ptr = value;
 		} else if (EXPECTED(!IS_WRONG_PROPERTY_OFFSET(property_offset))) {
 			if (UNEXPECTED(zend_lazy_object_must_init(zobj))) {
-				goto lazy_init;
+				return forward_write_to_lazy_object(zobj, name, value,
+						cache_slot, /* guarded */ true);
 			}
 
 			goto write_std_property;
@@ -1198,26 +1248,9 @@ write_std_property:
 exit:
 	return variable_ptr;
 
-lazy_init:;
-	/* backup value as it may change during initialization */
-	zval backup;
-	ZVAL_COPY(&backup, value);
-
-	zobj = zend_lazy_object_init(zobj);
-	if (UNEXPECTED(!zobj)) {
-		variable_ptr = &EG(error_zval);
-		zval_ptr_dtor(&backup);
-		goto exit;
-	}
-
-	variable_ptr = zend_std_write_property(zobj, name, &backup, cache_slot);
-	zval_ptr_dtor(&backup);
-
-	if (variable_ptr == &backup) {
-		variable_ptr = value;
-	}
-
-	return variable_ptr;
+lazy_init:
+	return forward_write_to_lazy_object(zobj, name, value, cache_slot,
+			/* guarded */ false);
 }
 /* }}} */
 
@@ -1538,6 +1571,17 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
 		if (!zobj) {
 			return;
 		}
+
+		if (UNEXPECTED(guard)) {
+			guard = zend_get_property_guard(zobj, name);
+			if (!((*guard) & IN_UNSET)) {
+				(*guard) |= IN_UNSET;
+				zend_std_unset_property(zobj, name, cache_slot);
+				(*guard) &= ~IN_UNSET;
+				return;
+			}
+		}
+
 		zend_std_unset_property(zobj, name, cache_slot);
 		return;
 	}
@@ -2323,6 +2367,8 @@ found:
 			}
 			(*guard) &= ~IN_ISSET;
 			OBJ_RELEASE(zobj);
+		} else {
+			goto lazy_init;
 		}
 	}
 
@@ -2338,6 +2384,16 @@ lazy_init:
 				goto exit;
 			}
 
+			if (UNEXPECTED(zobj->ce->__isset)) {
+				uint32_t *guard = zend_get_property_guard(zobj, name);
+				if (!((*guard) & IN_ISSET)) {
+					(*guard) |= IN_ISSET;
+					result = zend_std_has_property(zobj, name, has_set_exists, cache_slot);
+					(*guard) &= ~IN_ISSET;
+					return result;
+				}
+			}
+
 			return zend_std_has_property(zobj, name, has_set_exists, cache_slot);
 		}
 	}