Fixed bug #38220 (Crash on some object operations)

3 files changed, 123 insertions, 20 deletions

diff --git a/Zend/tests/bug38220.phpt b/Zend/tests/bug38220.phpt
new file mode 100755
index 0000000000..d64e409778
--- /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 fe6328e157..70e51426fd 100644
--- 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 6dc4ba4aa3..bb713680d6 100644
--- 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);
 	}
 }