From 7342bea142aae5d6649dccc9a6ed2e950199e71d Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Thu, 13 Mar 2025 12:50:01 +0100 Subject: [PATCH 1/3] Fix lazy proxy calling magic methods twice --- Zend/tests/lazy_objects/gh18038-001.phpt | 29 +++++++ Zend/tests/lazy_objects/gh18038-002.phpt | 38 +++++++++ Zend/tests/lazy_objects/gh18038-003.phpt | 30 ++++++++ Zend/tests/lazy_objects/gh18038-004.phpt | 45 +++++++++++ Zend/tests/lazy_objects/gh18038-005.phpt | 28 +++++++ Zend/tests/lazy_objects/gh18038-006.phpt | 37 +++++++++ Zend/tests/lazy_objects/gh18038-007.phpt | 41 ++++++++++ Zend/tests/lazy_objects/gh18038-008.phpt | 28 +++++++ Zend/tests/lazy_objects/gh18038-009.phpt | 41 ++++++++++ Zend/tests/lazy_objects/gh18038-010.phpt | 36 +++++++++ Zend/tests/lazy_objects/gh18038-011.phpt | 45 +++++++++++ Zend/tests/lazy_objects/gh18038-012.phpt | 28 +++++++ Zend/zend_object_handlers.c | 98 +++++++++++++++++++----- 13 files changed, 503 insertions(+), 21 deletions(-) create mode 100644 Zend/tests/lazy_objects/gh18038-001.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-002.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-003.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-004.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-005.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-006.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-007.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-008.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-009.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-010.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-011.phpt create mode 100644 Zend/tests/lazy_objects/gh18038-012.phpt diff --git a/Zend/tests/lazy_objects/gh18038-001.phpt b/Zend/tests/lazy_objects/gh18038-001.phpt new file mode 100644 index 0000000000000..88e453c4a251f --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-001.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-18038 001: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-002.phpt b/Zend/tests/lazy_objects/gh18038-002.phpt new file mode 100644 index 0000000000000..4c12f21de8115 --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-002.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-18038 002: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-003.phpt b/Zend/tests/lazy_objects/gh18038-003.phpt new file mode 100644 index 0000000000000..1db0588a70cfa --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-003.phpt @@ -0,0 +1,30 @@ +--TEST-- +GH-18038 003: Lazy proxy calls magic methods twice +--FILE-- +$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 diff --git a/Zend/tests/lazy_objects/gh18038-004.phpt b/Zend/tests/lazy_objects/gh18038-004.phpt new file mode 100644 index 0000000000000..8810efb6bec2e --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-004.phpt @@ -0,0 +1,45 @@ +--TEST-- +GH-18038 004: Lazy proxy calls magic methods twice +--FILE-- +$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 diff --git a/Zend/tests/lazy_objects/gh18038-005.phpt b/Zend/tests/lazy_objects/gh18038-005.phpt new file mode 100644 index 0000000000000..b728d45253daf --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-005.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-18038 005: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-006.phpt b/Zend/tests/lazy_objects/gh18038-006.phpt new file mode 100644 index 0000000000000..256f0be8e202b --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-006.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-18038 006: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-007.phpt b/Zend/tests/lazy_objects/gh18038-007.phpt new file mode 100644 index 0000000000000..9925190a19801 --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-007.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-18038 007: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-008.phpt b/Zend/tests/lazy_objects/gh18038-008.phpt new file mode 100644 index 0000000000000..34d6e766163de --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-008.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-18038 008: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-009.phpt b/Zend/tests/lazy_objects/gh18038-009.phpt new file mode 100644 index 0000000000000..3c165a71ccffe --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-009.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-18038 009: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/tests/lazy_objects/gh18038-010.phpt b/Zend/tests/lazy_objects/gh18038-010.phpt new file mode 100644 index 0000000000000..0e9acb6836a92 --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-010.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-18038 010: Lazy proxy calls magic methods twice +--FILE-- +$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 +string(10) "C::__unset" +lazy proxy object(C)#%d (1) { + ["instance"]=> + object(C)#%d (1) { + ["_"]=> + NULL + } +} diff --git a/Zend/tests/lazy_objects/gh18038-011.phpt b/Zend/tests/lazy_objects/gh18038-011.phpt new file mode 100644 index 0000000000000..b356f9e4f384d --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-011.phpt @@ -0,0 +1,45 @@ +--TEST-- +GH-18038 011: Lazy proxy calls magic methods twice +--FILE-- +$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 + } +} diff --git a/Zend/tests/lazy_objects/gh18038-012.phpt b/Zend/tests/lazy_objects/gh18038-012.phpt new file mode 100644 index 0000000000000..5bc83333d81c7 --- /dev/null +++ b/Zend/tests/lazy_objects/gh18038-012.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-18038 012: Lazy proxy calls magic methods twice +--FILE-- +$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) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index a4dba771e3ef5..8bc9058baf9df 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -946,6 +946,18 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int 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 zend_always_inline 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 @@ found:; 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_ISSET)) { + (*guard) |= IN_ISSET; + zend_std_unset_property(zobj, name, cache_slot); + (*guard) &= ~IN_ISSET; + return; + } + } + zend_std_unset_property(zobj, name, cache_slot); return; } @@ -2323,6 +2367,8 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has } (*guard) &= ~IN_ISSET; OBJ_RELEASE(zobj); + } else { + goto lazy_init; } } @@ -2338,6 +2384,16 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has 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); } } From aa3f8edd278291b32487a28e7fd07c21db593766 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 17 Mar 2025 18:11:45 +0100 Subject: [PATCH 2/3] Fix guard propagation for unset() --- Zend/tests/lazy_objects/gh18038-010.phpt | 1 - Zend/zend_object_handlers.c | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Zend/tests/lazy_objects/gh18038-010.phpt b/Zend/tests/lazy_objects/gh18038-010.phpt index 0e9acb6836a92..584c18580a210 100644 --- a/Zend/tests/lazy_objects/gh18038-010.phpt +++ b/Zend/tests/lazy_objects/gh18038-010.phpt @@ -26,7 +26,6 @@ var_dump($obj); --EXPECTF-- string(10) "C::__unset" init -string(10) "C::__unset" lazy proxy object(C)#%d (1) { ["instance"]=> object(C)#%d (1) { diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 8bc9058baf9df..3719af30024d2 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1574,10 +1574,10 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void if (UNEXPECTED(guard)) { guard = zend_get_property_guard(zobj, name); - if (!((*guard) & IN_ISSET)) { - (*guard) |= IN_ISSET; + if (!((*guard) & IN_UNSET)) { + (*guard) |= IN_UNSET; zend_std_unset_property(zobj, name, cache_slot); - (*guard) &= ~IN_ISSET; + (*guard) &= ~IN_UNSET; return; } } From e911c063f12e46dca4c8d3df4062af83d1b2bd99 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 25 Mar 2025 11:07:24 +0100 Subject: [PATCH 3/3] Remove the zend_always_inline attribute --- Zend/zend_object_handlers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 3719af30024d2..4aea3501cf269 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -982,7 +982,7 @@ static zend_always_inline bool property_uses_strict_types(void) { && ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)); } -static zend_always_inline zval *forward_write_to_lazy_object(zend_object *zobj, +static zval *forward_write_to_lazy_object(zend_object *zobj, zend_string *name, zval *value, void **cache_slot, bool guarded) { zval *variable_ptr;