Revert "Allow random $this on non-internal Closures again"

This reverts commit 35d0405c4790f0ce668c9e1b8b05197e55d29a05.

11 files changed, 87 insertions, 78 deletions

diff --git a/Zend/tests/bug70630.phpt b/Zend/tests/bug70630.phpt
index 8a42a1f5a4..a46c460df0 100644
--- a/Zend/tests/bug70630.phpt
+++ b/Zend/tests/bug70630.phpt
@@ -4,7 +4,7 @@ Bug #70630 (Closure::call/bind() crash with ReflectionFunction->getClosure())
 <?php
 
 class a {}
-function foo() { print "ok\n"; }
+function foo() {}
 
 foreach (["substr", "foo"] as $fn) {
 	$x = (new ReflectionFunction($fn))->getClosure();
@@ -15,9 +15,10 @@ foreach (["substr", "foo"] as $fn) {
 ?>
 --EXPECTF--
 
-Warning: substr() expects at least 2 parameters, 0 given in %s on line %d
+Warning: Cannot bind function substr to an object in %s on line %d
 
-Warning: Cannot bind function substr to a class scope in %s on line %d
-ok
+Warning: Cannot bind function substr to an object or class in %s on line %d
 
-Warning: Cannot bind function foo to a class scope in %s on line %d
+Warning: Cannot bind function foo to an object in %s on line %d
+
+Warning: Cannot bind function foo to an object or class in %s on line %d
diff --git a/Zend/tests/closure_041.phpt b/Zend/tests/closure_041.phpt
index 6c368f6e17..947517b4ed 100644
--- a/Zend/tests/closure_041.phpt
+++ b/Zend/tests/closure_041.phpt
@@ -53,9 +53,9 @@ $d = $nonstaticScoped->bindTo(null); $d(); echo "\n";
 $d->bindTo($d);
 
 echo "After binding, with same-class instance for the bound ones", "\n";
-$d = $staticUnscoped->bindTo(new A); /* $d(); */ echo "\n";
+$d = $staticUnscoped->bindTo(new A); $d(); echo "\n";
 $d = $nonstaticUnscoped->bindTo(new A); $d(); echo " (should be scoped to dummy class)\n";
-$d = $staticScoped->bindTo(new A); /* $d(); */ echo "\n";
+$d = $staticScoped->bindTo(new A); $d(); echo "\n";
 $d = $nonstaticScoped->bindTo(new A); $d(); echo "\n";
 
 echo "After binding, with different instance for the bound ones", "\n";
@@ -64,7 +64,6 @@ $d = $nonstaticScoped->bindTo(new B); $d(); echo "\n";
 
 echo "Done.\n";
 
-?>
 --EXPECTF--
 Before binding
 scoped to A: bool(false)
@@ -87,12 +86,14 @@ bound: no
 After binding, with same-class instance for the bound ones
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
-
+scoped to A: bool(false)
+bound: no
 scoped to A: bool(false)
 bound: A (should be scoped to dummy class)
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
-
+scoped to A: bool(true)
+bound: no
 scoped to A: bool(true)
 bound: A
 After binding, with different instance for the bound ones
diff --git a/Zend/tests/closure_043.phpt b/Zend/tests/closure_043.phpt
index 7763b41d5e..98c88fda39 100644
--- a/Zend/tests/closure_043.phpt
+++ b/Zend/tests/closure_043.phpt
@@ -26,20 +26,19 @@ $d = $staticUnscoped->bindTo(null, null); $d(); echo "\n";
 $d = $staticScoped->bindTo(null, null); $d(); echo "\n";
 
 echo "After binding, null scope, with instance", "\n";
-$d = $staticUnscoped->bindTo(new A, null); /* $d(); */ echo "\n";
-$d = $staticScoped->bindTo(new A, null); /* $d();n*/ echo "\n";
+$d = $staticUnscoped->bindTo(new A, null); $d(); echo "\n";
+$d = $staticScoped->bindTo(new A, null); $d(); echo "\n";
 
 echo "After binding, with scope, no instance", "\n";
 $d = $staticUnscoped->bindTo(null, 'A'); $d(); echo "\n";
 $d = $staticScoped->bindTo(null, 'A'); $d(); echo "\n";
 
 echo "After binding, with scope, with instance", "\n";
-$d = $staticUnscoped->bindTo(new A, 'A'); /* $d(); */ echo "\n";
-$d = $staticScoped->bindTo(new A, 'A'); /* $d(); */ echo "\n";
+$d = $staticUnscoped->bindTo(new A, 'A'); $d(); echo "\n";
+$d = $staticScoped->bindTo(new A, 'A'); $d(); echo "\n";
 
 echo "Done.\n";
 
-?>
 --EXPECTF--
 Before binding
 bool(false)
@@ -58,9 +57,13 @@ bool(false)
 After binding, null scope, with instance
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
+bool(false)
+bool(false)
 
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
+bool(false)
+bool(false)
 
 After binding, with scope, no instance
 bool(true)
@@ -72,8 +75,12 @@ bool(false)
 After binding, with scope, with instance
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
+bool(true)
+bool(false)
 
 
 Warning: Cannot bind an instance to a static closure in %s on line %d
+bool(true)
+bool(false)
 
 Done.
diff --git a/Zend/tests/closure_061.phpt b/Zend/tests/closure_061.phpt
index 344b9438be..342b05ca5d 100644
--- a/Zend/tests/closure_061.phpt
+++ b/Zend/tests/closure_061.phpt
@@ -1,5 +1,5 @@
 --TEST--
-Closure::call() or Closure::bind() to independent class
+Closure::call() or Closure::bind() to independent class must fail
 --FILE--
 <?php
 
@@ -53,4 +53,11 @@ var_dump($baz->getVar());
 --EXPECTF--
 string(3) "baz"
 string(3) "bar"
-string(3) "foo"
+
+Warning: Cannot bind function foo::initClass to object of class baz in %s on line %d
+
+Fatal error: Uncaught Error: Function name must be a string in %s:%d
+Stack trace:
+#0 %s(%d): callMethodOn('foo', 'initClass', Object(baz))
+#1 {main}
+  thrown in %s on line %d
diff --git a/Zend/tests/closure_062.phpt b/Zend/tests/closure_062.phpt
index 3c147e89f1..574b43a3c2 100644
--- a/Zend/tests/closure_062.phpt
+++ b/Zend/tests/closure_062.phpt
@@ -1,9 +1,9 @@
 --TEST--
-Test Closure binding to unknown scopes with Closure::call()
+Force failure with Closure binding to unknown scopes/$this with Closure::call()
 --FILE--
 <?php
 
-function foo() { echo get_class($this), "\n"; }
+function foo() { print "FAIL\n"; }
 $x = (new ReflectionFunction("foo"))->getClosure();
 $x->call(new stdClass);
 
@@ -26,17 +26,14 @@ class d { function foo() { print "Success\n"; yield; } }
 $x = (new ReflectionMethod("d", "foo"))->getClosure(new d);
 $x->call(new d)->current();
 
-// internal functions with unknown scope must fail
-$x = (new ReflectionMethod("Closure", "bindTo"))->getClosure(function() {});
-$x->call(new a);
-
 ?>
 --EXPECTF--
-stdClass
-stdClass
+
+Warning: Cannot bind function foo to an object in %s on line %d
+
+Warning: Cannot bind function a::foo to object of class stdClass in %s on line %d
 b
-stdClass
+
+Warning: Cannot bind function c::foo to object of class stdClass in %s on line %d
 a
 Success
-
-Warning: Cannot bind closure of internal method Closure::bindTo to unrelated object of class a in %s on line %d
diff --git a/Zend/tests/closure_063.phpt b/Zend/tests/closure_063.phpt
index d826a5efd0..e854b578e2 100644
--- a/Zend/tests/closure_063.phpt
+++ b/Zend/tests/closure_063.phpt
@@ -3,18 +3,17 @@ Force failure with Closure binding to unknown scopes/$this with Closure::bind()
 --FILE--
 <?php
 
-function foo() { echo get_class($this), "\n"; }
+function foo() { print "FAIL\n"; }
 $x = (new ReflectionFunction("foo"))->getClosure();
-$x->bindTo(new stdClass)();
-$x->bindTo(new stdClass, "stdClass");
+$x->bindTo(new stdClass);
 
 class a { function foo() { echo get_class($this), "\n"; } }
 $x = (new ReflectionMethod("a", "foo"))->getClosure(new a);
-$x->bindTo(new stdClass)();
+$x->bindTo(new stdClass);
 
 class c extends stdClass { function foo() { echo get_class($this), "\n"; } }
 $x = (new ReflectionMethod("c", "foo"))->getClosure(new c);
-$x->bindTo(new stdClass)();
+$x->bindTo(new stdClass);
 
 class b extends a {}
 $x = (new ReflectionMethod("a", "foo"))->getClosure(new a);
@@ -25,12 +24,6 @@ $x->bindTo(new b, "c");
 $x = (new ReflectionMethod("a", "foo"))->getClosure(new a);
 $x->bindTo(new a)();
 
-class z extends SplStack {}
-$x = (new ReflectionMethod("SplStack", "pop"))->getClosure(new SplStack);
-$z = new z; $z->push(20);
-var_dump($x->bindTo($z)());
-$x->bindTo($z, "z");
-
 class d { function foo() { print "Success\n"; yield; } }
 class e extends d {}
 $x = (new ReflectionMethod("d", "foo"))->getClosure(new d);
@@ -41,19 +34,17 @@ $x->bindTo(new e, "e");
 
 ?>
 --EXPECTF--
-stdClass
 
-Warning: Cannot bind closure to scope of internal class stdClass in %s on line %d
-stdClass
-stdClass
+Warning: Cannot bind function foo to an object or class in %s on line %d
+
+Warning: Cannot bind function a::foo to object of class stdClass in %s on line %d
+
+Warning: Cannot bind function c::foo to object of class stdClass in %s on line %d
 b
 b
 
 Warning: Cannot bind function a::foo to scope class c in %s on line %d
 a
-int(20)
-
-Warning: Cannot bind function SplDoublyLinkedList::pop to scope class z in %s on line %d
 Success
 Success
 Success
diff --git a/Zend/tests/closure_call.phpt b/Zend/tests/closure_call.phpt
index ecaa12eb40..e8bed36aec 100644
--- a/Zend/tests/closure_call.phpt
+++ b/Zend/tests/closure_call.phpt
@@ -36,6 +36,7 @@ $elePHPant->x = 7;
 // Try on a StdClass
 var_dump($bar->call($elePHPant));
 
+
 $beta = function ($z) {
     return $this->x * $z;
 };
@@ -59,6 +60,8 @@ $foo->call(new FooBar);
 int(0)
 int(0)
 int(3)
-int(7)
+
+Warning: Cannot bind closure to object of internal class stdClass in %s line %d
+NULL
 int(21)
 int(3)
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index 03f7153bb3..c06aac609d 100644
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -94,11 +94,22 @@ ZEND_METHOD(Closure, call)
 		return;
 	}
 
+	if (closure->func.type != ZEND_USER_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) {
+		/* verify that we aren't binding methods to a wrong object */
+		if (closure->func.common.scope == NULL) {
+			zend_error(E_WARNING, "Cannot bind function %s to an object", ZSTR_VAL(closure->func.common.function_name));
+			return;
+		} else if (!instanceof_function(Z_OBJCE_P(newthis), closure->func.common.scope)) {
+			zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(Z_OBJCE_P(newthis)->name));
+			return;
+		}
+	}
+
 	newobj = Z_OBJ_P(newthis);
 
-	if (closure->func.type == ZEND_INTERNAL_FUNCTION && closure->func.common.scope != NULL && !instanceof_function(newobj->ce, closure->func.common.scope)) {
+	if (newobj->ce != closure->func.common.scope && newobj->ce->type == ZEND_INTERNAL_CLASS) {
 		/* rebinding to internal class is not allowed */
-		zend_error(E_WARNING, "Cannot bind closure of internal method %s::%s to unrelated object of class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(newobj->ce->name));
+		zend_error(E_WARNING, "Cannot bind closure to object of internal class %s", ZSTR_VAL(newobj->ce->name));
 		return;
 	}
 
@@ -127,7 +138,7 @@ ZEND_METHOD(Closure, call)
 	}
 
 	if (closure->func.type == ZEND_USER_FUNCTION && (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE)) {
-		/* use scope of passed object; we must not change scope of functions and methods, only true Closures */
+		/* use scope of passed object */
 		fci_cache.function_handler->common.scope = Z_OBJCE_P(newthis);
 
 		/* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */
@@ -166,7 +177,6 @@ ZEND_METHOD(Closure, bind)
 
 	if ((newthis != NULL) && (closure->func.common.fn_flags & ZEND_ACC_STATIC)) {
 		zend_error(E_WARNING, "Cannot bind an instance to a static closure");
-		RETURN_NULL();
 	}
 
 	if (scope_arg != NULL) { /* scope argument was given */
@@ -188,7 +198,7 @@ ZEND_METHOD(Closure, bind)
 		if (ce && ce != closure->func.common.scope && ce->type == ZEND_INTERNAL_CLASS) {
 			/* rebinding to internal class is not allowed */
 			zend_error(E_WARNING, "Cannot bind closure to scope of internal class %s", ZSTR_VAL(ce->name));
-			RETURN_NULL();
+			return;
 		}
 	} else { /* scope argument not given; do not change the scope by default */
 		ce = closure->func.common.scope;
@@ -520,37 +530,32 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
 {
 	zend_closure *closure;
 
+	if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) {
+		/* use dummy scope if we're binding an object without specifying a scope */
+		/* maybe it would be better to create one for this purpose */
+		scope = zend_ce_closure;
+	}
+
 	if (func->type != ZEND_USER_FUNCTION || (func->common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) {
-		/* verify that we aren't binding a internal function to a wrong scope */
+		/* verify that we aren't binding internal function to a wrong scope */
 		if (func->common.scope != NULL) {
-			if (scope != func->common.scope) {
-				if (scope) {
-					zend_error(E_WARNING, "Cannot bind function %s::%s to scope class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(scope->name));
-				} else {
-					zend_error(E_WARNING, "Cannot unbind function %s::%s from its scope", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name));
-				}
+			if (scope && scope != func->common.scope) {
+				zend_error(E_WARNING, "Cannot bind function %s::%s to scope class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(scope->name));
 				ZVAL_NULL(res);
 				return;
 			}
-			if (func->type == ZEND_INTERNAL_FUNCTION && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF) && !instanceof_function(Z_OBJCE_P(this_ptr), func->common.scope)) {
-				/* rebinding to internal class is not allowed */
-				zend_error(E_WARNING, "Cannot bind closure of internal method %s::%s to unrelated object of class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(Z_OBJCE_P(this_ptr)->name));
+			if (scope && this_ptr && (func->common.fn_flags & ZEND_ACC_STATIC) == 0 && !instanceof_function(Z_OBJCE_P(this_ptr), func->common.scope)) {
+				zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(Z_OBJCE_P(this_ptr)->name));
 				ZVAL_NULL(res);
 				return;
 			}
 		} else if (scope) {
-			zend_error(E_WARNING, "Cannot bind function %s to a class scope", ZSTR_VAL(func->common.function_name));
+			zend_error(E_WARNING, "Cannot bind function %s to an object or class", ZSTR_VAL(func->common.function_name));
 			ZVAL_NULL(res);
 			return;
 		}
 	}
 
-	if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) {
-		/* use dummy scope if we're binding an object without specifying a scope */
-		/* maybe it would be better to create one for this purpose */
-		scope = zend_ce_closure;
-	}
-
 	object_init_ex(res, zend_ce_closure);
 
 	closure = (zend_closure *) Z_OBJ_P(res);
diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index 2030e0265a..965b08eb38 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -851,9 +851,6 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) /
 		if (EXPECTED((func->op_array.fn_flags & ZEND_ACC_GENERATOR) == 0)) {
 			zend_init_execute_data(call, &func->op_array, fci->retval);
 			zend_execute_ex(call);
-			if (UNEXPECTED(ZEND_CALL_INFO(call) & ZEND_CALL_CLOSURE)) {
-        			OBJ_RELEASE((zend_object*)func->op_array.prototype);
-			}
 		} else {
 			zend_generator_create_zval(call, &func->op_array, fci->retval);
 		}
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index a4b082b7b5..fd0e35458f 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -2418,6 +2418,9 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY)
 			}
 			zend_vm_stack_free_extra_args_ex(call_info, execute_data);
 			EG(current_execute_data) = EX(prev_execute_data);
+			if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) {
+				OBJ_RELEASE((zend_object*)EX(func)->op_array.prototype);
+			}
 		} else /* if (call_kind == ZEND_CALL_TOP_CODE) */ {
 			zend_array *symbol_table = EX(symbol_table);
 
@@ -3858,9 +3861,6 @@ ZEND_VM_C_LABEL(fcall_end_change_scope):
 		}
 		OBJ_RELEASE(object);
 	}
-	if (UNEXPECTED((ZEND_CALL_INFO(call) & ZEND_CALL_CLOSURE) && (fbc->common.fn_flags & ZEND_ACC_GENERATOR) == 0)) {
-		OBJ_RELEASE((zend_object*)fbc->op_array.prototype);
-	}
 	EG(scope) = EX(func)->op_array.scope;
 
 ZEND_VM_C_LABEL(fcall_end):
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index e2a4fa94b0..b8da79921d 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -534,6 +534,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_
 			}
 			zend_vm_stack_free_extra_args_ex(call_info, execute_data);
 			EG(current_execute_data) = EX(prev_execute_data);
+			if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) {
+				OBJ_RELEASE((zend_object*)EX(func)->op_array.prototype);
+			}
 		} else /* if (call_kind == ZEND_CALL_TOP_CODE) */ {
 			zend_array *symbol_table = EX(symbol_table);
 
@@ -916,9 +919,6 @@ fcall_end_change_scope:
 		}
 		OBJ_RELEASE(object);
 	}
-	if (UNEXPECTED((ZEND_CALL_INFO(call) & ZEND_CALL_CLOSURE) && (fbc->common.fn_flags & ZEND_ACC_GENERATOR) == 0)) {
-		OBJ_RELEASE((zend_object*)fbc->op_array.prototype);
-	}
 	EG(scope) = EX(func)->op_array.scope;
 
 fcall_end: