Skip to content

Reference dynamic functions through dynamic_defs #5595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Reference dynamic functions through dynamic_defs #5595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6dc79f7
Reference dynamic functions through dynamic_defs
nikic May 18, 2020
b08e977
Properly handle static variables
nikic Feb 22, 2021
a9ff74f
Add additional test for static variables
nikic Feb 24, 2021
01e1f2a
Explicitly destroy closure static_variables
nikic Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Zend/Optimizer/compact_literals.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 11 additions & 3 deletions Zend/Optimizer/zend_optimizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
Expand Down
21 changes: 21 additions & 0 deletions Zend/tests/static_variable_in_dynamic_function.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
21 changes: 21 additions & 0 deletions Zend/tests/static_variable_in_dynamic_function_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions Zend/zend_closures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
52 changes: 22 additions & 30 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 7 additions & 1 deletion Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need "dynamic_func_defs" for each op array? or only for "main"?
May be we may store then in "literals"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need "dynamic_func_defs" for each op array? or only for "main"?

We need it for each op_array, as all op_arrays can contain closures or dynamically defined functions.

May be we may store then in "literals"?

Storing the functions in literals was my first attempt (#5593), but I found the implementation more complicated (though this may be due to unnecessary refcounted wrappers I added).

void *reserved[ZEND_MAX_RESERVED_RESOURCES];
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_execute_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 23 additions & 3 deletions Zend/zend_opcode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*);

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 8 additions & 13 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2824,6 +2824,7 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
ZEND_VM_LEAVE();
} else if (EXPECTED((call_info & ZEND_CALL_TOP) == 0)) {
zend_detach_symbol_table(execute_data);
zend_destroy_static_vars(&EX(func)->op_array);
destroy_op_array(&EX(func)->op_array);
efree_size(EX(func), sizeof(zend_op_array));
#ifdef ZEND_PREFER_RELOAD
Expand Down Expand Up @@ -6231,6 +6232,7 @@ ZEND_VM_HANDLER(73, ZEND_INCLUDE_OR_EVAL, CONST|TMPVAR|CV, ANY, EVAL, SPEC(OBSER
zend_vm_stack_free_call_frame(call);
}

zend_destroy_static_vars(new_op_array);
destroy_op_array(new_op_array);
efree_size(new_op_array, sizeof(zend_op_array));
if (UNEXPECTED(EG(exception) != NULL)) {
Expand Down Expand Up @@ -7583,12 +7585,14 @@ ZEND_VM_HANDLER(146, ZEND_DECLARE_ANON_CLASS, ANY, ANY, CACHE_SLOT)
ZEND_VM_NEXT_OPCODE();
}

ZEND_VM_HANDLER(141, ZEND_DECLARE_FUNCTION, ANY, ANY)
ZEND_VM_HANDLER(141, ZEND_DECLARE_FUNCTION, ANY, NUM)
{
zend_function *func;
USE_OPLINE

SAVE_OPLINE();
do_bind_function(RT_CONSTANT(opline, opline->op1));
func = (zend_function *) EX(func)->op_array.dynamic_func_defs[opline->op2.num];
do_bind_function(func, RT_CONSTANT(opline, opline->op1));
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

Expand Down Expand Up @@ -7853,23 +7857,14 @@ ZEND_VM_HANDLER(143, ZEND_DECLARE_CONST, CONST, CONST)
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(142, ZEND_DECLARE_LAMBDA_FUNCTION, CONST, UNUSED, CACHE_SLOT)
ZEND_VM_HANDLER(142, ZEND_DECLARE_LAMBDA_FUNCTION, CONST, NUM)
{
USE_OPLINE
zend_function *func;
zval *zfunc;
zval *object;
zend_class_entry *called_scope;

func = CACHED_PTR(opline->extended_value);
if (UNEXPECTED(func == NULL)) {
zfunc = zend_hash_find_ex(EG(function_table), Z_STR_P(RT_CONSTANT(opline, opline->op1)), 1);
ZEND_ASSERT(zfunc != NULL);
func = Z_FUNC_P(zfunc);
ZEND_ASSERT(func->type == ZEND_USER_FUNCTION);
CACHE_PTR(opline->extended_value, func);
}

func = (zend_function *) EX(func)->op_array.dynamic_func_defs[opline->op2.num];
if (Z_TYPE(EX(This)) == IS_OBJECT) {
called_scope = Z_OBJCE(EX(This));
if (UNEXPECTED((func->common.fn_flags & ZEND_ACC_STATIC) ||
Expand Down
Loading