From dd86987b2cbead2af1e95460cc5cebe03e83f736 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 29 Apr 2021 16:37:53 +0200 Subject: [PATCH] Replay warnings during inheritance (#6928) Since 3e6b447979a2b1f351faf40bee9c6cf7e362d85a it is again possible to have warnings (deprecations) during inheritance, and more such functionality is likely in the future. This is a problem, because such warnings will only be shown on the first request if the opcache inheritance cache is used. This currently causes test failures in --repeat builds. Fix this by uplifting the error recording functionality from opcache to Zend, and then using it to persist a warning trace in the inheritance cache, which can then be used to replay the warnings on subsequent executions. --- Zend/zend.c | 42 +++++++++++++++++++ Zend/zend.h | 5 +++ Zend/zend_execute_API.c | 4 ++ Zend/zend_globals.h | 6 +++ Zend/zend_inheritance.c | 6 +++ ext/opcache/ZendAccelerator.c | 74 ++++++++++----------------------- ext/opcache/ZendAccelerator.h | 4 -- ext/opcache/zend_persist.c | 19 ++++----- ext/opcache/zend_persist.h | 2 + ext/opcache/zend_persist_calc.c | 13 +++--- 10 files changed, 103 insertions(+), 72 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index 3bb6fe2045e..6635a6e7def 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -779,6 +779,9 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{ zend_get_windows_version_info(&executor_globals->windows_version_info); #endif executor_globals->flags = EG_FLAGS_INITIAL; + executor_globals->record_errors = false; + executor_globals->num_errors = 0; + executor_globals->errors = NULL; } /* }}} */ @@ -1356,6 +1359,20 @@ ZEND_API ZEND_COLD void zend_error_zstr_at( return; } + if (EG(record_errors)) { + zend_error_info *info = emalloc(sizeof(zend_error_info)); + info->type = type; + info->lineno = error_lineno; + info->filename = zend_string_copy(error_filename); + info->message = zend_string_copy(message); + + /* This is very inefficient for a large number of errors. + * Use pow2 realloc if it becomes a problem. */ + EG(num_errors)++; + EG(errors) = erealloc(EG(errors), sizeof(zend_error_info) * EG(num_errors)); + EG(errors)[EG(num_errors)-1] = info; + } + /* Report about uncaught exception in case of fatal errors */ if (EG(exception)) { zend_execute_data *ex; @@ -1594,6 +1611,31 @@ ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message) { zend_error_zstr_at(type, filename, lineno, message); } +ZEND_API void zend_begin_record_errors(void) +{ + ZEND_ASSERT(!EG(record_errors) && "Error recoreding already enabled"); + EG(record_errors) = true; + EG(num_errors) = 0; + EG(errors) = NULL; +} + +ZEND_API void zend_free_recorded_errors(void) +{ + if (!EG(num_errors)) { + return; + } + + for (uint32_t i = 0; i < EG(num_errors); i++) { + zend_error_info *info = EG(errors)[i]; + zend_string_release(info->filename); + zend_string_release(info->message); + efree(info); + } + efree(EG(errors)); + EG(errors) = NULL; + EG(num_errors) = 0; +} + ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */ { va_list va; diff --git a/Zend/zend.h b/Zend/zend.h index 7d449c8f329..4b791b04867 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -119,6 +119,7 @@ typedef struct _zend_class_dependency { } zend_class_dependency; typedef struct _zend_inheritance_cache_entry zend_inheritance_cache_entry; +typedef struct _zend_error_info zend_error_info; struct _zend_inheritance_cache_entry { zend_inheritance_cache_entry *next; @@ -126,6 +127,8 @@ struct _zend_inheritance_cache_entry { zend_class_entry *parent; zend_class_dependency *dependencies; uint32_t dependencies_count; + uint32_t num_warnings; + zend_error_info **warnings; zend_class_entry *traits_and_interfaces[1]; }; @@ -388,6 +391,8 @@ typedef struct _zend_error_info { ZEND_API void zend_save_error_handling(zend_error_handling *current); ZEND_API void zend_replace_error_handling(zend_error_handling_t error_handling, zend_class_entry *exception_class, zend_error_handling *current); ZEND_API void zend_restore_error_handling(zend_error_handling *saved); +ZEND_API void zend_begin_record_errors(void); +ZEND_API void zend_free_recorded_errors(void); #define DEBUG_BACKTRACE_PROVIDE_OBJECT (1<<0) #define DEBUG_BACKTRACE_IGNORE_ARGS (1<<1) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index d73ed0ea9eb..0a6c28e59e5 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -188,6 +188,10 @@ void init_executor(void) /* {{{ */ EG(get_gc_buffer).start = EG(get_gc_buffer).end = EG(get_gc_buffer).cur = NULL; + EG(record_errors) = false; + EG(num_errors) = 0; + EG(errors) = NULL; + zend_fiber_init(); zend_weakrefs_init(); diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index b94ef1de8d0..4a40a2158ff 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -260,6 +260,12 @@ struct _zend_executor_globals { /* Pointer to fatal error that occurred in a fiber while switching to {main}. */ zend_error_info *fiber_error; + /* If record_errors is enabled, all emitted diagnostics will be recorded, + * in addition to being processed as usual. */ + bool record_errors; + uint32_t num_errors; + zend_error_info **errors; + void *reserved[ZEND_MAX_RESERVED_RESOURCES]; }; diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index d4616d4bd56..58c658ffbc2 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -2681,6 +2681,10 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string } return ret; } + + /* Make sure warnings (such as deprecations) thrown during inheritance + * will be recoreded in the inheritance cache. */ + zend_begin_record_errors(); } else { is_cacheable = 0; } @@ -2751,6 +2755,7 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string } zend_build_properties_info_table(ce); + EG(record_errors) = false; if (!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE)) { ce->ce_flags |= ZEND_ACC_LINKED; @@ -2796,6 +2801,7 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string } } + zend_free_recorded_errors(); if (traits_and_interfaces) { free_alloca(traits_and_interfaces, use_heap); } diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index a55befbb91f..6284f478746 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -122,7 +122,6 @@ static zend_class_entry* (*accelerator_orig_inheritance_cache_get)(zend_class_en static zend_class_entry* (*accelerator_orig_inheritance_cache_add)(zend_class_entry *ce, zend_class_entry *proto, zend_class_entry *parent, zend_class_entry **traits_and_interfaces, HashTable *dependencies); static zend_result (*accelerator_orig_zend_stream_open_function)(zend_file_handle *handle ); static zend_string *(*accelerator_orig_zend_resolve_path)(zend_string *filename); -static void (*accelerator_orig_zend_error_cb)(int type, zend_string *error_filename, const uint32_t error_lineno, zend_string *message); static zif_handler orig_chdir = NULL; static ZEND_INI_MH((*orig_include_path_on_modify)) = NULL; static zend_result (*orig_post_startup_cb)(void); @@ -1686,43 +1685,11 @@ static void zend_accel_set_auto_globals(int mask) ZCG(auto_globals_mask) |= mask; } -static void persistent_error_cb(int type, zend_string *filename, const uint32_t lineno, zend_string *message) { - if (ZCG(record_warnings)) { - zend_error_info *warning = emalloc(sizeof(zend_error_info)); - warning->type = type; - warning->lineno = lineno; - warning->filename = zend_string_copy(filename); - warning->message = zend_string_copy(message); - - ZCG(num_warnings)++; - ZCG(warnings) = erealloc(ZCG(warnings), sizeof(zend_error_info) * ZCG(num_warnings)); - ZCG(warnings)[ZCG(num_warnings)-1] = warning; +static void replay_warnings(uint32_t num_warnings, zend_error_info **warnings) { + for (uint32_t i = 0; i < num_warnings; i++) { + zend_error_info *warning = warnings[i]; + zend_error_zstr_at(warning->type, warning->filename, warning->lineno, warning->message); } - accelerator_orig_zend_error_cb(type, filename, lineno, message); -} - -static void replay_warnings(zend_persistent_script *script) { - for (uint32_t i = 0; i < script->num_warnings; i++) { - zend_error_info *warning = script->warnings[i]; - accelerator_orig_zend_error_cb( - warning->type, warning->filename, warning->lineno, warning->message); - } -} - -static void free_recorded_warnings() { - if (!ZCG(num_warnings)) { - return; - } - - for (uint32_t i = 0; i < ZCG(num_warnings); i++) { - zend_error_info *warning = ZCG(warnings)[i]; - zend_string_release(warning->filename); - zend_string_release(warning->message); - efree(warning); - } - efree(ZCG(warnings)); - ZCG(warnings) = NULL; - ZCG(num_warnings) = 0; } static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handle, int type, zend_op_array **op_array_p) @@ -1802,9 +1769,9 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl /* Override them with ours */ ZVAL_UNDEF(&EG(user_error_handler)); - ZCG(record_warnings) = ZCG(accel_directives).record_warnings; - ZCG(num_warnings) = 0; - ZCG(warnings) = NULL; + if (ZCG(accel_directives).record_warnings) { + zend_begin_record_errors(); + } zend_try { orig_compiler_options = CG(compiler_options); @@ -1827,11 +1794,11 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl /* Restore originals */ CG(active_op_array) = orig_active_op_array; EG(user_error_handler) = orig_user_error_handler; - ZCG(record_warnings) = 0; + EG(record_errors) = 0; if (!op_array) { /* compilation failed */ - free_recorded_warnings(); + zend_free_recorded_errors(); if (do_bailout) { zend_bailout(); } @@ -1850,10 +1817,10 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl (op_array->fn_flags & ZEND_ACC_EARLY_BINDING) ? zend_build_delayed_early_binding_list(op_array) : (uint32_t)-1; - new_persistent_script->num_warnings = ZCG(num_warnings); - new_persistent_script->warnings = ZCG(warnings); - ZCG(num_warnings) = 0; - ZCG(warnings) = NULL; + new_persistent_script->num_warnings = EG(num_errors); + new_persistent_script->warnings = EG(errors); + EG(num_errors) = 0; + EG(errors) = NULL; efree(op_array); /* we have valid persistent_script, so it's safe to free op_array */ @@ -1935,7 +1902,7 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type) } } } - replay_warnings(persistent_script); + replay_warnings(persistent_script->num_warnings, persistent_script->warnings); if (persistent_script->ping_auto_globals_mask & ~ZCG(auto_globals_mask)) { zend_accel_set_auto_globals(persistent_script->ping_auto_globals_mask & ~ZCG(auto_globals_mask)); @@ -2252,7 +2219,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) } } } - replay_warnings(persistent_script); + replay_warnings(persistent_script->num_warnings, persistent_script->warnings); from_shared_memory = 1; } @@ -2327,6 +2294,7 @@ static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce, if (ZCSG(map_ptr_last) > CG(map_ptr_last)) { zend_map_ptr_extend(ZCSG(map_ptr_last)); } + replay_warnings(entry->num_warnings, entry->warnings); return entry->ce; } @@ -2398,6 +2366,7 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce, } ZCG(current_persistent_script) = &dummy; zend_persist_class_entry_calc(ce); + zend_persist_warnings_calc(EG(num_errors), EG(errors)); size = dummy.size; zend_shared_alloc_clear_xlat_table(); @@ -2454,6 +2423,11 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce, entry->next = proto->inheritance_cache; proto->inheritance_cache = entry; + entry->num_warnings = EG(num_errors); + entry->warnings = zend_persist_warnings(EG(num_errors), EG(errors)); + EG(num_errors) = 0; + EG(errors) = NULL; + zend_shared_alloc_destroy_xlat_table(); zend_shared_alloc_unlock(); @@ -3301,10 +3275,6 @@ file_cache_fallback: accelerator_orig_zend_resolve_path = zend_resolve_path; zend_resolve_path = persistent_zend_resolve_path; - /* Override error callback, so we can store errors that occur during compilation */ - accelerator_orig_zend_error_cb = zend_error_cb; - zend_error_cb = persistent_error_cb; - /* Override chdir() function */ if ((func = zend_hash_str_find_ptr(CG(function_table), "chdir", sizeof("chdir")-1)) != NULL && func->type == ZEND_INTERNAL_FUNCTION) { diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index b4cf3052812..fc9ac1682c7 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -219,10 +219,6 @@ typedef struct _zend_accel_globals { /* preallocated shared-memory block to save current script */ void *mem; zend_persistent_script *current_persistent_script; - /* Temporary storage for warnings before they are moved into persistent_script. */ - bool record_warnings; - uint32_t num_warnings; - zend_error_info **warnings; /* cache to save hash lookup on the same INCLUDE opcode */ const zend_op *cache_opline; zend_persistent_script *cache_persistent_script; diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index ecabcc2f21d..13fd3dfebf9 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -1268,17 +1268,16 @@ static void zend_accel_persist_class_table(HashTable *class_table) } ZEND_HASH_FOREACH_END(); } -static void zend_persist_warnings(zend_persistent_script *script) { - if (script->warnings) { - script->warnings = zend_shared_memdup_free( - script->warnings, script->num_warnings * sizeof(zend_error_info *)); - for (uint32_t i = 0; i < script->num_warnings; i++) { - script->warnings[i] = zend_shared_memdup_free( - script->warnings[i], sizeof(zend_error_info)); - zend_accel_store_string(script->warnings[i]->filename); - zend_accel_store_string(script->warnings[i]->message); +zend_error_info **zend_persist_warnings(uint32_t num_warnings, zend_error_info **warnings) { + if (warnings) { + warnings = zend_shared_memdup_free(warnings, num_warnings * sizeof(zend_error_info *)); + for (uint32_t i = 0; i < num_warnings; i++) { + warnings[i] = zend_shared_memdup_free(warnings[i], sizeof(zend_error_info)); + zend_accel_store_string(warnings[i]->filename); + zend_accel_store_string(warnings[i]->message); } } + return warnings; } zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script, int for_shm) @@ -1329,7 +1328,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script ZEND_MAP_PTR_NEW(script->script.main_op_array.static_variables_ptr); } } - zend_persist_warnings(script); + script->warnings = zend_persist_warnings(script->num_warnings, script->warnings); if (for_shm) { ZCSG(map_ptr_last) = CG(map_ptr_last); diff --git a/ext/opcache/zend_persist.h b/ext/opcache/zend_persist.h index f491cb8ca65..55129c41e91 100644 --- a/ext/opcache/zend_persist.h +++ b/ext/opcache/zend_persist.h @@ -28,5 +28,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script void zend_persist_class_entry_calc(zend_class_entry *ce); zend_class_entry *zend_persist_class_entry(zend_class_entry *ce); void zend_update_parent_ce(zend_class_entry *ce); +void zend_persist_warnings_calc(uint32_t num_warnings, zend_error_info **warnings); +zend_error_info **zend_persist_warnings(uint32_t num_warnings, zend_error_info **warnings); #endif /* ZEND_PERSIST_H */ diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index e3df7c99cf7..2d951863a30 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -561,12 +561,12 @@ static void zend_accel_persist_class_table_calc(HashTable *class_table) } ZEND_HASH_FOREACH_END(); } -static void zend_persist_warnings_calc(zend_persistent_script *script) { - ADD_SIZE(script->num_warnings * sizeof(zend_error_info *)); - for (uint32_t i = 0; i < script->num_warnings; i++) { +void zend_persist_warnings_calc(uint32_t num_warnings, zend_error_info **warnings) { + ADD_SIZE(num_warnings * sizeof(zend_error_info *)); + for (uint32_t i = 0; i < num_warnings; i++) { ADD_SIZE(sizeof(zend_error_info)); - ADD_STRING(script->warnings[i]->filename); - ADD_STRING(script->warnings[i]->message); + ADD_STRING(warnings[i]->filename); + ADD_STRING(warnings[i]->message); } } @@ -606,7 +606,8 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s zend_persist_op_array_calc(&p->val); } ZEND_HASH_FOREACH_END(); zend_persist_op_array_calc_ex(&new_persistent_script->script.main_op_array); - zend_persist_warnings_calc(new_persistent_script); + zend_persist_warnings_calc( + new_persistent_script->num_warnings, new_persistent_script->warnings); new_persistent_script->corrupted = 0;