Skip to content

Commit 47a2e5c

Browse files
nikicnikic
committed
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 GH-5595.
1 parent b86dfb0 commit 47a2e5c

19 files changed

+245
-131
lines changed

‎Zend/Optimizer/compact_literals.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,6 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
241241
case ZEND_RECV_INIT:
242242
LITERAL_INFO(opline->op2.constant, LITERAL_VALUE, 1);
243243
break;
244-
case ZEND_DECLARE_FUNCTION:
245-
LITERAL_INFO(opline->op1.constant, LITERAL_VALUE, 2);
246-
break;
247244
case ZEND_DECLARE_CLASS:
248245
case ZEND_DECLARE_CLASS_DELAYED:
249246
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
776773
bind_var_slot[opline->op2.constant] = opline->extended_value;
777774
}
778775
break;
779-
case ZEND_DECLARE_LAMBDA_FUNCTION:
780776
case ZEND_DECLARE_ANON_CLASS:
781777
case ZEND_DECLARE_CLASS_DELAYED:
782778
opline->extended_value = cache_size;

‎Zend/Optimizer/zend_optimizer.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,16 +1367,24 @@ static bool needs_live_range(zend_op_array *op_array, zend_op *def_opline) {
13671367
return (type & (MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE|MAY_BE_REF)) != 0;
13681368
}
13691369

1370+
static void zend_foreach_op_array_helper(
1371+
zend_op_array *op_array, zend_op_array_func_t func, void *context) {
1372+
func(op_array, context);
1373+
for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
1374+
func(op_array->dynamic_func_defs[i], context);
1375+
}
1376+
}
1377+
13701378
void zend_foreach_op_array(zend_script *script, zend_op_array_func_t func, void *context)
13711379
{
13721380
zend_class_entry *ce;
13731381
zend_string *key;
13741382
zend_op_array *op_array;
13751383

1376-
func(&script->main_op_array, context);
1384+
zend_foreach_op_array_helper(&script->main_op_array, func, context);
13771385

13781386
ZEND_HASH_FOREACH_PTR(&script->function_table, op_array) {
1379-
func(op_array, context);
1387+
zend_foreach_op_array_helper(op_array, func, context);
13801388
} ZEND_HASH_FOREACH_END();
13811389

13821390
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
13871395
if (op_array->scope == ce
13881396
&& op_array->type == ZEND_USER_FUNCTION
13891397
&& !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) {
1390-
func(op_array, context);
1398+
zend_foreach_op_array_helper(op_array, func, context);
13911399
}
13921400
} ZEND_HASH_FOREACH_END();
13931401
} ZEND_HASH_FOREACH_END();

‎Zend/tests/static_variable_in_dynamic_function.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Static variables in dynamically declared function (first use before dynamic def dtor)
3+
--FILE--
4+
<?php
5+
6+
$code = <<<'CODE'
7+
if (1) {
8+
function test() {
9+
static $x = 0;
10+
var_dump(++$x);
11+
}
12+
test();
13+
}
14+
CODE;
15+
eval($code);
16+
test();
17+
18+
?>
19+
--EXPECT--
20+
int(1)
21+
int(2)

‎Zend/tests/static_variable_in_dynamic_function_2.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Static variables in dynamically declared function (first use after dynamic def dtor)
3+
--FILE--
4+
<?php
5+
6+
$code = <<<'CODE'
7+
if (1) {
8+
function test() {
9+
static $x = 0;
10+
var_dump(++$x);
11+
}
12+
}
13+
CODE;
14+
eval($code);
15+
test();
16+
test();
17+
18+
?>
19+
--EXPECT--
20+
int(1)
21+
int(2)

‎Zend/zend.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,6 +1696,7 @@ ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count
16961696
ret = zend_exception_error(EG(exception), E_ERROR);
16971697
}
16981698
}
1699+
zend_destroy_static_vars(op_array);
16991700
destroy_op_array(op_array);
17001701
efree_size(op_array, sizeof(zend_op_array));
17011702
} else if (type==ZEND_REQUIRE) {

‎Zend/zend_closures.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -486,10 +486,9 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
486486
zend_object_std_dtor(&closure->std);
487487

488488
if (closure->func.type == ZEND_USER_FUNCTION) {
489-
/* We shared static_variables with the original function.
490-
* Unshare now so we don't try to destroy them. */
491-
if (closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE) {
492-
ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr, NULL);
489+
/* We don't own the static variables of fake closures. */
490+
if (!(closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
491+
zend_destroy_static_vars(&closure->func.op_array);
493492
}
494493
destroy_op_array(&closure->func.op_array);
495494
}

‎Zend/zend_compile.c

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,27 +1091,19 @@ static zend_never_inline ZEND_COLD ZEND_NORETURN void do_bind_function_error(zen
10911091
}
10921092
}
10931093

1094-
ZEND_API zend_result do_bind_function(zval *lcname) /* {{{ */
1094+
ZEND_API zend_result do_bind_function(zend_function *func, zval *lcname) /* {{{ */
10951095
{
1096-
zend_function *function;
1097-
zval *rtd_key, *zv;
1098-
1099-
rtd_key = lcname + 1;
1100-
zv = zend_hash_find_ex(EG(function_table), Z_STR_P(rtd_key), 1);
1101-
if (UNEXPECTED(!zv)) {
1102-
do_bind_function_error(Z_STR_P(lcname), NULL, 0);
1096+
zend_function *added_func = zend_hash_add_ptr(EG(function_table), Z_STR_P(lcname), func);
1097+
if (UNEXPECTED(!added_func)) {
1098+
do_bind_function_error(Z_STR_P(lcname), &func->op_array, 0);
11031099
return FAILURE;
11041100
}
1105-
function = (zend_function*)Z_PTR_P(zv);
1106-
if (UNEXPECTED(function->common.fn_flags & ZEND_ACC_PRELOADED)
1107-
&& !(CG(compiler_options) & ZEND_COMPILE_PRELOAD)) {
1108-
zv = zend_hash_add(EG(function_table), Z_STR_P(lcname), zv);
1109-
} else {
1110-
zv = zend_hash_set_bucket_key(EG(function_table), (Bucket*)zv, Z_STR_P(lcname));
1101+
1102+
if (func->op_array.refcount) {
1103+
++*func->op_array.refcount;
11111104
}
1112-
if (UNEXPECTED(!zv)) {
1113-
do_bind_function_error(Z_STR_P(lcname), &function->op_array, 0);
1114-
return FAILURE;
1105+
if (func->common.function_name) {
1106+
zend_string_addref(func->common.function_name);
11151107
}
11161108
return SUCCESS;
11171109
}
@@ -6954,9 +6946,18 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name,
69546946
}
69556947
/* }}} */
69566948

6949+
static uint32_t zend_add_dynamic_func_def(zend_op_array *def) {
6950+
zend_op_array *op_array = CG(active_op_array);
6951+
uint32_t def_offset = op_array->num_dynamic_func_defs++;
6952+
op_array->dynamic_func_defs = erealloc(
6953+
op_array->dynamic_func_defs, op_array->num_dynamic_func_defs * sizeof(zend_op_array *));
6954+
op_array->dynamic_func_defs[def_offset] = def;
6955+
return def_offset;
6956+
}
6957+
69576958
static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_ast_decl *decl, bool toplevel) /* {{{ */
69586959
{
6959-
zend_string *unqualified_name, *name, *lcname, *key;
6960+
zend_string *unqualified_name, *name, *lcname;
69606961
zend_op *opline;
69616962

69626963
unqualified_name = decl->name;
@@ -6992,25 +6993,16 @@ static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_as
69926993
return;
69936994
}
69946995

6995-
/* Generate RTD keys until we find one that isn't in use yet. */
6996-
key = NULL;
6997-
do {
6998-
zend_tmp_string_release(key);
6999-
key = zend_build_runtime_definition_key(lcname, decl->start_lineno);
7000-
} while (!zend_hash_add_ptr(CG(function_table), key, op_array));
7001-
6996+
uint32_t func_ref = zend_add_dynamic_func_def(op_array);
70026997
if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
70036998
opline = zend_emit_op_tmp(result, ZEND_DECLARE_LAMBDA_FUNCTION, NULL, NULL);
7004-
opline->extended_value = zend_alloc_cache_slot();
7005-
opline->op1_type = IS_CONST;
7006-
LITERAL_STR(opline->op1, key);
6999+
opline->op2.num = func_ref;
70077000
} else {
70087001
opline = get_next_op();
70097002
opline->opcode = ZEND_DECLARE_FUNCTION;
70107003
opline->op1_type = IS_CONST;
70117004
LITERAL_STR(opline->op1, zend_string_copy(lcname));
7012-
/* RTD key is placed after lcname literal in op1 */
7013-
zend_add_literal_string(&key);
7005+
opline->op2.num = func_ref;
70147006
}
70157007
zend_string_release_ex(lcname, 0);
70167008
}

‎Zend/zend_compile.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,13 @@ struct _zend_op_array {
456456
zend_string *doc_comment;
457457

458458
int last_literal;
459+
uint32_t num_dynamic_func_defs;
459460
zval *literals;
460461

462+
/* Functions that are declared dynamically are stored here and
463+
* referenced by index from opcodes. */
464+
zend_op_array **dynamic_func_defs;
465+
461466
void *reserved[ZEND_MAX_RESERVED_RESOURCES];
462467
};
463468

@@ -781,7 +786,7 @@ bool zend_handle_encoding_declaration(zend_ast *ast);
781786
/* parser-driven code generators */
782787
void zend_do_free(znode *op1);
783788

784-
ZEND_API zend_result do_bind_function(zval *lcname);
789+
ZEND_API zend_result do_bind_function(zend_function *func, zval *lcname);
785790
ZEND_API zend_result do_bind_class(zval *lcname, zend_string *lc_parent_name);
786791
ZEND_API uint32_t zend_build_delayed_early_binding_list(const zend_op_array *op_array);
787792
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, ...);
812817
ZEND_API int open_file_for_scanning(zend_file_handle *file_handle);
813818
ZEND_API void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_size);
814819
ZEND_API void destroy_op_array(zend_op_array *op_array);
820+
ZEND_API void zend_destroy_static_vars(zend_op_array *op_array);
815821
ZEND_API void zend_destroy_file_handle(zend_file_handle *file_handle);
816822
ZEND_API void zend_cleanup_mutable_class_data(zend_class_entry *ce);
817823
ZEND_API void zend_cleanup_internal_class_data(zend_class_entry *ce);

‎Zend/zend_execute_API.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,7 @@ ZEND_API zend_result zend_eval_stringl(const char *str, size_t str_len, zval *re
12241224
}
12251225

12261226
EG(no_extensions)=0;
1227+
zend_destroy_static_vars(new_op_array);
12271228
destroy_op_array(new_op_array);
12281229
efree_size(new_op_array, sizeof(zend_op_array));
12291230
retval = SUCCESS;

‎Zend/zend_opcode.c

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
8585
op_array->last_literal = 0;
8686
op_array->literals = NULL;
8787

88+
op_array->num_dynamic_func_defs = 0;
89+
op_array->dynamic_func_defs = NULL;
90+
8891
ZEND_MAP_PTR_INIT(op_array->run_time_cache, NULL);
8992
op_array->cache_size = zend_op_array_extension_handles * sizeof(void*);
9093

@@ -511,16 +514,20 @@ void zend_class_add_ref(zval *zv)
511514
}
512515
}
513516

514-
ZEND_API void destroy_op_array(zend_op_array *op_array)
517+
ZEND_API void zend_destroy_static_vars(zend_op_array *op_array)
515518
{
516-
uint32_t i;
517-
518519
if (ZEND_MAP_PTR(op_array->static_variables_ptr)) {
519520
HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
520521
if (ht) {
521522
zend_array_destroy(ht);
523+
ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
522524
}
523525
}
526+
}
527+
528+
ZEND_API void destroy_op_array(zend_op_array *op_array)
529+
{
530+
uint32_t i;
524531

525532
if ((op_array->fn_flags & ZEND_ACC_HEAP_RT_CACHE)
526533
&& ZEND_MAP_PTR(op_array->run_time_cache)) {
@@ -600,6 +607,19 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
600607
if (op_array->static_variables) {
601608
zend_array_destroy(op_array->static_variables);
602609
}
610+
if (op_array->num_dynamic_func_defs) {
611+
for (i = 0; i < op_array->num_dynamic_func_defs; i++) {
612+
/* Closures overwrite static_variables in their copy.
613+
* Make sure to destroy them when the prototype function is destroyed. */
614+
if (op_array->dynamic_func_defs[i]->static_variables
615+
&& (op_array->dynamic_func_defs[i]->fn_flags & ZEND_ACC_CLOSURE)) {
616+
zend_array_destroy(op_array->dynamic_func_defs[i]->static_variables);
617+
op_array->dynamic_func_defs[i]->static_variables = NULL;
618+
}
619+
destroy_op_array(op_array->dynamic_func_defs[i]);
620+
}
621+
efree(op_array->dynamic_func_defs);
622+
}
603623
}
604624

605625
static void zend_update_extended_stmts(zend_op_array *op_array)

‎Zend/zend_vm_def.h

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2824,6 +2824,7 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
28242824
ZEND_VM_LEAVE();
28252825
} else if (EXPECTED((call_info & ZEND_CALL_TOP) == 0)) {
28262826
zend_detach_symbol_table(execute_data);
2827+
zend_destroy_static_vars(&EX(func)->op_array);
28272828
destroy_op_array(&EX(func)->op_array);
28282829
efree_size(EX(func), sizeof(zend_op_array));
28292830
#ifdef ZEND_PREFER_RELOAD
@@ -6231,6 +6232,7 @@ ZEND_VM_HANDLER(73, ZEND_INCLUDE_OR_EVAL, CONST|TMPVAR|CV, ANY, EVAL, SPEC(OBSER
62316232
zend_vm_stack_free_call_frame(call);
62326233
}
62336234

6235+
zend_destroy_static_vars(new_op_array);
62346236
destroy_op_array(new_op_array);
62356237
efree_size(new_op_array, sizeof(zend_op_array));
62366238
if (UNEXPECTED(EG(exception) != NULL)) {
@@ -7583,12 +7585,14 @@ ZEND_VM_HANDLER(146, ZEND_DECLARE_ANON_CLASS, ANY, ANY, CACHE_SLOT)
75837585
ZEND_VM_NEXT_OPCODE();
75847586
}
75857587

7586-
ZEND_VM_HANDLER(141, ZEND_DECLARE_FUNCTION, ANY, ANY)
7588+
ZEND_VM_HANDLER(141, ZEND_DECLARE_FUNCTION, ANY, NUM)
75877589
{
7590+
zend_function *func;
75887591
USE_OPLINE
75897592

75907593
SAVE_OPLINE();
7591-
do_bind_function(RT_CONSTANT(opline, opline->op1));
7594+
func = (zend_function *) EX(func)->op_array.dynamic_func_defs[opline->op2.num];
7595+
do_bind_function(func, RT_CONSTANT(opline, opline->op1));
75927596
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
75937597
}
75947598

@@ -7853,23 +7857,14 @@ ZEND_VM_HANDLER(143, ZEND_DECLARE_CONST, CONST, CONST)
78537857
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
78547858
}
78557859

7856-
ZEND_VM_HANDLER(142, ZEND_DECLARE_LAMBDA_FUNCTION, CONST, UNUSED, CACHE_SLOT)
7860+
ZEND_VM_HANDLER(142, ZEND_DECLARE_LAMBDA_FUNCTION, CONST, NUM)
78577861
{
78587862
USE_OPLINE
78597863
zend_function *func;
7860-
zval *zfunc;
78617864
zval *object;
78627865
zend_class_entry *called_scope;
78637866

7864-
func = CACHED_PTR(opline->extended_value);
7865-
if (UNEXPECTED(func == NULL)) {
7866-
zfunc = zend_hash_find_ex(EG(function_table), Z_STR_P(RT_CONSTANT(opline, opline->op1)), 1);
7867-
ZEND_ASSERT(zfunc != NULL);
7868-
func = Z_FUNC_P(zfunc);
7869-
ZEND_ASSERT(func->type == ZEND_USER_FUNCTION);
7870-
CACHE_PTR(opline->extended_value, func);
7871-
}
7872-
7867+
func = (zend_function *) EX(func)->op_array.dynamic_func_defs[opline->op2.num];
78737868
if (Z_TYPE(EX(This)) == IS_OBJECT) {
78747869
called_scope = Z_OBJCE(EX(This));
78757870
if (UNEXPECTED((func->common.fn_flags & ZEND_ACC_STATIC) ||

0 commit comments

Comments
 (0)