Fixed bug #38220 (Crash on some object operations)

diff --git a/NEWS b/NEWS
index 3da303319fed3bf0384891fd6284bfcb692492ef..17f35c85e3951bcd1fe1581282564ed9ee42b32a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ PHP                                                                        NEWS
   . ext/filepro (Derick, Tony)
   . ext/hwapi (Derick, Tony)
 
+- Fixed bug #38220 (Crash on some object operations). (Dmitry)
 - Fixed bug #38217 (ReflectionClass::newInstanceArgs() tries to allocate too 
   much memory). (Tony)
 - Fixed bug #38214 (gif interlace output cannot work). (Pierre)

diff --git a/Zend/tests/bug38220.phpt b/Zend/tests/bug38220.phpt
new file mode 100755 (executable)
index 0000000..d64e409
--- /dev/null
+++ b/Zend/tests/bug38220.phpt
@@ -0,0 +1,92 @@
+--TEST--
+Bug #38220 Crash on some object operations 
+--FILE--
+<?php
+class drv {
+       public $obj;
+
+       function func1() {
+               echo "func1(): {$this->obj->i}\n";
+       }
+
+       function close() {
+               echo "close(): {$this->obj->i}\n";
+       }
+}
+
+class A {
+       public $i;
+
+       function __construct($i) {
+               $this->i = $i;
+
+       }
+
+       function __call($method, $args) {
+               $drv = myserv::drv();
+
+               $drv->obj = $this;
+
+               echo "before call $method\n";
+               print_r($this);
+               call_user_func_array(array($drv, $method), $args);
+               echo "after call $method\n";
+
+               // Uncomment this line to work without crash
+//             $drv->obj = null;
+       }
+
+       function __destruct() {
+               echo "A::__destruct()\n";
+               $this->close();
+       }
+}
+
+class myserv {
+       private static $drv = null;
+
+       static function drv() {
+               if (is_null(self::$drv))
+                       self::$drv = new drv;
+               return self::$drv;
+       }
+}
+
+$obj1 = new A(1);
+$obj1->func1();
+
+$obj2 = new A(2);
+unset($obj1);
+$obj2->func1();
+?>
+--EXPECT--
+before call func1
+A Object
+(
+    [i] => 1
+)
+func1(): 1
+after call func1
+A::__destruct()
+before call close
+A Object
+(
+    [i] => 1
+)
+close(): 1
+after call close
+before call func1
+A Object
+(
+    [i] => 2
+)
+func1(): 1
+after call func1
+A::__destruct()
+before call close
+A Object
+(
+    [i] => 2
+)
+close(): 2
+after call close

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
index fe6328e1579aa807d7c54077c05f330a76a68b54..70e51426fd60a3b2ace8ca1eb4d0e2b9858d39ca 100644 (file)
--- a/Zend/zend_object_handlers.c
+++ b/Zend/zend_object_handlers.c
@@ -373,7 +373,6 @@ static void zend_std_write_property(zval *object, zval *member, zval *value TSRM
        zend_object *zobj;
        zval *tmp_member = NULL;
        zval **variable_ptr;
-       int setter_done = 0;
        zend_property_info *property_info;
 
        zobj = Z_OBJ_P(object);
@@ -390,10 +389,8 @@ static void zend_std_write_property(zval *object, zval *member, zval *value TSRM
        property_info = zend_get_property_info(zobj->ce, member, (zobj->ce->__set != NULL) TSRMLS_CC);
 
        if (property_info && zend_hash_quick_find(zobj->properties, property_info->name, property_info->name_length+1, property_info->h, (void **) &variable_ptr) == SUCCESS) {
-               if (*variable_ptr == value) {
-                       /* if we already have this value there, we don't actually need to do anything */
-                       setter_done = 1;
-               } else {
+               /* if we already have this value there, we don't actually need to do anything */
+               if (*variable_ptr != value) {
                        /* if we are assigning reference, we shouldn't move it, but instead assign variable
                           to the same pointer */
                        if (PZVAL_IS_REF(*variable_ptr)) {
@@ -406,10 +403,20 @@ static void zend_std_write_property(zval *object, zval *member, zval *value TSRM
                                        zval_copy_ctor(*variable_ptr);
                                }
                                zval_dtor(&garbage);
-                               setter_done = 1;
+                       } else {
+                               zval *garbage = *variable_ptr;
+
+                               /* if we assign referenced variable, we should separate it */
+                               value->refcount++;
+                               if (PZVAL_IS_REF(value)) {
+                                       SEPARATE_ZVAL(&value);
+                               }
+                               *variable_ptr = value;
+                               zval_ptr_dtor(&garbage);
                        }
                }
        } else {
+               int setter_done = 0;
                zend_guard *guard;
 
                if (zobj->ce->__set &&
@@ -422,18 +429,18 @@ static void zend_std_write_property(zval *object, zval *member, zval *value TSRM
                        setter_done = 1;
                        guard->in_set = 0;
                }
-       }
-
-       if (!setter_done) {
-               zval **foo;
+               if (!setter_done) {
+                       zval **foo;
 
-               /* if we assign referenced variable, we should separate it */
-               value->refcount++;
-               if (PZVAL_IS_REF(value)) {
-                       SEPARATE_ZVAL(&value);
+                       /* if we assign referenced variable, we should separate it */
+                       value->refcount++;
+                       if (PZVAL_IS_REF(value)) {
+                               SEPARATE_ZVAL(&value);
+                       }
+                       zend_hash_quick_update(zobj->properties, property_info->name, property_info->name_length+1, property_info->h, &value, sizeof(zval *), (void **) &foo);
                }
-               zend_hash_quick_update(zobj->properties, property_info->name, property_info->name_length+1, property_info->h, &value, sizeof(zval *), (void **) &foo);
        }
+
        if (tmp_member) {
                zval_ptr_dtor(&tmp_member);
        }

diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
index 6dc4ba4aa356464d4b6f7bf3c13489f245b1612b..bb713680d6b508c4447f37cd55460494742ba284 100644 (file)
--- a/Zend/zend_objects.c
+++ b/Zend/zend_objects.c
@@ -52,7 +52,7 @@ ZEND_API void zend_objects_destroy_object(zend_object *object, zend_object_handl
        zend_function *destructor = object->ce->destructor;
 
        if (destructor) {
-               zval zobj, *obj = &zobj;
+               zval *obj;
                zval *old_exception;
 
                if (destructor->op_array.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) {
@@ -85,10 +85,12 @@ ZEND_API void zend_objects_destroy_object(zend_object *object, zend_object_handl
                        }
                }
 
-               Z_TYPE(zobj) = IS_OBJECT;
-               Z_OBJ_HANDLE(zobj) = handle;
-               Z_OBJ_HT(zobj) = &std_object_handlers;
-               INIT_PZVAL(obj);
+               MAKE_STD_ZVAL(obj);
+               Z_TYPE_P(obj) = IS_OBJECT;
+               Z_OBJ_HANDLE_P(obj) = handle;
+               /* TODO: We cannot set proper handlers. */
+               Z_OBJ_HT_P(obj) = &std_object_handlers; 
+               zval_copy_ctor(obj);
 
                /* Make sure that destructors are protected from previously thrown exceptions.
                 * For example, if an exception was thrown in a function and when the function's
@@ -103,6 +105,7 @@ ZEND_API void zend_objects_destroy_object(zend_object *object, zend_object_handl
                                zval *file = zend_read_property(default_exception_ce, old_exception, "file", sizeof("file")-1, 1 TSRMLS_CC);
                                zval *line = zend_read_property(default_exception_ce, old_exception, "line", sizeof("line")-1, 1 TSRMLS_CC);
 
+                               zval_ptr_dtor(&obj);
                                zval_ptr_dtor(&EG(exception));
                                EG(exception) = old_exception;
                                zend_error(E_ERROR, "Ignoring exception from %s::__destruct() while an exception is already active (Uncaught %s in %s on line %ld)", 
@@ -110,6 +113,7 @@ ZEND_API void zend_objects_destroy_object(zend_object *object, zend_object_handl
                        }
                        EG(exception) = old_exception;
                }
+               zval_ptr_dtor(&obj);
        }
 }