From: Nikita Popov Date: Thu, 3 Sep 2020 09:56:55 +0000 (+0200) Subject: Don't intern compiled_filename X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7620ea15807a84e76cb1cb2f9d5234ea787aae2e;p=php Don't intern compiled_filename For php-ast interning the file name is an effective memory leak, see php-ast#134. I don't think there's any reason to do this. At some point this was needed due to bugs in the interned string mechanism that caused issues if the string was later interned, e.g. through a __FILE__ reference. These issues have since been resolved. In conjunction with the filenames_table removal in c4016ecd446ef26bb3dc77735b6e441e151ea985 this means that filenames now need to be refcounted like normal strings. In particular the filename reference in op_arrays and CEs are refcounted. --- diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index a60ceb1c84..21db0a3f7d 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -442,14 +442,17 @@ void shutdown_compiler(void) /* {{{ */ ZEND_API zend_string *zend_set_compiled_filename(zend_string *new_compiled_filename) /* {{{ */ { - new_compiled_filename = zend_new_interned_string(zend_string_copy(new_compiled_filename)); - CG(compiled_filename) = new_compiled_filename; + CG(compiled_filename) = zend_string_copy(new_compiled_filename); return new_compiled_filename; } /* }}} */ ZEND_API void zend_restore_compiled_filename(zend_string *original_compiled_filename) /* {{{ */ { + if (CG(compiled_filename)) { + zend_string_release(CG(compiled_filename)); + CG(compiled_filename) = NULL; + } CG(compiled_filename) = original_compiled_filename; } /* }}} */ @@ -7345,7 +7348,7 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / } ce->ce_flags |= decl->flags; - ce->info.user.filename = zend_get_compiled_filename(); + ce->info.user.filename = zend_string_copy(zend_get_compiled_filename()); ce->info.user.line_start = decl->start_lineno; ce->info.user.line_end = decl->end_lineno; diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l index b00e4e3edb..c6537c664b 100644 --- a/Zend/zend_language_scanner.l +++ b/Zend/zend_language_scanner.l @@ -233,8 +233,9 @@ ZEND_API void zend_save_lexical_state(zend_lex_state *lex_state) lex_state->in = SCNG(yy_in); lex_state->yy_state = YYSTATE; - lex_state->filename = zend_get_compiled_filename(); + lex_state->filename = CG(compiled_filename); lex_state->lineno = CG(zend_lineno); + CG(compiled_filename) = NULL; lex_state->script_org = SCNG(script_org); lex_state->script_org_size = SCNG(script_org_size); diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index a9236faf6a..6681ef7b68 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -61,7 +61,7 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz op_array->T = 0; op_array->function_name = NULL; - op_array->filename = zend_get_compiled_filename(); + op_array->filename = zend_string_copy(zend_get_compiled_filename()); op_array->doc_comment = NULL; op_array->attributes = NULL; @@ -354,6 +354,7 @@ ZEND_API void destroy_zend_class(zval *zv) } efree(ce->interfaces); } + zend_string_release_ex(ce->info.user.filename, 0); if (ce->info.user.doc_comment) { zend_string_release_ex(ce->info.user.doc_comment, 0); } @@ -496,6 +497,7 @@ ZEND_API void destroy_op_array(zend_op_array *op_array) } efree(op_array->opcodes); + zend_string_release_ex(op_array->filename, 0); if (op_array->doc_comment) { zend_string_release_ex(op_array->doc_comment, 0); } diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 725b33d913..8683dc106f 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4528,7 +4528,6 @@ static int accel_preload(const char *config, zend_bool in_child) if (ret == SUCCESS) { zend_persistent_script *script; - zend_string *filename; int ping_auto_globals_mask; int i; zend_class_entry *ce; @@ -4668,7 +4667,7 @@ static int accel_preload(const char *config, zend_bool in_child) script->ping_auto_globals_mask = ping_auto_globals_mask; /* Store all functions and classes in a single pseudo-file */ - filename = zend_string_init("$PRELOAD$", sizeof("$PRELOAD$") - 1, 0); + CG(compiled_filename) = zend_string_init("$PRELOAD$", sizeof("$PRELOAD$") - 1, 0); #if ZEND_USE_ABS_CONST_ADDR init_op_array(&script->script.main_op_array, ZEND_USER_FUNCTION, 1); #else @@ -4690,8 +4689,8 @@ static int accel_preload(const char *config, zend_bool in_child) ZEND_PASS_TWO_UPDATE_CONSTANT(&script->script.main_op_array, script->script.main_op_array.opcodes, script->script.main_op_array.opcodes[0].op1); zend_vm_set_opcode_handler(script->script.main_op_array.opcodes); - script->script.main_op_array.filename = filename; - script->script.filename = zend_string_copy(filename); + script->script.filename = CG(compiled_filename); + CG(compiled_filename) = NULL; script->script.first_early_binding_opline = (uint32_t)-1; @@ -4728,8 +4727,6 @@ static int accel_preload(const char *config, zend_bool in_child) SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); - zend_string_release(filename); - ZEND_ASSERT(ZCSG(preload_script)->arena_size == 0); preload_load(); diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index ed923a03cc..415aa76093 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -565,8 +565,7 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc } if (op_array->filename) { - /* do not free! PHP has centralized filename storage, compiler will free it */ - zend_accel_memdup_string(op_array->filename); + zend_accel_store_string(op_array->filename); } if (op_array->arg_info) { @@ -864,8 +863,7 @@ static void zend_persist_class_entry(zval *zv) HT_FLAGS(&ce->constants_table) &= (HASH_FLAG_UNINITIALIZED | HASH_FLAG_STATIC_KEYS); if (ce->info.user.filename) { - /* do not free! PHP has centralized filename storage, compiler will free it */ - zend_accel_memdup_string(ce->info.user.filename); + zend_accel_store_string(ce->info.user.filename); } if (ce->info.user.doc_comment) { if (ZCG(accel_directives).save_comments) {