From 0a74da385dd1cf25cf7870fdc82cbe1c7256fab3 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 19 May 2020 15:01:18 +0200 Subject: [PATCH] Add support for replaying warnings in opcache If opcache.record_warnings is enabled, opcache will record compilation warnings and replay them when the file is included again. The primary use case I have in mind for this is automated testing of the opcache file cache. This resolves bug #76535. --- NEWS | 1 + UPGRADING | 5 ++ ext/opcache/ZendAccelerator.c | 66 +++++++++++++++++++++++++++ ext/opcache/ZendAccelerator.h | 14 ++++++ ext/opcache/tests/warning_replay.inc | 3 ++ ext/opcache/tests/warning_replay.phpt | 15 ++++++ ext/opcache/zend_accelerator_module.c | 2 + ext/opcache/zend_file_cache.c | 34 ++++++++++++++ ext/opcache/zend_persist.c | 14 ++++++ ext/opcache/zend_persist_calc.c | 10 ++++ php.ini-development | 5 ++ php.ini-production | 5 ++ 12 files changed, 174 insertions(+) create mode 100644 ext/opcache/tests/warning_replay.inc create mode 100644 ext/opcache/tests/warning_replay.phpt diff --git a/NEWS b/NEWS index 7d42d73cfc..e27c38689c 100644 --- a/NEWS +++ b/NEWS @@ -90,6 +90,7 @@ PHP NEWS - OpCache: . Fixed bug #78654 (Incorrectly computed opcache checksum on files with non-ascii characters). (mhagstrand) + . Fixed bug #76535 (Opcache does not replay compile-time warnings). (Nikita) - PCRE: . Don't ignore invalid escape sequences. (sjon) diff --git a/UPGRADING b/UPGRADING index 05929c548b..a3c471c319 100644 --- a/UPGRADING +++ b/UPGRADING @@ -517,6 +517,11 @@ PHP 8.0 UPGRADE NOTES . Introduce DOMParentNode and DOMChildNode with new traversal and manipulation APIs RFC: https://wiki.php.net/rfc/dom_living_standard_api +- Opcache: + . If the opcache.record_warnings ini setting is enabled, opcache will record + compile-time warnings and replay them on the next include, even if it is + served from cache. + - Zip: . Extension updated to version 1.19.0 . New ZipArchive::lastId property to get index value of last added entry. diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 2b858bc228..79941a7d13 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -121,6 +121,7 @@ zend_bool fallback_process = 0; /* process uses file cache fallback */ static zend_op_array *(*accelerator_orig_compile_file)(zend_file_handle *file_handle, int type); static int (*accelerator_orig_zend_stream_open_function)(const char *filename, zend_file_handle *handle ); static zend_string *(*accelerator_orig_zend_resolve_path)(const char *filename, size_t filename_len); +static void (*accelerator_orig_zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args); static zif_handler orig_chdir = NULL; static ZEND_INI_MH((*orig_include_path_on_modify)) = NULL; static int (*orig_post_startup_cb)(void); @@ -1661,6 +1662,56 @@ static void zend_accel_init_auto_globals(void) } } +static void persistent_error_cb(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) { + if (ZCG(record_warnings)) { + zend_recorded_warning *warning = emalloc(sizeof(zend_recorded_warning)); + va_list args_copy; + warning->type = type; + warning->error_lineno = error_lineno; + warning->error_filename = zend_string_init(error_filename, strlen(error_filename), 0); + va_copy(args_copy, args); + warning->error_message = zend_vstrpprintf(0, format, args_copy); + va_end(args_copy); + + ZCG(num_warnings)++; + ZCG(warnings) = erealloc(ZCG(warnings), sizeof(zend_recorded_warning) * ZCG(num_warnings)); + ZCG(warnings)[ZCG(num_warnings)-1] = warning; + } + accelerator_orig_zend_error_cb(type, error_filename, error_lineno, format, args); +} + +/* Hack to get us a va_list to pass to zend_error_cb. */ +static void replay_warning_helper(const zend_recorded_warning *warning, ...) { + va_list va; + va_start(va, warning); + accelerator_orig_zend_error_cb( + warning->type, ZSTR_VAL(warning->error_filename), warning->error_lineno, "%s", va); + va_end(va); +} + +static void replay_warnings(zend_persistent_script *script) { + for (uint32_t i = 0; i < script->num_warnings; i++) { + zend_recorded_warning *warning = script->warnings[i]; + replay_warning_helper(warning, ZSTR_VAL(warning->error_message)); + } +} + +static void free_recorded_warnings() { + if (!ZCG(num_warnings)) { + return; + } + + for (uint32_t i = 0; i < ZCG(num_warnings); i++) { + zend_recorded_warning *warning = ZCG(warnings)[i]; + zend_string_release(warning->error_filename); + zend_string_release(warning->error_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, const char *key, zend_op_array **op_array_p) { zend_persistent_script *new_persistent_script; @@ -1739,6 +1790,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; zend_try { orig_compiler_options = CG(compiler_options); @@ -1761,9 +1815,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; if (!op_array) { /* compilation failed */ + free_recorded_warnings(); if (do_bailout) { zend_bailout(); } @@ -1782,6 +1838,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; efree(op_array); /* we have valid persistent_script, so it's safe to free op_array */ @@ -1866,6 +1926,7 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type) } } } + replay_warnings(persistent_script); zend_file_handle_dtor(file_handle); if (persistent_script->ping_auto_globals_mask) { @@ -2187,6 +2248,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) } } } + replay_warnings(persistent_script); zend_file_handle_dtor(file_handle); from_shared_memory = 1; } @@ -3051,6 +3113,10 @@ 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 7dd9267e8f..05a05fa1b6 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -109,6 +109,13 @@ typedef enum _zend_accel_restart_reason { ACCEL_RESTART_USER /* restart scheduled by opcache_reset() */ } zend_accel_restart_reason; +typedef struct _zend_recorded_warning { + int type; + uint32_t error_lineno; + zend_string *error_filename; + zend_string *error_message; +} zend_recorded_warning; + typedef struct _zend_persistent_script { zend_script script; zend_long compiler_halt_offset; /* position of __HALT_COMPILER or -1 */ @@ -117,6 +124,8 @@ typedef struct _zend_persistent_script { zend_bool corrupted; zend_bool is_phar; zend_bool empty; + uint32_t num_warnings; + zend_recorded_warning **warnings; void *mem; /* shared memory area used by script structures */ size_t size; /* size of used shared memory */ @@ -151,6 +160,7 @@ typedef struct _zend_accel_directives { zend_bool validate_timestamps; zend_bool revalidate_path; zend_bool save_comments; + zend_bool record_warnings; zend_bool protect_memory; zend_bool file_override_enabled; zend_bool enable_cli; @@ -221,6 +231,10 @@ typedef struct _zend_accel_globals { void *arena_mem; zend_persistent_script *current_persistent_script; zend_bool is_immutable_class; + /* Temporary storage for warnings before they are moved into persistent_script. */ + zend_bool record_warnings; + uint32_t num_warnings; + zend_recorded_warning **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/tests/warning_replay.inc b/ext/opcache/tests/warning_replay.inc new file mode 100644 index 0000000000..4931d79573 --- /dev/null +++ b/ext/opcache/tests/warning_replay.inc @@ -0,0 +1,3 @@ + +--EXPECTF-- +Warning: Unsupported declare 'unknown' in %swarning_replay.inc on line 3 + +Warning: Unsupported declare 'unknown' in %swarning_replay.inc on line 3 diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 151bc31123..4a535591c0 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -280,6 +280,7 @@ ZEND_INI_BEGIN() STD_PHP_INI_BOOLEAN("opcache.protect_memory" , "0" , PHP_INI_SYSTEM, OnUpdateBool, accel_directives.protect_memory, zend_accel_globals, accel_globals) STD_PHP_INI_BOOLEAN("opcache.save_comments" , "1" , PHP_INI_SYSTEM, OnUpdateBool, accel_directives.save_comments, zend_accel_globals, accel_globals) + STD_PHP_INI_BOOLEAN("opcache.record_warnings" , "0" , PHP_INI_SYSTEM, OnUpdateBool, accel_directives.record_warnings, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.optimization_level" , DEFAULT_OPTIMIZATION_LEVEL , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.optimization_level, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.opt_debug_level" , "0" , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.opt_debug_level, zend_accel_globals, accel_globals) @@ -767,6 +768,7 @@ ZEND_FUNCTION(opcache_get_configuration) add_assoc_bool(&directives, "opcache.protect_memory", ZCG(accel_directives).protect_memory); add_assoc_bool(&directives, "opcache.save_comments", ZCG(accel_directives).save_comments); + add_assoc_bool(&directives, "opcache.record_warnings", ZCG(accel_directives).record_warnings); add_assoc_bool(&directives, "opcache.enable_file_override", ZCG(accel_directives).file_override_enabled); add_assoc_long(&directives, "opcache.optimization_level", ZCG(accel_directives).optimization_level); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index a853fe3c84..631d712ba4 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -802,6 +802,26 @@ static void zend_file_cache_serialize_class(zval *zv, ZEND_MAP_PTR_INIT(ce->static_members_table, &ce->default_static_members_table); } +static void zend_file_cache_serialize_warnings( + zend_persistent_script *script, zend_file_cache_metainfo *info, void *buf) +{ + if (script->warnings) { + zend_recorded_warning **warnings; + SERIALIZE_PTR(script->warnings); + warnings = script->warnings; + UNSERIALIZE_PTR(warnings); + + for (uint32_t i = 0; i < script->num_warnings; i++) { + zend_recorded_warning *warning; + SERIALIZE_PTR(warnings[i]); + warning = warnings[i]; + UNSERIALIZE_PTR(warning); + SERIALIZE_STR(warning->error_filename); + SERIALIZE_STR(warning->error_message); + } + } +} + static void zend_file_cache_serialize(zend_persistent_script *script, zend_file_cache_metainfo *info, void *buf) @@ -823,6 +843,7 @@ static void zend_file_cache_serialize(zend_persistent_script *script, zend_file_cache_serialize_hash(&new_script->script.class_table, script, info, buf, zend_file_cache_serialize_class); zend_file_cache_serialize_hash(&new_script->script.function_table, script, info, buf, zend_file_cache_serialize_func); zend_file_cache_serialize_op_array(&new_script->script.main_op_array, script, info, buf); + zend_file_cache_serialize_warnings(new_script, info, buf); SERIALIZE_PTR(new_script->arena_mem); new_script->mem = NULL; @@ -1506,6 +1527,18 @@ static void zend_file_cache_unserialize_class(zval *zv, } } +static void zend_file_cache_unserialize_warnings(zend_persistent_script *script, void *buf) +{ + if (script->warnings) { + UNSERIALIZE_PTR(script->warnings); + for (uint32_t i = 0; i < script->num_warnings; i++) { + UNSERIALIZE_PTR(script->warnings[i]); + UNSERIALIZE_STR(script->warnings[i]->error_filename); + UNSERIALIZE_STR(script->warnings[i]->error_message); + } + } +} + static void zend_file_cache_unserialize(zend_persistent_script *script, void *buf) { @@ -1518,6 +1551,7 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_hash(&script->script.function_table, script, buf, zend_file_cache_unserialize_func, ZEND_FUNCTION_DTOR); zend_file_cache_unserialize_op_array(&script->script.main_op_array, script, buf); + zend_file_cache_unserialize_warnings(script, buf); UNSERIALIZE_PTR(script->arena_mem); } diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 23f1a5fb61..ccdc3cbdb9 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -1087,6 +1087,19 @@ 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_recorded_warning *)); + for (uint32_t i = 0; i < script->num_warnings; i++) { + script->warnings[i] = zend_shared_memdup_free( + script->warnings[i], sizeof(zend_recorded_warning)); + zend_accel_store_string(script->warnings[i]->error_filename); + zend_accel_store_string(script->warnings[i]->error_message); + } + } +} + zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script, const char **key, unsigned int key_length, int for_shm) { Bucket *p; @@ -1136,6 +1149,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script zend_persist_op_array(&p->val); } ZEND_HASH_FOREACH_END(); zend_persist_op_array_ex(&script->script.main_op_array, script); + zend_persist_warnings(script); if (for_shm) { ZCSG(map_ptr_last) = CG(map_ptr_last); diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 5de27b9efb..8a1e99c65d 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -514,6 +514,15 @@ 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_recorded_warning *)); + for (uint32_t i = 0; i < script->num_warnings; i++) { + ADD_SIZE(sizeof(zend_recorded_warning)); + ADD_STRING(script->warnings[i]->error_filename); + ADD_STRING(script->warnings[i]->error_message); + } +} + uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_script, const char *key, unsigned int key_length, int for_shm) { Bucket *p; @@ -556,6 +565,7 @@ 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); #if defined(__AVX__) || defined(__SSE2__) /* Align size to 64-byte boundary */ diff --git a/php.ini-development b/php.ini-development index 5ef32525b1..1fcd0e45a2 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1776,6 +1776,11 @@ ldap.max_links = -1 ; size of the optimized code. ;opcache.save_comments=1 +; If enabled, compilation warnings (including notices and deprecations) will +; be recorded and replayed each time a file is included. Otherwise, compilation +; warnings will only be emitted when the file is first cached. +;opcache.record_warnings=0 + ; Allow file existence override (file_exists, etc.) performance feature. ;opcache.enable_file_override=0 diff --git a/php.ini-production b/php.ini-production index fd9cffa639..761b0c2107 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1778,6 +1778,11 @@ ldap.max_links = -1 ; size of the optimized code. ;opcache.save_comments=1 +; If enabled, compilation warnings (including notices and deprecations) will +; be recorded and replayed each time a file is included. Otherwise, compilation +; warnings will only be emitted when the file is first cached. +;opcache.record_warnings=0 + ; Allow file existence override (file_exists, etc.) performance feature. ;opcache.enable_file_override=0 -- 2.40.0