From ba99aa133c907b9d46e0f18e1461a3a89c17d4f1 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 14 Nov 2018 16:32:07 +0300 Subject: [PATCH] Fixed issues related to optimization and persitence of classes linked with interfaces, traits or internal classes. --- Zend/zend_compile.h | 5 +- Zend/zend_inheritance.c | 1 + ext/opcache/Optimizer/optimize_func_calls.c | 2 + ext/opcache/Optimizer/zend_call_graph.c | 8 ++- ext/opcache/Optimizer/zend_optimizer.c | 24 +++++-- ext/opcache/zend_persist.c | 77 +++++++++++++++++---- 6 files changed, 93 insertions(+), 24 deletions(-) diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 2912316ccc4..b1ab5b3273b 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -260,7 +260,7 @@ typedef struct _zend_oparray_context { /* User class has methods with static variables | | | */ #define ZEND_HAS_STATIC_IN_METHODS (1 << 15) /* X | | | */ /* | | | */ -/* Function Flags (unused: 26...30) | | | */ +/* Function Flags (unused: 27...30) | | | */ /* ============== | | | */ /* | | | */ /* deprecation flag | | | */ @@ -310,6 +310,9 @@ typedef struct _zend_oparray_context { /* internal function is allocated at arena (int only) | | | */ #define ZEND_ACC_ARENA_ALLOCATED (1 << 25) /* | X | | */ /* | | | */ +/* op_array is a clone of trait method | | | */ +#define ZEND_ACC_TRAIT_CLONE (1 << 26) /* | X | | */ +/* | | | */ /* op_array uses strict mode types | | | */ #define ZEND_ACC_STRICT_TYPES (1 << 31) /* | X | | */ diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 00ae866054d..0ba108d18d6 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1308,6 +1308,7 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s } else { new_fn = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); memcpy(new_fn, fn, sizeof(zend_op_array)); + new_fn->op_array.fn_flags |= ZEND_ACC_TRAIT_CLONE; new_fn->op_array.fn_flags &= ~ZEND_ACC_IMMUTABLE; } function_add_ref(new_fn); diff --git a/ext/opcache/Optimizer/optimize_func_calls.c b/ext/opcache/Optimizer/optimize_func_calls.c index 1eae2ca22a0..dc3494a2bab 100644 --- a/ext/opcache/Optimizer/optimize_func_calls.c +++ b/ext/opcache/Optimizer/optimize_func_calls.c @@ -95,6 +95,8 @@ static void zend_try_inline_call(zend_op_array *op_array, zend_op *fcall, zend_o { if (func->type == ZEND_USER_FUNCTION && !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS)) + /* TODO: function copied from trait may be inconsistent ??? */ + && !(func->op_array.fn_flags & (ZEND_ACC_TRAIT_CLONE)) && fcall->extended_value >= func->op_array.required_num_args && func->op_array.opcodes[func->op_array.num_args].opcode == ZEND_RETURN) { diff --git a/ext/opcache/Optimizer/zend_call_graph.c b/ext/opcache/Optimizer/zend_call_graph.c index 4dde2982804..a20180af379 100644 --- a/ext/opcache/Optimizer/zend_call_graph.c +++ b/ext/opcache/Optimizer/zend_call_graph.c @@ -53,6 +53,7 @@ static int zend_op_array_collect(zend_call_graph *call_graph, zend_op_array *op_ static int zend_foreach_op_array(zend_call_graph *call_graph, zend_script *script, zend_op_array_func_t func) { zend_class_entry *ce; + zend_string *key; zend_op_array *op_array; if (func(call_graph, &script->main_op_array) != SUCCESS) { @@ -65,9 +66,12 @@ static int zend_foreach_op_array(zend_call_graph *call_graph, zend_script *scrip } } ZEND_HASH_FOREACH_END(); - ZEND_HASH_FOREACH_PTR(&script->class_table, ce) { + ZEND_HASH_FOREACH_STR_KEY_PTR(&script->class_table, key, ce) { + if (ce->refcount > 1 && !zend_string_equals_ci(key, ce->name)) { + continue; + } ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) { - if (op_array->scope == ce) { + if (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) { if (func(call_graph, op_array) != SUCCESS) { return FAILURE; } diff --git a/ext/opcache/Optimizer/zend_optimizer.c b/ext/opcache/Optimizer/zend_optimizer.c index 35843d19709..5946d1ec8e4 100644 --- a/ext/opcache/Optimizer/zend_optimizer.c +++ b/ext/opcache/Optimizer/zend_optimizer.c @@ -1421,6 +1421,7 @@ static void zend_adjust_fcall_stack_size_graph(zend_op_array *op_array) int zend_optimize_script(zend_script *script, zend_long optimization_level, zend_long debug_level) { zend_class_entry *ce; + zend_string *key; zend_op_array *op_array; zend_string *name; zend_optimizer_ctx ctx; @@ -1440,9 +1441,12 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend zend_optimize_op_array(op_array, &ctx); } ZEND_HASH_FOREACH_END(); - ZEND_HASH_FOREACH_PTR(&script->class_table, ce) { + ZEND_HASH_FOREACH_STR_KEY_PTR(&script->class_table, key, ce) { + if (ce->refcount > 1 && !zend_string_equals_ci(key, ce->name)) { + continue; + } ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, name, op_array) { - if (op_array->scope == ce) { + if (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) { zend_optimize_op_array(op_array, &ctx); } } ZEND_HASH_FOREACH_END(); @@ -1544,16 +1548,22 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend zend_adjust_fcall_stack_size(op_array, &ctx); } ZEND_HASH_FOREACH_END(); - ZEND_HASH_FOREACH_PTR(&script->class_table, ce) { + ZEND_HASH_FOREACH_STR_KEY_PTR(&script->class_table, key, ce) { + if (ce->refcount > 1 && !zend_string_equals_ci(key, ce->name)) { + continue; + } ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, name, op_array) { - if (op_array->scope == ce) { + if (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) { zend_adjust_fcall_stack_size(op_array, &ctx); } } ZEND_HASH_FOREACH_END(); } ZEND_HASH_FOREACH_END(); } - ZEND_HASH_FOREACH_PTR(&script->class_table, ce) { + ZEND_HASH_FOREACH_STR_KEY_PTR(&script->class_table, key, ce) { + if (ce->refcount > 1 && !zend_string_equals_ci(key, ce->name)) { + continue; + } ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, name, op_array) { if (op_array->scope != ce && op_array->type == ZEND_USER_FUNCTION) { zend_op_array *orig_op_array = @@ -1561,10 +1571,12 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend ZEND_ASSERT(orig_op_array != NULL); if (orig_op_array != op_array) { + uint32_t fn_flags = op_array->fn_flags; zend_function *prototype = op_array->prototype; HashTable *ht = op_array->static_variables; *op_array = *orig_op_array; + op_array->fn_flags = fn_flags; op_array->prototype = prototype; op_array->static_variables = ht; } @@ -1582,7 +1594,7 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend ZEND_HASH_FOREACH_PTR(&script->class_table, ce) { ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, name, op_array) { - if (op_array->scope == ce) { + if (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) { zend_dump_op_array(op_array, ZEND_DUMP_RT_CONSTANTS, "after optimizer", NULL); } } ZEND_HASH_FOREACH_END(); diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index f28e7d669fc..9ab4cd6d33b 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -719,6 +719,7 @@ static void zend_persist_class_method(zval *zv) static void zend_persist_property_info(zval *zv) { zend_property_info *prop = zend_shared_alloc_get_xlat_entry(Z_PTR_P(zv)); + zend_class_entry *ce; if (prop) { Z_PTR_P(zv) = prop; @@ -729,7 +730,10 @@ static void zend_persist_property_info(zval *zv) } else { prop = Z_PTR_P(zv) = zend_shared_memdup_arena_put(Z_PTR_P(zv), sizeof(zend_property_info)); } - prop->ce = zend_shared_alloc_get_xlat_entry(prop->ce); + ce = zend_shared_alloc_get_xlat_entry(prop->ce); + if (ce) { + prop->ce = ce; + } zend_accel_store_interned_string(prop->name); if (prop->doc_comment) { if (ZCG(accel_directives).save_comments) { @@ -747,6 +751,7 @@ static void zend_persist_property_info(zval *zv) static void zend_persist_class_constant(zval *zv) { zend_class_constant *c = zend_shared_alloc_get_xlat_entry(Z_PTR_P(zv)); + zend_class_entry *ce; if (c) { Z_PTR_P(zv) = c; @@ -758,7 +763,10 @@ static void zend_persist_class_constant(zval *zv) c = Z_PTR_P(zv) = zend_shared_memdup_arena_put(Z_PTR_P(zv), sizeof(zend_class_constant)); } zend_persist_zval(&c->value); - c->ce = zend_shared_alloc_get_xlat_entry(c->ce); + ce = zend_shared_alloc_get_xlat_entry(c->ce); + if (ce) { + c->ce = ce; + } if (c->doc_comment) { if (ZCG(accel_directives).save_comments) { zend_string *doc_comment = zend_shared_alloc_get_xlat_entry(c->doc_comment); @@ -982,43 +990,82 @@ static int zend_update_parent_ce(zval *zv) /* update methods */ if (ce->constructor) { - ce->constructor = zend_shared_alloc_get_xlat_entry(ce->constructor); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->constructor); + if (tmp != NULL) { + ce->constructor = tmp; + } } if (ce->destructor) { - ce->destructor = zend_shared_alloc_get_xlat_entry(ce->destructor); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->destructor); + if (tmp != NULL) { + ce->destructor = tmp; + } } if (ce->clone) { - ce->clone = zend_shared_alloc_get_xlat_entry(ce->clone); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->clone); + if (tmp != NULL) { + ce->clone = tmp; + } } if (ce->__get) { - ce->__get = zend_shared_alloc_get_xlat_entry(ce->__get); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__get); + if (tmp != NULL) { + ce->__get = tmp; + } } if (ce->__set) { - ce->__set = zend_shared_alloc_get_xlat_entry(ce->__set); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__set); + if (tmp != NULL) { + ce->__set = tmp; + } } if (ce->__call) { - ce->__call = zend_shared_alloc_get_xlat_entry(ce->__call); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__call); + if (tmp != NULL) { + ce->__call = tmp; + } } if (ce->serialize_func) { - ce->serialize_func = zend_shared_alloc_get_xlat_entry(ce->serialize_func); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->serialize_func); + if (tmp != NULL) { + ce->serialize_func = tmp; + } } if (ce->unserialize_func) { - ce->unserialize_func = zend_shared_alloc_get_xlat_entry(ce->unserialize_func); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->unserialize_func); + if (tmp != NULL) { + ce->unserialize_func = tmp; + } } if (ce->__isset) { - ce->__isset = zend_shared_alloc_get_xlat_entry(ce->__isset); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__isset); + if (tmp != NULL) { + ce->__isset = tmp; + } } if (ce->__unset) { - ce->__unset = zend_shared_alloc_get_xlat_entry(ce->__unset); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__unset); + if (tmp != NULL) { + ce->__unset = tmp; + } } if (ce->__tostring) { - ce->__tostring = zend_shared_alloc_get_xlat_entry(ce->__tostring); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__tostring); + if (tmp != NULL) { + ce->__tostring = tmp; + } } if (ce->__callstatic) { - ce->__callstatic = zend_shared_alloc_get_xlat_entry(ce->__callstatic); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__callstatic); + if (tmp != NULL) { + ce->__callstatic = tmp; + } } if (ce->__debugInfo) { - ce->__debugInfo = zend_shared_alloc_get_xlat_entry(ce->__debugInfo); + zend_function *tmp = zend_shared_alloc_get_xlat_entry(ce->__debugInfo); + if (tmp != NULL) { + ce->__debugInfo = tmp; + } } // zend_hash_apply(&ce->properties_info, (apply_func_t) zend_update_property_info_ce); return 0;