Reference dynamic functions through dynamic_defs

Currently, dynamically declared functions and closures are inserted
into the function table under a runtime definition key, and then later
possibly renamed. When opcache is not used and a file containing a
closure is repeatedly included, this leads to a very large memory leak,
as the no longer needed closure declarations will never be freed
(https://bugs.php.net/bug.php?id=76982).

With this patch, dynamic functions are instead stored in a
dynamic_func_defs member on the op_array, which opcodes reference
by index. When the parent op_array is destroyed, the dynamic_func_defs
it contains are also destroyed (unless they are stilled used elsewhere,
e.g. because they have been bound, or are used by a live closure). This
resolves the fundamental part of the leak, though doesn't completely
fix it yet due to some arena allocations.

The main non-obvious change here is to static variable handling:
We can't destroy static_variables_ptr in destroy_op_array, as e.g.
that would clear the static variables in a dynamic function when
the op_array containing it is destroyed. Static variable destruction
is separated out for this reason (we already do static variable
destruction separately for normal functions, so we only need to
handle main scripts).

Closes phpGH-5595.

Author: nikic

Zend/Optimizer/compact_literals.c

@@ -241,9 +241,6 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 				case ZEND_RECV_INIT:
 					LITERAL_INFO(opline->op2.constant, LITERAL_VALUE, 1);
 					break;
-				case ZEND_DECLARE_FUNCTION:
-					LITERAL_INFO(opline->op1.constant, LITERAL_VALUE, 2);
-					break;
 				case ZEND_DECLARE_CLASS:
 				case ZEND_DECLARE_CLASS_DELAYED:
 					LITERAL_INFO(opline->op1.constant, LITERAL_VALUE, 2);
@@ -776,7 +773,6 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 						bind_var_slot[opline->op2.constant] = opline->extended_value;
 					}
 					break;
-				case ZEND_DECLARE_LAMBDA_FUNCTION:
 				case ZEND_DECLARE_ANON_CLASS:
 				case ZEND_DECLARE_CLASS_DELAYED:
 					opline->extended_value = cache_size;

Zend/Optimizer/zend_optimizer.c

@@ -1367,16 +1367,24 @@ static bool needs_live_range(zend_op_array *op_array, zend_op *def_opline) {
 	return (type & (MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE|MAY_BE_REF)) != 0;
 }
 
+static void zend_foreach_op_array_helper(
+		zend_op_array *op_array, zend_op_array_func_t func, void *context) {
+	func(op_array, context);
+	for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
+		func(op_array->dynamic_func_defs[i], context);
+	}
+}
+
 void zend_foreach_op_array(zend_script *script, zend_op_array_func_t func, void *context)
 {
 	zend_class_entry *ce;
 	zend_string *key;
 	zend_op_array *op_array;
 
-	func(&script->main_op_array, context);
+	zend_foreach_op_array_helper(&script->main_op_array, func, context);
 
 	ZEND_HASH_FOREACH_PTR(&script->function_table, op_array) {
-		func(op_array, context);
+		zend_foreach_op_array_helper(op_array, func, context);
 	} ZEND_HASH_FOREACH_END();
 
 	ZEND_HASH_FOREACH_STR_KEY_PTR(&script->class_table, key, ce) {
@@ -1387,7 +1395,7 @@ void zend_foreach_op_array(zend_script *script, zend_op_array_func_t func, void
 			if (op_array->scope == ce
 					&& op_array->type == ZEND_USER_FUNCTION
 					&& !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) {
-				func(op_array, context);
+				zend_foreach_op_array_helper(op_array, func, context);
 			}
 		} ZEND_HASH_FOREACH_END();
 	} ZEND_HASH_FOREACH_END();

Zend/tests/static_variable_in_dynamic_function.phpt

@@ -0,0 +1,21 @@
+--TEST--
+Static variables in dynamically declared function (first use before dynamic def dtor)
+--FILE--
+<?php
+
+$code = <<<'CODE'
+if (1) {
+    function test() {
+        static $x = 0;
+        var_dump(++$x);
+    }
+    test();
+}
+CODE;
+eval($code);
+test();
+
+?>
+--EXPECT--
+int(1)
+int(2)

Zend/tests/static_variable_in_dynamic_function_2.phpt

@@ -0,0 +1,21 @@
+--TEST--
+Static variables in dynamically declared function (first use after dynamic def dtor)
+--FILE--
+<?php
+
+$code = <<<'CODE'
+if (1) {
+    function test() {
+        static $x = 0;
+        var_dump(++$x);
+    }
+}
+CODE;
+eval($code);
+test();
+test();
+
+?>
+--EXPECT--
+int(1)
+int(2)

Zend/zend.c

@@ -1696,6 +1696,7 @@ ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count
 					ret = zend_exception_error(EG(exception), E_ERROR);
 				}
 			}
+			zend_destroy_static_vars(op_array);
 			destroy_op_array(op_array);
 			efree_size(op_array, sizeof(zend_op_array));
 		} else if (type==ZEND_REQUIRE) {

Zend/zend_closures.c

@@ -486,10 +486,9 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
 	zend_object_std_dtor(&closure->std);
 
 	if (closure->func.type == ZEND_USER_FUNCTION) {
-		/* We shared static_variables with the original function.
-		 * Unshare now so we don't try to destroy them. */
-		if (closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE) {
-			ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr, NULL);
+		/* We don't own the static variables of fake closures. */
+		if (!(closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
+			zend_destroy_static_vars(&closure->func.op_array);
 		}
 		destroy_op_array(&closure->func.op_array);
 	}

Zend/zend_compile.c

@@ -1091,27 +1091,19 @@ static zend_never_inline ZEND_COLD ZEND_NORETURN void do_bind_function_error(zen
 	}
 }
 
-ZEND_API zend_result do_bind_function(zval *lcname) /* {{{ */
+ZEND_API zend_result do_bind_function(zend_function *func, zval *lcname) /* {{{ */
 {
-	zend_function *function;
-	zval *rtd_key, *zv;
-
-	rtd_key = lcname + 1;
-	zv = zend_hash_find_ex(EG(function_table), Z_STR_P(rtd_key), 1);
-	if (UNEXPECTED(!zv)) {
-		do_bind_function_error(Z_STR_P(lcname), NULL, 0);
+	zend_function *added_func = zend_hash_add_ptr(EG(function_table), Z_STR_P(lcname), func);
+	if (UNEXPECTED(!added_func)) {
+		do_bind_function_error(Z_STR_P(lcname), &func->op_array, 0);
 		return FAILURE;
 	}
-	function = (zend_function*)Z_PTR_P(zv);
-	if (UNEXPECTED(function->common.fn_flags & ZEND_ACC_PRELOADED)
-			&& !(CG(compiler_options) & ZEND_COMPILE_PRELOAD)) {
-		zv = zend_hash_add(EG(function_table), Z_STR_P(lcname), zv);
-	} else {
-		zv = zend_hash_set_bucket_key(EG(function_table), (Bucket*)zv, Z_STR_P(lcname));
+
+	if (func->op_array.refcount) {
+		++*func->op_array.refcount;
 	}
-	if (UNEXPECTED(!zv)) {
-		do_bind_function_error(Z_STR_P(lcname), &function->op_array, 0);
-		return FAILURE;
+	if (func->common.function_name) {
+		zend_string_addref(func->common.function_name);
 	}
 	return SUCCESS;
 }
@@ -6954,9 +6946,18 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name,
 }
 /* }}} */
 
+static uint32_t zend_add_dynamic_func_def(zend_op_array *def) {
+	zend_op_array *op_array = CG(active_op_array);
+	uint32_t def_offset = op_array->num_dynamic_func_defs++;
+	op_array->dynamic_func_defs = erealloc(
+		op_array->dynamic_func_defs, op_array->num_dynamic_func_defs * sizeof(zend_op_array *));
+	op_array->dynamic_func_defs[def_offset] = def;
+	return def_offset;
+}
+
 static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_ast_decl *decl, bool toplevel) /* {{{ */
 {
-	zend_string *unqualified_name, *name, *lcname, *key;
+	zend_string *unqualified_name, *name, *lcname;
 	zend_op *opline;
 
 	unqualified_name = decl->name;
@@ -6992,25 +6993,16 @@ static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_as
 		return;
 	}
 
-	/* Generate RTD keys until we find one that isn't in use yet. */
-	key = NULL;
-	do {
-		zend_tmp_string_release(key);
-		key = zend_build_runtime_definition_key(lcname, decl->start_lineno);
-	} while (!zend_hash_add_ptr(CG(function_table), key, op_array));
-
+	uint32_t func_ref = zend_add_dynamic_func_def(op_array);
 	if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
 		opline = zend_emit_op_tmp(result, ZEND_DECLARE_LAMBDA_FUNCTION, NULL, NULL);
-		opline->extended_value = zend_alloc_cache_slot();
-		opline->op1_type = IS_CONST;
-		LITERAL_STR(opline->op1, key);
+		opline->op2.num = func_ref;
 	} else {
 		opline = get_next_op();
 		opline->opcode = ZEND_DECLARE_FUNCTION;
 		opline->op1_type = IS_CONST;
 		LITERAL_STR(opline->op1, zend_string_copy(lcname));
-		/* RTD key is placed after lcname literal in op1 */
-		zend_add_literal_string(&key);
+		opline->op2.num = func_ref;
 	}
 	zend_string_release_ex(lcname, 0);
 }

Zend/zend_compile.h

@@ -456,8 +456,13 @@ struct _zend_op_array {
 	zend_string *doc_comment;
 
 	int last_literal;
+	uint32_t num_dynamic_func_defs;
 	zval *literals;
 
+	/* Functions that are declared dynamically are stored here and
+	 * referenced by index from opcodes. */
+	zend_op_array **dynamic_func_defs;
+
 	void *reserved[ZEND_MAX_RESERVED_RESOURCES];
 };
 
@@ -781,7 +786,7 @@ bool zend_handle_encoding_declaration(zend_ast *ast);
 /* parser-driven code generators */
 void zend_do_free(znode *op1);
 
-ZEND_API zend_result do_bind_function(zval *lcname);
+ZEND_API zend_result do_bind_function(zend_function *func, zval *lcname);
 ZEND_API zend_result do_bind_class(zval *lcname, zend_string *lc_parent_name);
 ZEND_API uint32_t zend_build_delayed_early_binding_list(const zend_op_array *op_array);
 ZEND_API void zend_do_delayed_early_binding(zend_op_array *op_array, uint32_t first_early_binding_opline);
@@ -812,6 +817,7 @@ ZEND_API int zend_execute_scripts(int type, zval *retval, int file_count, ...);
 ZEND_API int open_file_for_scanning(zend_file_handle *file_handle);
 ZEND_API void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_size);
 ZEND_API void destroy_op_array(zend_op_array *op_array);
+ZEND_API void zend_destroy_static_vars(zend_op_array *op_array);
 ZEND_API void zend_destroy_file_handle(zend_file_handle *file_handle);
 ZEND_API void zend_cleanup_mutable_class_data(zend_class_entry *ce);
 ZEND_API void zend_cleanup_internal_class_data(zend_class_entry *ce);

Zend/zend_execute_API.c

@@ -1224,6 +1224,7 @@ ZEND_API zend_result zend_eval_stringl(const char *str, size_t str_len, zval *re
 		}
 
 		EG(no_extensions)=0;
+		zend_destroy_static_vars(new_op_array);
 		destroy_op_array(new_op_array);
 		efree_size(new_op_array, sizeof(zend_op_array));
 		retval = SUCCESS;

Zend/zend_opcode.c

@@ -85,6 +85,9 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
 	op_array->last_literal = 0;
 	op_array->literals = NULL;
 
+	op_array->num_dynamic_func_defs = 0;
+	op_array->dynamic_func_defs = NULL;
+
 	ZEND_MAP_PTR_INIT(op_array->run_time_cache, NULL);
 	op_array->cache_size = zend_op_array_extension_handles * sizeof(void*);
 
@@ -511,16 +514,20 @@ void zend_class_add_ref(zval *zv)
 	}
 }
 
-ZEND_API void destroy_op_array(zend_op_array *op_array)
+ZEND_API void zend_destroy_static_vars(zend_op_array *op_array)
 {
-	uint32_t i;
-
 	if (ZEND_MAP_PTR(op_array->static_variables_ptr)) {
 		HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
 		if (ht) {
 			zend_array_destroy(ht);
+			ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
 		}
 	}
+}
+
+ZEND_API void destroy_op_array(zend_op_array *op_array)
+{
+	uint32_t i;
 
 	if ((op_array->fn_flags & ZEND_ACC_HEAP_RT_CACHE)
 	 && ZEND_MAP_PTR(op_array->run_time_cache)) {
@@ -600,6 +607,19 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
 	if (op_array->static_variables) {
 		zend_array_destroy(op_array->static_variables);
 	}
+	if (op_array->num_dynamic_func_defs) {
+		for (i = 0; i < op_array->num_dynamic_func_defs; i++) {
+			/* Closures overwrite static_variables in their copy.
+			 * Make sure to destroy them when the prototype function is destroyed. */
+			if (op_array->dynamic_func_defs[i]->static_variables
+					&& (op_array->dynamic_func_defs[i]->fn_flags & ZEND_ACC_CLOSURE)) {
+				zend_array_destroy(op_array->dynamic_func_defs[i]->static_variables);
+				op_array->dynamic_func_defs[i]->static_variables = NULL;
+			}
+			destroy_op_array(op_array->dynamic_func_defs[i]);
+		}
+		efree(op_array->dynamic_func_defs);
+	}
 }
 
 static void zend_update_extended_stmts(zend_op_array *op_array)