From: Bob Weinand <bobwei9@hotmail.com>
Date: Sun, 21 Jun 2015 14:39:28 +0000 (+0200)
Subject: Fix bad run_time_cache with Closure::call()
X-Git-Tag: php-7.0.0alpha2~2^2~36
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3c288b12b45763b3215ee0463e51f8be9d643425;p=php

Fix bad run_time_cache with Closure::call()
This also fixes a memory "leak" (memory is allocated on unbounded arena without limits) on each new Closure instantiation.
Closures with same scope now all share the same run_time_cache (as long as it is arena allocated)
---

diff --git a/Zend/tests/closure_060.phpt b/Zend/tests/closure_060.phpt
new file mode 100644
index 0000000000..f03160dab9
--- /dev/null
+++ b/Zend/tests/closure_060.phpt
@@ -0,0 +1,25 @@
+--TEST--
+runtime cache must be invalidated for Closure::call()
+--FILE--
+<?php
+
+class A {
+	private static $priv = 7;
+
+	static function get() {
+		return function() {
+			var_dump(isset(A::$priv));
+		};
+	}
+}
+
+$closure = A::get();
+$closure(); // init rt_cache
+$closure->call(new class(){}, null);
+$closure();
+
+?>
+--EXPECT--
+bool(true)
+bool(false)
+bool(true)
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index b28810a236..4e642ffb1c 100644
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -127,9 +127,19 @@ ZEND_METHOD(Closure, call)
 	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 (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);
+	}
+
 	if (zend_call_function(&fci, &fci_cache) == SUCCESS && Z_TYPE(closure_result) != IS_UNDEF) {
 		ZVAL_COPY_VALUE(return_value, &closure_result);
 	}
+
+	if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope != Z_OBJCE_P(newthis)) {
+		efree(my_function.op_array.run_time_cache);
+	}
 }
 /* }}} */
 
@@ -138,7 +148,7 @@ ZEND_METHOD(Closure, call)
 ZEND_METHOD(Closure, bind)
 {
 	zval *newthis, *zclosure, *scope_arg = NULL;
-	zend_closure *closure;
+	zend_closure *closure, *new_closure;
 	zend_class_entry *ce, *called_scope;
 
 	if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Oo!|z", &zclosure, zend_ce_closure, &newthis, &scope_arg) == FAILURE) {
@@ -183,6 +193,15 @@ ZEND_METHOD(Closure, bind)
 	}
 
 	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);
+
+		new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA;
+	}
 }
 /* }}} */
 
@@ -297,6 +316,10 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
 	zend_object_std_dtor(&closure->std);
 
 	if (closure->func.type == ZEND_USER_FUNCTION) {
+		if (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA) {
+			efree(closure->func.op_array.run_time_cache);
+			closure->func.op_array.run_time_cache = NULL;
+		}
 		destroy_op_array(&closure->func.op_array);
 	}
 
@@ -510,7 +533,10 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
 			zend_hash_init(closure->func.op_array.static_variables, zend_hash_num_elements(static_variables), NULL, ZVAL_PTR_DTOR, 0);
 			zend_hash_apply_with_arguments(static_variables, zval_copy_static_var, 1, closure->func.op_array.static_variables);
 		}
-		closure->func.op_array.run_time_cache = NULL;
+		if (UNEXPECTED(!closure->func.op_array.run_time_cache)) {
+			closure->func.op_array.run_time_cache = func->op_array.run_time_cache = zend_arena_alloc(&CG(arena), func->op_array.cache_size);
+			memset(func->op_array.run_time_cache, 0, func->op_array.cache_size);
+		}
 		if (closure->func.op_array.refcount) {
 			(*closure->func.op_array.refcount)++;
 		}
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
index 106d0fc475..f1f7195494 100644
--- a/Zend/zend_compile.h
+++ b/Zend/zend_compile.h
@@ -240,6 +240,8 @@ typedef struct _zend_try_catch_element {
 #define ZEND_ACC_CLOSURE              0x100000
 #define ZEND_ACC_GENERATOR            0x800000
 
+#define ZEND_ACC_NO_RT_ARENA          0x10000
+
 /* call through user function trampoline. e.g. __call, __callstatic */
 #define ZEND_ACC_CALL_VIA_TRAMPOLINE  0x200000