From: Dmitry Stogov Date: Tue, 6 Oct 2015 20:48:15 +0000 (+0300) Subject: Revert "Improve 517b55362 (scope rebinding on method Closures)" X-Git-Tag: php-7.1.0alpha1~1025^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e193d35c1ed1307b06cc25c2b720d0141703798c;p=php Revert "Improve 517b55362 (scope rebinding on method Closures)" This reverts commit 881c50252066132f83e190325e344f532be19033. --- diff --git a/Zend/tests/bug70630.phpt b/Zend/tests/bug70630.phpt index a46c460df0..3dabca66da 100644 --- a/Zend/tests/bug70630.phpt +++ b/Zend/tests/bug70630.phpt @@ -17,8 +17,8 @@ foreach (["substr", "foo"] as $fn) { Warning: Cannot bind function substr to an object in %s on line %d -Warning: Cannot bind function substr to an object or class in %s on line %d +Warning: Cannot bind function substr to an object 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 +Warning: Cannot bind function foo to an object in %s on line %d diff --git a/Zend/tests/closure_061.phpt b/Zend/tests/closure_061.phpt index 342b05ca5d..a195956840 100644 --- a/Zend/tests/closure_061.phpt +++ b/Zend/tests/closure_061.phpt @@ -56,8 +56,9 @@ string(3) "bar" 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 +Fatal error: Uncaught Error: Using $this when not in object context in %s:%d Stack trace: -#0 %s(%d): callMethodOn('foo', 'initClass', Object(baz)) -#1 {main} +#0 %s(%d): initClass() +#1 %s(%d): callMethodOn('foo', 'initClass', Object(baz)) +#2 {main} thrown in %s on line %d diff --git a/Zend/tests/closure_062.phpt b/Zend/tests/closure_062.phpt deleted file mode 100644 index 574b43a3c2..0000000000 --- a/Zend/tests/closure_062.phpt +++ /dev/null @@ -1,39 +0,0 @@ ---TEST-- -Force failure with Closure binding to unknown scopes/$this with Closure::call() ---FILE-- -getClosure(); -$x->call(new stdClass); - -class a { function foo() { echo get_class($this), "\n"; } } -$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); -$x->call(new stdClass); - -class b extends a {} -$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); -$x->call(new b); - -class c extends stdClass { function foo() { echo get_class($this), "\n"; } } -$x = (new ReflectionMethod("c", "foo"))->getClosure(new c); -$x->call(new stdClass); - -$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); -$x->call(new a); - -class d { function foo() { print "Success\n"; yield; } } -$x = (new ReflectionMethod("d", "foo"))->getClosure(new d); -$x->call(new d)->current(); - -?> ---EXPECTF-- - -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 - -Warning: Cannot bind function c::foo to object of class stdClass in %s on line %d -a -Success diff --git a/Zend/tests/closure_063.phpt b/Zend/tests/closure_063.phpt deleted file mode 100644 index e854b578e2..0000000000 --- a/Zend/tests/closure_063.phpt +++ /dev/null @@ -1,53 +0,0 @@ ---TEST-- -Force failure with Closure binding to unknown scopes/$this with Closure::bind() ---FILE-- -getClosure(); -$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); - -class c extends stdClass { function foo() { echo get_class($this), "\n"; } } -$x = (new ReflectionMethod("c", "foo"))->getClosure(new c); -$x->bindTo(new stdClass); - -class b extends a {} -$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); -$x->bindTo(new b)(); -$x->bindTo(new b, "a")(); -$x->bindTo(new b, "c"); - -$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); -$x->bindTo(new a)(); - -class d { function foo() { print "Success\n"; yield; } } -class e extends d {} -$x = (new ReflectionMethod("d", "foo"))->getClosure(new d); -$x->bindTo(new d)()->current(); -$x->bindTo(new e)()->current(); -$x->bindTo(new e, "d")()->current(); -$x->bindTo(new e, "e"); - -?> ---EXPECTF-- - -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 -Success -Success -Success - -Warning: Cannot bind function d::foo to scope class e in %s on line %d - diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index c06aac609d..8de7b6e488 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -126,23 +126,17 @@ ZEND_METHOD(Closure, call) if (fci_cache.function_handler->common.fn_flags & ZEND_ACC_GENERATOR) { zval new_closure; - zend_create_closure(&new_closure, fci_cache.function_handler, closure->func.common.scope, closure->called_scope, &closure->this_ptr); + zend_create_closure(&new_closure, fci_cache.function_handler, Z_OBJCE_P(newthis), closure->called_scope, newthis); closure = (zend_closure *) Z_OBJ(new_closure); - closure->func.common.fn_flags |= ZEND_ACC_PUBLIC; - zval_ptr_dtor_nogc(&closure->this_ptr); - ZVAL_COPY(&closure->this_ptr, newthis); fci_cache.function_handler = &closure->func; } else { memcpy(&my_function, fci_cache.function_handler, fci_cache.function_handler->type == ZEND_USER_FUNCTION ? sizeof(zend_op_array) : sizeof(zend_internal_function)); - fci_cache.function_handler = &my_function; - } - - if (closure->func.type == ZEND_USER_FUNCTION && (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE)) { /* use scope of passed object */ - fci_cache.function_handler->common.scope = Z_OBJCE_P(newthis); + my_function.common.scope = Z_OBJCE_P(newthis); + fci_cache.function_handler = &my_function; /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ - if (closure->func.common.scope != Z_OBJCE_P(newthis)) { + if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope != Z_OBJCE_P(newthis)) { my_function.op_array.run_time_cache = emalloc(my_function.op_array.cache_size); memset(my_function.op_array.run_time_cache, 0, my_function.op_array.cache_size); } @@ -155,7 +149,7 @@ ZEND_METHOD(Closure, call) if (fci_cache.function_handler->common.fn_flags & ZEND_ACC_GENERATOR) { /* copied upon generator creation */ --GC_REFCOUNT(&closure->std); - } else if (ZEND_USER_CODE(my_function.type) && (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) && closure->func.common.scope != Z_OBJCE_P(newthis)) { + } else if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope != Z_OBJCE_P(newthis)) { efree(my_function.op_array.run_time_cache); } } @@ -210,18 +204,31 @@ ZEND_METHOD(Closure, bind) called_scope = ce; } - zend_create_closure(return_value, &closure->func, ce, called_scope, newthis); + /* verify that we aren't binding methods to a wrong object */ + if (closure->func.type != ZEND_USER_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) { + if (!closure->func.common.scope) { + if (ce) { + zend_error(E_WARNING, "Cannot bind function %s to an object", ZSTR_VAL(closure->func.common.function_name)); + return; + } + } else if (!ce) { + zend_error(E_WARNING, "Cannot bind function %s::%s to no class", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name)); + return; + } else if (!instanceof_function(ce, closure->func.common.scope)) { + zend_error(E_WARNING, "Cannot bind function %s::%s to class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(ce->name)); + return; + } + } - if (Z_TYPE_P(return_value) != IS_NULL) { - new_closure = (zend_closure *) Z_OBJ_P(return_value); + zend_create_closure(return_value, &closure->func, ce, called_scope, newthis); + new_closure = (zend_closure *) Z_OBJ_P(return_value); - /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ - if (ZEND_USER_CODE(closure->func.type) && (closure->func.common.scope != new_closure->func.common.scope || (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { - new_closure->func.op_array.run_time_cache = emalloc(new_closure->func.op_array.cache_size); - memset(new_closure->func.op_array.run_time_cache, 0, new_closure->func.op_array.cache_size); + /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ + if (ZEND_USER_CODE(closure->func.type) && (closure->func.common.scope != new_closure->func.common.scope || (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { + new_closure->func.op_array.run_time_cache = emalloc(new_closure->func.op_array.cache_size); + memset(new_closure->func.op_array.run_time_cache, 0, new_closure->func.op_array.cache_size); - new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA; - } + new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA; } } /* }}} */ @@ -367,7 +374,8 @@ static zend_object *zend_closure_clone(zval *zobject) /* {{{ */ zend_closure *closure = (zend_closure *)Z_OBJ_P(zobject); zval result; - zend_create_closure(&result, &closure->func, closure->func.common.scope, closure->called_scope, &closure->this_ptr); + zend_create_closure(&result, &closure->func, + closure->func.common.scope, closure->called_scope, &closure->this_ptr); return Z_OBJ(result); } /* }}} */ @@ -525,37 +533,10 @@ void zend_register_closure_ce(void) /* {{{ */ } /* }}} */ -/* res will be set to IS_NULL or contains a Closure object */ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr) /* {{{ */ { 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 internal function to a wrong scope */ - if (func->common.scope != NULL) { - 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 (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 an object or class", ZSTR_VAL(func->common.function_name)); - ZVAL_NULL(res); - return; - } - } - object_init_ex(res, zend_ce_closure); closure = (zend_closure *) Z_OBJ_P(res); @@ -564,6 +545,12 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure->func.common.prototype = (zend_function *) closure; closure->func.common.fn_flags |= ZEND_ACC_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 (closure->func.type == ZEND_USER_FUNCTION) { if (closure->func.op_array.static_variables) { HashTable *static_variables = closure->func.op_array.static_variables; @@ -580,6 +567,24 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent (*closure->func.op_array.refcount)++; } } + if (closure->func.type != ZEND_USER_FUNCTION || (func->common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) { + /* verify that we aren't binding internal function to a wrong scope */ + if(func->common.scope != NULL) { + if(scope && !instanceof_function(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)); + scope = NULL; + } + if(scope && this_ptr && (func->common.fn_flags & ZEND_ACC_STATIC) == 0 && + !instanceof_function(Z_OBJCE_P(this_ptr), closure->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)); + scope = NULL; + } + } else { + /* if it's a free function, we won't set scope & this since they're meaningless */ + this_ptr = NULL; + scope = NULL; + } + } ZVAL_UNDEF(&closure->this_ptr); /* Invariant: