Skip to content

Commit dd86987

Browse files
nikicnikic
authored
Replay warnings during inheritance (#6928)
Since 3e6b447 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.
1 parent f84936b commit dd86987

File tree

10 files changed

+103
-72
lines changed

10 files changed

+103
-72
lines changed

‎Zend/zend.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,9 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
779779
zend_get_windows_version_info(&executor_globals->windows_version_info);
780780
#endif
781781
executor_globals->flags = EG_FLAGS_INITIAL;
782+
executor_globals->record_errors = false;
783+
executor_globals->num_errors = 0;
784+
executor_globals->errors = NULL;
782785
}
783786
/* }}} */
784787

@@ -1356,6 +1359,20 @@ ZEND_API ZEND_COLD void zend_error_zstr_at(
13561359
return;
13571360
}
13581361

1362+
if (EG(record_errors)) {
1363+
zend_error_info *info = emalloc(sizeof(zend_error_info));
1364+
info->type = type;
1365+
info->lineno = error_lineno;
1366+
info->filename = zend_string_copy(error_filename);
1367+
info->message = zend_string_copy(message);
1368+
1369+
/* This is very inefficient for a large number of errors.
1370+
* Use pow2 realloc if it becomes a problem. */
1371+
EG(num_errors)++;
1372+
EG(errors) = erealloc(EG(errors), sizeof(zend_error_info) * EG(num_errors));
1373+
EG(errors)[EG(num_errors)-1] = info;
1374+
}
1375+
13591376
/* Report about uncaught exception in case of fatal errors */
13601377
if (EG(exception)) {
13611378
zend_execute_data *ex;
@@ -1594,6 +1611,31 @@ ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message) {
15941611
zend_error_zstr_at(type, filename, lineno, message);
15951612
}
15961613

1614+
ZEND_API void zend_begin_record_errors(void)
1615+
{
1616+
ZEND_ASSERT(!EG(record_errors) && "Error recoreding already enabled");
1617+
EG(record_errors) = true;
1618+
EG(num_errors) = 0;
1619+
EG(errors) = NULL;
1620+
}
1621+
1622+
ZEND_API void zend_free_recorded_errors(void)
1623+
{
1624+
if (!EG(num_errors)) {
1625+
return;
1626+
}
1627+
1628+
for (uint32_t i = 0; i < EG(num_errors); i++) {
1629+
zend_error_info *info = EG(errors)[i];
1630+
zend_string_release(info->filename);
1631+
zend_string_release(info->message);
1632+
efree(info);
1633+
}
1634+
efree(EG(errors));
1635+
EG(errors) = NULL;
1636+
EG(num_errors) = 0;
1637+
}
1638+
15971639
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */
15981640
{
15991641
va_list va;

‎Zend/zend.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,16 @@ typedef struct _zend_class_dependency {
119119
} zend_class_dependency;
120120

121121
typedef struct _zend_inheritance_cache_entry zend_inheritance_cache_entry;
122+
typedef struct _zend_error_info zend_error_info;
122123

123124
struct _zend_inheritance_cache_entry {
124125
zend_inheritance_cache_entry *next;
125126
zend_class_entry *ce;
126127
zend_class_entry *parent;
127128
zend_class_dependency *dependencies;
128129
uint32_t dependencies_count;
130+
uint32_t num_warnings;
131+
zend_error_info **warnings;
129132
zend_class_entry *traits_and_interfaces[1];
130133
};
131134

@@ -388,6 +391,8 @@ typedef struct _zend_error_info {
388391
ZEND_API void zend_save_error_handling(zend_error_handling *current);
389392
ZEND_API void zend_replace_error_handling(zend_error_handling_t error_handling, zend_class_entry *exception_class, zend_error_handling *current);
390393
ZEND_API void zend_restore_error_handling(zend_error_handling *saved);
394+
ZEND_API void zend_begin_record_errors(void);
395+
ZEND_API void zend_free_recorded_errors(void);
391396

392397
#define DEBUG_BACKTRACE_PROVIDE_OBJECT (1<<0)
393398
#define DEBUG_BACKTRACE_IGNORE_ARGS (1<<1)

‎Zend/zend_execute_API.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@ void init_executor(void) /* {{{ */
188188

189189
EG(get_gc_buffer).start = EG(get_gc_buffer).end = EG(get_gc_buffer).cur = NULL;
190190

191+
EG(record_errors) = false;
192+
EG(num_errors) = 0;
193+
EG(errors) = NULL;
194+
191195
zend_fiber_init();
192196
zend_weakrefs_init();
193197

‎Zend/zend_globals.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,12 @@ struct _zend_executor_globals {
260260
/* Pointer to fatal error that occurred in a fiber while switching to {main}. */
261261
zend_error_info *fiber_error;
262262

263+
/* If record_errors is enabled, all emitted diagnostics will be recorded,
264+
* in addition to being processed as usual. */
265+
bool record_errors;
266+
uint32_t num_errors;
267+
zend_error_info **errors;
268+
263269
void *reserved[ZEND_MAX_RESERVED_RESOURCES];
264270
};
265271

‎Zend/zend_inheritance.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2681,6 +2681,10 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
26812681
}
26822682
return ret;
26832683
}
2684+
2685+
/* Make sure warnings (such as deprecations) thrown during inheritance
2686+
* will be recoreded in the inheritance cache. */
2687+
zend_begin_record_errors();
26842688
} else {
26852689
is_cacheable = 0;
26862690
}
@@ -2751,6 +2755,7 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
27512755
}
27522756

27532757
zend_build_properties_info_table(ce);
2758+
EG(record_errors) = false;
27542759

27552760
if (!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE)) {
27562761
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
27962801
}
27972802
}
27982803

2804+
zend_free_recorded_errors();
27992805
if (traits_and_interfaces) {
28002806
free_alloca(traits_and_interfaces, use_heap);
28012807
}

‎ext/opcache/ZendAccelerator.c

Lines changed: 22 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ static zend_class_entry* (*accelerator_orig_inheritance_cache_get)(zend_class_en
122122
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);
123123
static zend_result (*accelerator_orig_zend_stream_open_function)(zend_file_handle *handle );
124124
static zend_string *(*accelerator_orig_zend_resolve_path)(zend_string *filename);
125-
static void (*accelerator_orig_zend_error_cb)(int type, zend_string *error_filename, const uint32_t error_lineno, zend_string *message);
126125
static zif_handler orig_chdir = NULL;
127126
static ZEND_INI_MH((*orig_include_path_on_modify)) = NULL;
128127
static zend_result (*orig_post_startup_cb)(void);
@@ -1686,45 +1685,13 @@ static void zend_accel_set_auto_globals(int mask)
16861685
ZCG(auto_globals_mask) |= mask;
16871686
}
16881687

1689-
static void persistent_error_cb(int type, zend_string *filename, const uint32_t lineno, zend_string *message) {
1690-
if (ZCG(record_warnings)) {
1691-
zend_error_info *warning = emalloc(sizeof(zend_error_info));
1692-
warning->type = type;
1693-
warning->lineno = lineno;
1694-
warning->filename = zend_string_copy(filename);
1695-
warning->message = zend_string_copy(message);
1696-
1697-
ZCG(num_warnings)++;
1698-
ZCG(warnings) = erealloc(ZCG(warnings), sizeof(zend_error_info) * ZCG(num_warnings));
1699-
ZCG(warnings)[ZCG(num_warnings)-1] = warning;
1700-
}
1701-
accelerator_orig_zend_error_cb(type, filename, lineno, message);
1702-
}
1703-
1704-
static void replay_warnings(zend_persistent_script *script) {
1705-
for (uint32_t i = 0; i < script->num_warnings; i++) {
1706-
zend_error_info *warning = script->warnings[i];
1707-
accelerator_orig_zend_error_cb(
1708-
warning->type, warning->filename, warning->lineno, warning->message);
1688+
static void replay_warnings(uint32_t num_warnings, zend_error_info **warnings) {
1689+
for (uint32_t i = 0; i < num_warnings; i++) {
1690+
zend_error_info *warning = warnings[i];
1691+
zend_error_zstr_at(warning->type, warning->filename, warning->lineno, warning->message);
17091692
}
17101693
}
17111694

1712-
static void free_recorded_warnings() {
1713-
if (!ZCG(num_warnings)) {
1714-
return;
1715-
}
1716-
1717-
for (uint32_t i = 0; i < ZCG(num_warnings); i++) {
1718-
zend_error_info *warning = ZCG(warnings)[i];
1719-
zend_string_release(warning->filename);
1720-
zend_string_release(warning->message);
1721-
efree(warning);
1722-
}
1723-
efree(ZCG(warnings));
1724-
ZCG(warnings) = NULL;
1725-
ZCG(num_warnings) = 0;
1726-
}
1727-
17281695
static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handle, int type, zend_op_array **op_array_p)
17291696
{
17301697
zend_persistent_script *new_persistent_script;
@@ -1802,9 +1769,9 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
18021769

18031770
/* Override them with ours */
18041771
ZVAL_UNDEF(&EG(user_error_handler));
1805-
ZCG(record_warnings) = ZCG(accel_directives).record_warnings;
1806-
ZCG(num_warnings) = 0;
1807-
ZCG(warnings) = NULL;
1772+
if (ZCG(accel_directives).record_warnings) {
1773+
zend_begin_record_errors();
1774+
}
18081775

18091776
zend_try {
18101777
orig_compiler_options = CG(compiler_options);
@@ -1827,11 +1794,11 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
18271794
/* Restore originals */
18281795
CG(active_op_array) = orig_active_op_array;
18291796
EG(user_error_handler) = orig_user_error_handler;
1830-
ZCG(record_warnings) = 0;
1797+
EG(record_errors) = 0;
18311798

18321799
if (!op_array) {
18331800
/* compilation failed */
1834-
free_recorded_warnings();
1801+
zend_free_recorded_errors();
18351802
if (do_bailout) {
18361803
zend_bailout();
18371804
}
@@ -1850,10 +1817,10 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
18501817
(op_array->fn_flags & ZEND_ACC_EARLY_BINDING) ?
18511818
zend_build_delayed_early_binding_list(op_array) :
18521819
(uint32_t)-1;
1853-
new_persistent_script->num_warnings = ZCG(num_warnings);
1854-
new_persistent_script->warnings = ZCG(warnings);
1855-
ZCG(num_warnings) = 0;
1856-
ZCG(warnings) = NULL;
1820+
new_persistent_script->num_warnings = EG(num_errors);
1821+
new_persistent_script->warnings = EG(errors);
1822+
EG(num_errors) = 0;
1823+
EG(errors) = NULL;
18571824

18581825
efree(op_array); /* we have valid persistent_script, so it's safe to free op_array */
18591826

@@ -1935,7 +1902,7 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type)
19351902
}
19361903
}
19371904
}
1938-
replay_warnings(persistent_script);
1905+
replay_warnings(persistent_script->num_warnings, persistent_script->warnings);
19391906

19401907
if (persistent_script->ping_auto_globals_mask & ~ZCG(auto_globals_mask)) {
19411908
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)
22522219
}
22532220
}
22542221
}
2255-
replay_warnings(persistent_script);
2222+
replay_warnings(persistent_script->num_warnings, persistent_script->warnings);
22562223
from_shared_memory = 1;
22572224
}
22582225

@@ -2327,6 +2294,7 @@ static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce,
23272294
if (ZCSG(map_ptr_last) > CG(map_ptr_last)) {
23282295
zend_map_ptr_extend(ZCSG(map_ptr_last));
23292296
}
2297+
replay_warnings(entry->num_warnings, entry->warnings);
23302298
return entry->ce;
23312299
}
23322300

@@ -2398,6 +2366,7 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
23982366
}
23992367
ZCG(current_persistent_script) = &dummy;
24002368
zend_persist_class_entry_calc(ce);
2369+
zend_persist_warnings_calc(EG(num_errors), EG(errors));
24012370
size = dummy.size;
24022371

24032372
zend_shared_alloc_clear_xlat_table();
@@ -2454,6 +2423,11 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
24542423
entry->next = proto->inheritance_cache;
24552424
proto->inheritance_cache = entry;
24562425

2426+
entry->num_warnings = EG(num_errors);
2427+
entry->warnings = zend_persist_warnings(EG(num_errors), EG(errors));
2428+
EG(num_errors) = 0;
2429+
EG(errors) = NULL;
2430+
24572431
zend_shared_alloc_destroy_xlat_table();
24582432

24592433
zend_shared_alloc_unlock();
@@ -3301,10 +3275,6 @@ static zend_result accel_post_startup(void)
33013275
accelerator_orig_zend_resolve_path = zend_resolve_path;
33023276
zend_resolve_path = persistent_zend_resolve_path;
33033277

3304-
/* Override error callback, so we can store errors that occur during compilation */
3305-
accelerator_orig_zend_error_cb = zend_error_cb;
3306-
zend_error_cb = persistent_error_cb;
3307-
33083278
/* Override chdir() function */
33093279
if ((func = zend_hash_str_find_ptr(CG(function_table), "chdir", sizeof("chdir")-1)) != NULL &&
33103280
func->type == ZEND_INTERNAL_FUNCTION) {

‎ext/opcache/ZendAccelerator.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,6 @@ typedef struct _zend_accel_globals {
219219
/* preallocated shared-memory block to save current script */
220220
void *mem;
221221
zend_persistent_script *current_persistent_script;
222-
/* Temporary storage for warnings before they are moved into persistent_script. */
223-
bool record_warnings;
224-
uint32_t num_warnings;
225-
zend_error_info **warnings;
226222
/* cache to save hash lookup on the same INCLUDE opcode */
227223
const zend_op *cache_opline;
228224
zend_persistent_script *cache_persistent_script;

‎ext/opcache/zend_persist.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,17 +1268,16 @@ static void zend_accel_persist_class_table(HashTable *class_table)
12681268
} ZEND_HASH_FOREACH_END();
12691269
}
12701270

1271-
static void zend_persist_warnings(zend_persistent_script *script) {
1272-
if (script->warnings) {
1273-
script->warnings = zend_shared_memdup_free(
1274-
script->warnings, script->num_warnings * sizeof(zend_error_info *));
1275-
for (uint32_t i = 0; i < script->num_warnings; i++) {
1276-
script->warnings[i] = zend_shared_memdup_free(
1277-
script->warnings[i], sizeof(zend_error_info));
1278-
zend_accel_store_string(script->warnings[i]->filename);
1279-
zend_accel_store_string(script->warnings[i]->message);
1271+
zend_error_info **zend_persist_warnings(uint32_t num_warnings, zend_error_info **warnings) {
1272+
if (warnings) {
1273+
warnings = zend_shared_memdup_free(warnings, num_warnings * sizeof(zend_error_info *));
1274+
for (uint32_t i = 0; i < num_warnings; i++) {
1275+
warnings[i] = zend_shared_memdup_free(warnings[i], sizeof(zend_error_info));
1276+
zend_accel_store_string(warnings[i]->filename);
1277+
zend_accel_store_string(warnings[i]->message);
12801278
}
12811279
}
1280+
return warnings;
12821281
}
12831282

12841283
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
13291328
ZEND_MAP_PTR_NEW(script->script.main_op_array.static_variables_ptr);
13301329
}
13311330
}
1332-
zend_persist_warnings(script);
1331+
script->warnings = zend_persist_warnings(script->num_warnings, script->warnings);
13331332

13341333
if (for_shm) {
13351334
ZCSG(map_ptr_last) = CG(map_ptr_last);

‎ext/opcache/zend_persist.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script
2828
void zend_persist_class_entry_calc(zend_class_entry *ce);
2929
zend_class_entry *zend_persist_class_entry(zend_class_entry *ce);
3030
void zend_update_parent_ce(zend_class_entry *ce);
31+
void zend_persist_warnings_calc(uint32_t num_warnings, zend_error_info **warnings);
32+
zend_error_info **zend_persist_warnings(uint32_t num_warnings, zend_error_info **warnings);
3133

3234
#endif /* ZEND_PERSIST_H */

‎ext/opcache/zend_persist_calc.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,12 +561,12 @@ static void zend_accel_persist_class_table_calc(HashTable *class_table)
561561
} ZEND_HASH_FOREACH_END();
562562
}
563563

564-
static void zend_persist_warnings_calc(zend_persistent_script *script) {
565-
ADD_SIZE(script->num_warnings * sizeof(zend_error_info *));
566-
for (uint32_t i = 0; i < script->num_warnings; i++) {
564+
void zend_persist_warnings_calc(uint32_t num_warnings, zend_error_info **warnings) {
565+
ADD_SIZE(num_warnings * sizeof(zend_error_info *));
566+
for (uint32_t i = 0; i < num_warnings; i++) {
567567
ADD_SIZE(sizeof(zend_error_info));
568-
ADD_STRING(script->warnings[i]->filename);
569-
ADD_STRING(script->warnings[i]->message);
568+
ADD_STRING(warnings[i]->filename);
569+
ADD_STRING(warnings[i]->message);
570570
}
571571
}
572572

@@ -606,7 +606,8 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s
606606
zend_persist_op_array_calc(&p->val);
607607
} ZEND_HASH_FOREACH_END();
608608
zend_persist_op_array_calc_ex(&new_persistent_script->script.main_op_array);
609-
zend_persist_warnings_calc(new_persistent_script);
609+
zend_persist_warnings_calc(
610+
new_persistent_script->num_warnings, new_persistent_script->warnings);
610611

611612
new_persistent_script->corrupted = 0;
612613

0 commit comments

Comments
 (0)