From 14e7c16faa802e2dc43f26dc31e41bfdc3b9d8b3 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 25 Aug 2022 11:06:06 +0200 Subject: [PATCH 1/6] Implement opcache checksum skip list --- ext/opcache/ZendAccelerator.c | 6 ++- ext/opcache/ZendAccelerator.h | 6 +++ ext/opcache/jit/zend_jit.c | 3 +- ext/opcache/jit/zend_jit.h | 3 ++ ext/opcache/jit/zend_jit_trace.c | 4 ++ ext/opcache/tests/gh8065.inc | 3 ++ ext/opcache/tests/gh8065.phpt | 22 ++++++++ ext/opcache/zend_accelerator_util_funcs.c | 49 ++++++++++++++---- ext/opcache/zend_accelerator_util_funcs.h | 2 +- ext/opcache/zend_file_cache.c | 10 ++-- ext/opcache/zend_persist.c | 63 ++++++++++++++++++++++- ext/opcache/zend_persist.h | 1 + ext/opcache/zend_persist_calc.c | 49 +++++++++++++++++- 13 files changed, 201 insertions(+), 20 deletions(-) create mode 100644 ext/opcache/tests/gh8065.inc create mode 100644 ext/opcache/tests/gh8065.phpt diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 97b82378780b8..2b4c3fccd4d42 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2120,7 +2120,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) unsigned int checksum = zend_accel_script_checksum(persistent_script); if (checksum != persistent_script->dynamic_members.checksum ) { /* The checksum is wrong */ - zend_accel_error(ACCEL_LOG_INFO, "Checksum failed for '%s': expected=0x%08x, found=0x%08x", + zend_accel_error(ACCEL_LOG_WARNING, "Checksum failed for '%s': expected=0x%08x, found=0x%08x", ZSTR_VAL(persistent_script->script.filename), persistent_script->dynamic_members.checksum, checksum); zend_shared_alloc_lock(); if (!persistent_script->corrupted) { @@ -4763,6 +4763,10 @@ static int accel_finish_startup(void) ZCG(cwd_key_len) = 0; ZCG(cwd_check) = 1; + ZCG(checksum_skip_list) = NULL; + ZCG(checksum_skip_list_count) = 0; + ZCG(checksum_skip_list_capacity) = 0; + if (accel_preload(ZCG(accel_directives).preload, in_child) != SUCCESS) { ret = FAILURE; } diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index de5ec45de72c8..28b84886ecb9f 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -122,6 +122,8 @@ typedef struct _zend_persistent_script { void *mem; /* shared memory area used by script structures */ size_t size; /* size of used shared memory */ + /* list of offsets relative to (mem + ZEND_ALIGNED_SIZE(sizeof(zend_persistent_script))) to be skipped in the checksum calculation */ + uint32_t *checksum_skip_list; /* All entries that shouldn't be counted in the ADLER32 * checksum must be declared in this struct @@ -225,6 +227,10 @@ typedef struct _zend_accel_globals { /* preallocated buffer for keys */ zend_string key; char _key[MAXPATHLEN * 8]; + + uint32_t *checksum_skip_list; + uint32_t checksum_skip_list_count; + uint32_t checksum_skip_list_capacity; } zend_accel_globals; typedef struct _zend_string_table { diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index d2f6df0537c9d..c77d510b975fb 100644 --- a/ext/opcache/jit/zend_jit.c +++ b/ext/opcache/jit/zend_jit.c @@ -36,7 +36,6 @@ #include "Optimizer/zend_func_info.h" #include "Optimizer/zend_ssa.h" #include "Optimizer/zend_inference.h" -#include "Optimizer/zend_call_graph.h" #include "Optimizer/zend_dump.h" #if ZEND_JIT_TARGET_X86 @@ -1267,7 +1266,7 @@ static int zend_may_overflow(const zend_op *opline, const zend_ssa_op *ssa_op, c } } -static int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg) +ZEND_EXT_API int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg) { uint32_t flags; diff --git a/ext/opcache/jit/zend_jit.h b/ext/opcache/jit/zend_jit.h index 21d1774c111b6..28bcf2747ed73 100644 --- a/ext/opcache/jit/zend_jit.h +++ b/ext/opcache/jit/zend_jit.h @@ -19,6 +19,8 @@ #ifndef HAVE_JIT_H #define HAVE_JIT_H +#include "Optimizer/zend_call_graph.h" + #if defined(__x86_64__) || defined(i386) || defined(ZEND_WIN32) # define ZEND_JIT_TARGET_X86 1 # define ZEND_JIT_TARGET_ARM64 0 @@ -154,6 +156,7 @@ ZEND_EXT_API void zend_jit_activate(void); ZEND_EXT_API void zend_jit_deactivate(void); ZEND_EXT_API void zend_jit_status(zval *ret); ZEND_EXT_API void zend_jit_restart(void); +ZEND_EXT_API int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg); typedef struct _zend_lifetime_interval zend_lifetime_interval; typedef struct _zend_life_range zend_life_range; diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 14cd945bbe650..8d99622d80c99 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -16,6 +16,8 @@ +----------------------------------------------------------------------+ */ +#include "zend_persist.h" + static zend_op_array dummy_op_array; static zend_jit_trace_info *zend_jit_traces = NULL; static const void **zend_jit_exit_groups = NULL; @@ -8262,6 +8264,7 @@ static int zend_jit_setup_hot_trace_counters(zend_op_array *op_array) if (cfg.blocks[i].flags & ZEND_BB_LOOP_HEADER) { /* loop header */ opline = op_array->opcodes + cfg.blocks[i].start; + checksum_skip_list_add(&opline->handler); if (!(ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_UNSUPPORTED)) { opline->handler = (const void*)zend_jit_loop_trace_counter_handler; if (!ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->counter) { @@ -8286,6 +8289,7 @@ static int zend_jit_setup_hot_trace_counters(zend_op_array *op_array) } } + checksum_skip_list_add(&opline->handler); if (!ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->trace_flags) { /* function entry */ opline->handler = (const void*)zend_jit_func_trace_counter_handler; diff --git a/ext/opcache/tests/gh8065.inc b/ext/opcache/tests/gh8065.inc new file mode 100644 index 0000000000000..5f4002edd6290 --- /dev/null +++ b/ext/opcache/tests/gh8065.inc @@ -0,0 +1,3 @@ + +--EXPECT-- +Done diff --git a/ext/opcache/zend_accelerator_util_funcs.c b/ext/opcache/zend_accelerator_util_funcs.c index dd9d11146a1c2..1551a11a5eb9a 100644 --- a/ext/opcache/zend_accelerator_util_funcs.c +++ b/ext/opcache/zend_accelerator_util_funcs.c @@ -299,18 +299,33 @@ zend_op_array* zend_accel_load_script(zend_persistent_script *persistent_script, #define ADLER32_DO4(buf, i) ADLER32_DO2(buf, i); ADLER32_DO2(buf, i + 2); #define ADLER32_DO8(buf, i) ADLER32_DO4(buf, i); ADLER32_DO4(buf, i + 4); #define ADLER32_DO16(buf) ADLER32_DO8(buf, 0); ADLER32_DO8(buf, 8); - -unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len) +#define ADLER32_CHECK_SKIPPED(by) \ + do { \ + offset = buf - start; \ + while (*skip_list != 0 && *skip_list < offset) { \ + skip_list++; \ + } \ + skipped = *skip_list != 0 && *skip_list < (offset + (by)); \ + } while (0) + +unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len, uint32_t *skip_list) { + unsigned char *start = buf; unsigned int s1 = checksum & 0xffff; unsigned int s2 = (checksum >> 16) & 0xffff; unsigned char *end; + bool skipped; + uint32_t offset; while (len >= ADLER32_NMAX) { len -= ADLER32_NMAX; end = buf + ADLER32_NMAX; + skipped = false; do { - ADLER32_DO16(buf); + ADLER32_CHECK_SKIPPED(16); + if (!skipped) { + ADLER32_DO16(buf); + } buf += 16; } while (buf != end); s1 %= ADLER32_BASE; @@ -321,16 +336,27 @@ unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t le if (len >= 16) { end = buf + (len & 0xfff0); len &= 0xf; + skipped = false; do { - ADLER32_DO16(buf); + ADLER32_CHECK_SKIPPED(16); + if (!skipped) { + ADLER32_DO16(buf); + } buf += 16; } while (buf != end); } if (len) { end = buf + len; + skipped = false; do { - ADLER32_DO1(buf); - buf++; + ADLER32_CHECK_SKIPPED(1); + if (!skipped) { + ADLER32_DO1(buf); + buf++; + } else { + // skip_list contains a list of pointers so we skip the entire pointer size + buf += sizeof(void*); + } } while (buf != end); } s1 %= ADLER32_BASE; @@ -346,19 +372,20 @@ unsigned int zend_accel_script_checksum(zend_persistent_script *persistent_scrip size_t size = persistent_script->size; size_t persistent_script_check_block_size = ((char *)&(persistent_script->dynamic_members)) - (char *)persistent_script; unsigned int checksum = ADLER32_INIT; + uint32_t zero = 0; if (mem < (unsigned char*)persistent_script) { - checksum = zend_adler32(checksum, mem, (unsigned char*)persistent_script - mem); + checksum = zend_adler32(checksum, mem, (unsigned char*)persistent_script - mem, &zero); size -= (unsigned char*)persistent_script - mem; mem += (unsigned char*)persistent_script - mem; } - zend_adler32(checksum, mem, persistent_script_check_block_size); - mem += sizeof(*persistent_script); - size -= sizeof(*persistent_script); + checksum = zend_adler32(checksum, mem, persistent_script_check_block_size, &zero); + mem += ZEND_ALIGNED_SIZE(sizeof(*persistent_script)); + size -= ZEND_ALIGNED_SIZE(sizeof(*persistent_script)); if (size > 0) { - checksum = zend_adler32(checksum, mem, size); + checksum = zend_adler32(checksum, mem, size, persistent_script->checksum_skip_list); } return checksum; } diff --git a/ext/opcache/zend_accelerator_util_funcs.h b/ext/opcache/zend_accelerator_util_funcs.h index 88362436026d0..febac273d52d1 100644 --- a/ext/opcache/zend_accelerator_util_funcs.h +++ b/ext/opcache/zend_accelerator_util_funcs.h @@ -35,7 +35,7 @@ zend_op_array* zend_accel_load_script(zend_persistent_script *persistent_script, #define ADLER32_INIT 1 /* initial Adler-32 value */ -unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len); +unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len, uint32_t *skip_list); unsigned int zend_accel_script_checksum(zend_persistent_script *persistent_script); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 22ccc4d84305d..ee6ab9b2b61ae 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1030,8 +1030,11 @@ int zend_file_cache_script_store(zend_persistent_script *script, int in_shm) } zend_shared_alloc_destroy_xlat_table(); - info.checksum = zend_adler32(ADLER32_INIT, buf, script->size); - info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL((zend_string*)ZCG(mem)), info.str_size); + zend_string *const s = (zend_string*)ZCG(mem); + + uint32_t zero = 0; + info.checksum = zend_adler32(ADLER32_INIT, buf, script->size, &zero); + info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL(s), info.str_size, &zero); #if __has_feature(memory_sanitizer) /* The buffer may contain uninitialized regions. However, the uninitialized parts will not be @@ -1775,8 +1778,9 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl close(fd); /* verify checksum */ + uint32_t zero = 0; if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { + (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size, &zero)) != info.checksum) { zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); zend_file_cache_unlink(filename); zend_arena_release(&CG(arena), checkpoint); diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 6ca8c46892ae0..970cab91dc180 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -87,6 +87,60 @@ static void zend_persist_op_array(zval *zv); static const uint32_t uninitialized_bucket[-HT_MIN_MASK] = {HT_INVALID_IDX, HT_INVALID_IDX}; +static void checksum_skip_list_init(void) +{ + ZEND_ASSERT(ZCG(checksum_skip_list) == NULL); + ZCG(checksum_skip_list_count) = 0; + ZCG(checksum_skip_list_capacity) = 8; + ZCG(checksum_skip_list) = safe_emalloc(ZCG(checksum_skip_list_capacity), sizeof(uint32_t), 0); +} + +static void checksum_skip_list_extend(uint32_t size) +{ + ZEND_ASSERT(ZCG(checksum_skip_list) != NULL); + ZCG(checksum_skip_list) = safe_erealloc(ZCG(checksum_skip_list), ZCG(checksum_skip_list_capacity), sizeof(uint32_t), 0); +} + +void checksum_skip_list_add(void *p) +{ + ZEND_ASSERT(ZCG(checksum_skip_list) != NULL); + void *base_btr = ((char*)ZCG(current_persistent_script)->mem) + ZEND_ALIGNED_SIZE(sizeof(zend_persistent_script)); + ZEND_ASSERT(p >= base_btr); + if ((ZCG(checksum_skip_list_count) + 1) >= ZCG(checksum_skip_list_capacity)) { + ZCG(checksum_skip_list_capacity) *= 2; + checksum_skip_list_extend(ZCG(checksum_skip_list_capacity)); + } + ZCG(checksum_skip_list)[ZCG(checksum_skip_list_count)++] = p - base_btr; +} + +static int checksum_skip_list_compare(const uint32_t *l, const uint32_t *r) +{ + if (*l < *r) { + return -1; + } else if (*l == *r) { + return 0; + } else { + return 1; + } +} + +static void checksum_skip_list_swap(uint32_t *l, uint32_t *r) +{ + uint32_t tmp = *r; + *r = *l; + *l = tmp; +} + +static uint32_t *checksum_skip_list_persist(void) +{ + zend_sort((void *) ZCG(checksum_skip_list), ZCG(checksum_skip_list_count), sizeof(*ZCG(checksum_skip_list)), (compare_func_t) checksum_skip_list_compare, (swap_func_t) checksum_skip_list_swap); + uint32_t *result = zend_shared_memdup(ZCG(checksum_skip_list), (ZCG(checksum_skip_list_count) + 1) * sizeof(uint32_t)); + result[ZCG(checksum_skip_list_count)] = 0; + efree(ZCG(checksum_skip_list)); + ZCG(checksum_skip_list) = NULL; + return result; +} + static void zend_hash_persist(HashTable *ht) { uint32_t idx, nIndex; @@ -866,6 +920,9 @@ zend_class_entry *zend_persist_class_entry(zend_class_entry *orig_ce) ce->ce_flags |= ZEND_ACC_FILE_CACHED; } ce->inheritance_cache = NULL; + if (ZCG(checksum_skip_list) != NULL) { + checksum_skip_list_add(&ce->inheritance_cache); + } if (!(ce->ce_flags & ZEND_ACC_CACHED)) { if (ZSTR_HAS_CE_CACHE(ce->name)) { @@ -1290,6 +1347,8 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script script->corrupted = 0; ZCG(current_persistent_script) = script; + checksum_skip_list_init(); + if (!for_shm) { /* script is not going to be saved in SHM */ script->corrupted = 1; @@ -1346,7 +1405,9 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script } #endif - script->corrupted = 0; + script->checksum_skip_list = checksum_skip_list_persist(); + + script->corrupted = false; ZCG(current_persistent_script) = NULL; return script; diff --git a/ext/opcache/zend_persist.h b/ext/opcache/zend_persist.h index 4c7d6f2eeee15..4d73cb5ec185c 100644 --- a/ext/opcache/zend_persist.h +++ b/ext/opcache/zend_persist.h @@ -30,5 +30,6 @@ 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); +void checksum_skip_list_add(void *p); #endif /* ZEND_PERSIST_H */ diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 3f79290841a0c..327821e27f966 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -27,6 +27,11 @@ #include "zend_operators.h" #include "zend_attributes.h" +#ifdef HAVE_JIT +# include "Optimizer/zend_func_info.h" +# include "jit/zend_jit.h" +#endif + #define ADD_DUP_SIZE(m,s) ZCG(current_persistent_script)->size += zend_shared_memdup_size((void*)m, s) #define ADD_SIZE(m) ZCG(current_persistent_script)->size += ZEND_ALIGNED_SIZE(m) @@ -297,6 +302,36 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array) } ADD_SIZE(ZEND_ALIGNED_SIZE(zend_extensions_op_array_persist_calc(op_array))); + +#ifdef HAVE_JIT + if (JIT_G(on)) { + if (JIT_G(hot_loop)) { + zend_cfg cfg; + + if (zend_jit_build_cfg(op_array, &cfg) == SUCCESS) { + for (uint32_t i = 0; i < cfg.blocks_count; i++) { + if (cfg.blocks[i].flags & ZEND_BB_REACHABLE) { + if (cfg.blocks[i].flags & ZEND_BB_LOOP_HEADER) { + // opcode.handler + ZCG(checksum_skip_list_count)++; + } + } + } + } + } + + if (JIT_G(hot_func)) { + zend_op *opline = op_array->opcodes; + if (!(op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS)) { + while (opline->opcode == ZEND_RECV || opline->opcode == ZEND_RECV_INIT) { + opline++; + } + } + // opcode.handler + ZCG(checksum_skip_list_count)++; + } + } +#endif } static void zend_persist_op_array_calc(zval *zv) @@ -399,6 +434,11 @@ void zend_persist_class_entry_calc(zend_class_entry *ce) ADD_SIZE(sizeof(zend_class_entry)); + if (ZCG(current_persistent_script)) { + // inheritance_cache + ZCG(checksum_skip_list_count)++; + } + if (!(ce->ce_flags & ZEND_ACC_CACHED)) { ADD_INTERNED_STRING(ce->name); if (ce->parent_name && !(ce->ce_flags & ZEND_ACC_LINKED)) { @@ -572,6 +612,8 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s { Bucket *p; + ZCG(checksum_skip_list_count) = 0; + new_persistent_script->mem = NULL; new_persistent_script->size = 0; new_persistent_script->corrupted = 0; @@ -607,7 +649,12 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s zend_persist_warnings_calc( new_persistent_script->num_warnings, new_persistent_script->warnings); - new_persistent_script->corrupted = 0; + // null terminator + ZCG(checksum_skip_list_count)++; + ADD_SIZE(ZCG(checksum_skip_list_count) * sizeof(uint32_t)); + ZCG(checksum_skip_list_count) = 0; + + new_persistent_script->corrupted = false; ZCG(current_persistent_script) = NULL; From 91e64af60448b614b58f10a8f651bc20db8b9fde Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Feb 2023 03:17:48 +0100 Subject: [PATCH 2/6] Fix GH-8065: opcache.consistency_checks > 0 causes segfaults in PHP >= 8.1.5 in fpm context Fixes GH-8065 (see also for analysis) The class inheritance cache must be excluded from the checksum computation because it may change (legally) in between the computation and recomputation. Furthermore the handler pointer in the zend_op structure may be modified by the JIT, and the op_array->reserved[] could be changed as well. This approach is based on iluuu1994's original approach, with some slight changes. First let's explain the main idea. The idea is to keep a "skip list" of offsets to skip in the checksum computation. This will include the offsets described above (class inheritance cache, handler, reserved). These skips are of size pointer_size (equals to the native pointer size). It differs in the sense that this is a pragmatic approach. It was found that in the original approach it was very difficult to figure out what can be exactly changed when, and let to redundant computation of the CFG for example. Furthermore, such a process is also error-prone and complex to maintain. Instead of that, we just always skip the handler pointer and the reserved area. The positive is that it leads to code which is easier to understand and maintain. The negative is that we won't catch corruptions in those memory areas. However, I believe this is acceptable because of the following reasons: * We'd have to be very unlucky to *only* have corruptions in *those* areas. Usually, corruptions are not that well-targetted. * A checksum isn't a fully error-proof thing either, it's very possible that there is a corruption in other areas that the checksum doesn't capture. Let's now talk about the implementation. Keeping all those entries in a skip list wouldn't be efficient: we'd need an entry for every opcode, which would lead to many many entries. However, this approach actually implements a "compression" scheme. Instead of just storing the offset, we actually store a triple: . The offset indicates which pointer needs to be skipped. If the number of repetitions is greater than zero, it will do the following `repetitions` times: * Checksum the bytes at [offset + pointer_size, offset + pointer_size + size of area to be checked] * Move the offset to after the area that was just checksummed + pointer_size. This allows us to only use one skip list entry for all the opcodes in an op_array. The offsets also aren't checked in the zend_adler32() function itself, the zend_accel_script_checksum() function will checksum in "blocks" of bytes that should not be skipped, by setting the size for zend_alder32() in such a way that it computes the checksum until the next offset to skip. The main advantage of this is better performance, since there are fewer checks, and the accelerated SSE2 (or future other accelerated) routines can keep being used. Finally some other minor differences: * We precompute the exact size needed for the skip list. This avoids reallocations and also fixes one issue where there was a warning about the computed size not being equals to the actual size. --- ext/opcache/ZendAccelerator.c | 5 +- ext/opcache/ZendAccelerator.h | 24 +++++-- ext/opcache/jit/zend_jit.c | 3 +- ext/opcache/jit/zend_jit.h | 3 - ext/opcache/jit/zend_jit_trace.c | 4 -- ext/opcache/zend_accelerator_util_funcs.c | 75 +++++++++++----------- ext/opcache/zend_accelerator_util_funcs.h | 2 +- ext/opcache/zend_file_cache.c | 10 +-- ext/opcache/zend_persist.c | 78 +++++++++++------------ ext/opcache/zend_persist.h | 1 - ext/opcache/zend_persist_calc.c | 50 ++------------- 11 files changed, 108 insertions(+), 147 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 2b4c3fccd4d42..d8eb8c4225c4f 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4763,9 +4763,8 @@ static int accel_finish_startup(void) ZCG(cwd_key_len) = 0; ZCG(cwd_check) = 1; - ZCG(checksum_skip_list) = NULL; - ZCG(checksum_skip_list_count) = 0; - ZCG(checksum_skip_list_capacity) = 0; + ZCG(mem_checksum_skip_list) = NULL; + ZCG(mem_checksum_skip_list_count) = 0; if (accel_preload(ZCG(accel_directives).preload, in_child) != SUCCESS) { ret = FAILURE; diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 28b84886ecb9f..08dda07a800d8 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -109,6 +109,19 @@ typedef enum _zend_accel_restart_reason { ACCEL_RESTART_USER /* restart scheduled by opcache_reset() */ } zend_accel_restart_reason; +typedef struct _zend_accel_skip_list_entry { + /* The offset indicates which byte offset within the memory block must be skipped. + * The size of the skip is equal to the system's pointer size. */ + uint32_t offset; + /* To prevent creating a huge list with a lot of entries, we use a compression scheme + * based on the following two fields. If repetitions > 0, then the checksum algorithm + * will repeat `repetitions` times checksumming `checked_area_size` bytes, followed + * by skipping a pointer. For example, we use this to skip the `zend_op->handler` pointer. + * This allows us to only create one skip list entry per op array to handle *all* the opcodes. */ + uint32_t checked_area_size; + uint32_t repetitions; +} zend_accel_skip_list_entry; + typedef struct _zend_persistent_script { zend_script script; zend_long compiler_halt_offset; /* position of __HALT_COMPILER or -1 */ @@ -122,8 +135,10 @@ typedef struct _zend_persistent_script { void *mem; /* shared memory area used by script structures */ size_t size; /* size of used shared memory */ - /* list of offsets relative to (mem + ZEND_ALIGNED_SIZE(sizeof(zend_persistent_script))) to be skipped in the checksum calculation */ - uint32_t *checksum_skip_list; + + /* Some bytes must be skipped in the checksum calculation because they could've changed (legally) + * since the last checksum calculation. */ + zend_accel_skip_list_entry *mem_checksum_skip_list; /* All entries that shouldn't be counted in the ADLER32 * checksum must be declared in this struct @@ -228,9 +243,8 @@ typedef struct _zend_accel_globals { zend_string key; char _key[MAXPATHLEN * 8]; - uint32_t *checksum_skip_list; - uint32_t checksum_skip_list_count; - uint32_t checksum_skip_list_capacity; + zend_accel_skip_list_entry *mem_checksum_skip_list; + uint32_t mem_checksum_skip_list_count; } zend_accel_globals; typedef struct _zend_string_table { diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c index c77d510b975fb..d2f6df0537c9d 100644 --- a/ext/opcache/jit/zend_jit.c +++ b/ext/opcache/jit/zend_jit.c @@ -36,6 +36,7 @@ #include "Optimizer/zend_func_info.h" #include "Optimizer/zend_ssa.h" #include "Optimizer/zend_inference.h" +#include "Optimizer/zend_call_graph.h" #include "Optimizer/zend_dump.h" #if ZEND_JIT_TARGET_X86 @@ -1266,7 +1267,7 @@ static int zend_may_overflow(const zend_op *opline, const zend_ssa_op *ssa_op, c } } -ZEND_EXT_API int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg) +static int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg) { uint32_t flags; diff --git a/ext/opcache/jit/zend_jit.h b/ext/opcache/jit/zend_jit.h index 28bcf2747ed73..21d1774c111b6 100644 --- a/ext/opcache/jit/zend_jit.h +++ b/ext/opcache/jit/zend_jit.h @@ -19,8 +19,6 @@ #ifndef HAVE_JIT_H #define HAVE_JIT_H -#include "Optimizer/zend_call_graph.h" - #if defined(__x86_64__) || defined(i386) || defined(ZEND_WIN32) # define ZEND_JIT_TARGET_X86 1 # define ZEND_JIT_TARGET_ARM64 0 @@ -156,7 +154,6 @@ ZEND_EXT_API void zend_jit_activate(void); ZEND_EXT_API void zend_jit_deactivate(void); ZEND_EXT_API void zend_jit_status(zval *ret); ZEND_EXT_API void zend_jit_restart(void); -ZEND_EXT_API int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg); typedef struct _zend_lifetime_interval zend_lifetime_interval; typedef struct _zend_life_range zend_life_range; diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 8d99622d80c99..14cd945bbe650 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -16,8 +16,6 @@ +----------------------------------------------------------------------+ */ -#include "zend_persist.h" - static zend_op_array dummy_op_array; static zend_jit_trace_info *zend_jit_traces = NULL; static const void **zend_jit_exit_groups = NULL; @@ -8264,7 +8262,6 @@ static int zend_jit_setup_hot_trace_counters(zend_op_array *op_array) if (cfg.blocks[i].flags & ZEND_BB_LOOP_HEADER) { /* loop header */ opline = op_array->opcodes + cfg.blocks[i].start; - checksum_skip_list_add(&opline->handler); if (!(ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_UNSUPPORTED)) { opline->handler = (const void*)zend_jit_loop_trace_counter_handler; if (!ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->counter) { @@ -8289,7 +8286,6 @@ static int zend_jit_setup_hot_trace_counters(zend_op_array *op_array) } } - checksum_skip_list_add(&opline->handler); if (!ZEND_OP_TRACE_INFO(opline, jit_extension->offset)->trace_flags) { /* function entry */ opline->handler = (const void*)zend_jit_func_trace_counter_handler; diff --git a/ext/opcache/zend_accelerator_util_funcs.c b/ext/opcache/zend_accelerator_util_funcs.c index 1551a11a5eb9a..245451a9e1ec7 100644 --- a/ext/opcache/zend_accelerator_util_funcs.c +++ b/ext/opcache/zend_accelerator_util_funcs.c @@ -299,33 +299,18 @@ zend_op_array* zend_accel_load_script(zend_persistent_script *persistent_script, #define ADLER32_DO4(buf, i) ADLER32_DO2(buf, i); ADLER32_DO2(buf, i + 2); #define ADLER32_DO8(buf, i) ADLER32_DO4(buf, i); ADLER32_DO4(buf, i + 4); #define ADLER32_DO16(buf) ADLER32_DO8(buf, 0); ADLER32_DO8(buf, 8); -#define ADLER32_CHECK_SKIPPED(by) \ - do { \ - offset = buf - start; \ - while (*skip_list != 0 && *skip_list < offset) { \ - skip_list++; \ - } \ - skipped = *skip_list != 0 && *skip_list < (offset + (by)); \ - } while (0) - -unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len, uint32_t *skip_list) + +unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len) { - unsigned char *start = buf; unsigned int s1 = checksum & 0xffff; unsigned int s2 = (checksum >> 16) & 0xffff; unsigned char *end; - bool skipped; - uint32_t offset; while (len >= ADLER32_NMAX) { len -= ADLER32_NMAX; end = buf + ADLER32_NMAX; - skipped = false; do { - ADLER32_CHECK_SKIPPED(16); - if (!skipped) { - ADLER32_DO16(buf); - } + ADLER32_DO16(buf); buf += 16; } while (buf != end); s1 %= ADLER32_BASE; @@ -336,27 +321,16 @@ unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t le if (len >= 16) { end = buf + (len & 0xfff0); len &= 0xf; - skipped = false; do { - ADLER32_CHECK_SKIPPED(16); - if (!skipped) { - ADLER32_DO16(buf); - } + ADLER32_DO16(buf); buf += 16; } while (buf != end); } if (len) { end = buf + len; - skipped = false; do { - ADLER32_CHECK_SKIPPED(1); - if (!skipped) { - ADLER32_DO1(buf); - buf++; - } else { - // skip_list contains a list of pointers so we skip the entire pointer size - buf += sizeof(void*); - } + ADLER32_DO1(buf); + buf++; } while (buf != end); } s1 %= ADLER32_BASE; @@ -372,20 +346,43 @@ unsigned int zend_accel_script_checksum(zend_persistent_script *persistent_scrip size_t size = persistent_script->size; size_t persistent_script_check_block_size = ((char *)&(persistent_script->dynamic_members)) - (char *)persistent_script; unsigned int checksum = ADLER32_INIT; - uint32_t zero = 0; if (mem < (unsigned char*)persistent_script) { - checksum = zend_adler32(checksum, mem, (unsigned char*)persistent_script - mem, &zero); + checksum = zend_adler32(checksum, mem, (unsigned char*)persistent_script - mem); size -= (unsigned char*)persistent_script - mem; mem += (unsigned char*)persistent_script - mem; } - checksum = zend_adler32(checksum, mem, persistent_script_check_block_size, &zero); - mem += ZEND_ALIGNED_SIZE(sizeof(*persistent_script)); - size -= ZEND_ALIGNED_SIZE(sizeof(*persistent_script)); + checksum = zend_adler32(checksum, mem, persistent_script_check_block_size); + mem += sizeof(*persistent_script); + size -= sizeof(*persistent_script); - if (size > 0) { - checksum = zend_adler32(checksum, mem, size, persistent_script->checksum_skip_list); + if (UNEXPECTED(size == 0)) { + return checksum; } + + const zend_accel_skip_list_entry *skip_list_entry = persistent_script->mem_checksum_skip_list; + + size_t offset = 0; + uint32_t next_stop_read; + while ((next_stop_read = skip_list_entry->offset) != 0) { + checksum = zend_adler32(checksum, mem + offset, next_stop_read - offset); + offset = next_stop_read + sizeof(void *); /* skip over the pointer */ + + uint32_t repetitions = skip_list_entry->repetitions; + const uint32_t checked_area_size = skip_list_entry->checked_area_size; + while (repetitions != 0) { + checksum = zend_adler32(checksum, mem + offset, checked_area_size); + offset += checked_area_size + sizeof(void *); /* skip over the pointer */ + repetitions--; + } + + skip_list_entry++; + } + + if (size > offset) { + checksum = zend_adler32(checksum, mem + offset, size - offset); + } + return checksum; } diff --git a/ext/opcache/zend_accelerator_util_funcs.h b/ext/opcache/zend_accelerator_util_funcs.h index febac273d52d1..88362436026d0 100644 --- a/ext/opcache/zend_accelerator_util_funcs.h +++ b/ext/opcache/zend_accelerator_util_funcs.h @@ -35,7 +35,7 @@ zend_op_array* zend_accel_load_script(zend_persistent_script *persistent_script, #define ADLER32_INIT 1 /* initial Adler-32 value */ -unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len, uint32_t *skip_list); +unsigned int zend_adler32(unsigned int checksum, unsigned char *buf, uint32_t len); unsigned int zend_accel_script_checksum(zend_persistent_script *persistent_script); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index ee6ab9b2b61ae..22ccc4d84305d 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1030,11 +1030,8 @@ int zend_file_cache_script_store(zend_persistent_script *script, int in_shm) } zend_shared_alloc_destroy_xlat_table(); - zend_string *const s = (zend_string*)ZCG(mem); - - uint32_t zero = 0; - info.checksum = zend_adler32(ADLER32_INIT, buf, script->size, &zero); - info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL(s), info.str_size, &zero); + info.checksum = zend_adler32(ADLER32_INIT, buf, script->size); + info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL((zend_string*)ZCG(mem)), info.str_size); #if __has_feature(memory_sanitizer) /* The buffer may contain uninitialized regions. However, the uninitialized parts will not be @@ -1778,9 +1775,8 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl close(fd); /* verify checksum */ - uint32_t zero = 0; if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size, &zero)) != info.checksum) { + (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); zend_file_cache_unlink(filename); zend_arena_release(&CG(arena), checkpoint); diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 970cab91dc180..816cb0b2c4271 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -29,6 +29,7 @@ #include "zend_operators.h" #include "zend_interfaces.h" #include "zend_attributes.h" +#include "zend_sort.h" #ifdef HAVE_JIT # include "Optimizer/zend_func_info.h" @@ -87,57 +88,43 @@ static void zend_persist_op_array(zval *zv); static const uint32_t uninitialized_bucket[-HT_MIN_MASK] = {HT_INVALID_IDX, HT_INVALID_IDX}; -static void checksum_skip_list_init(void) +static void mem_checksum_skip_list_add(void *p, uint32_t checked_area_size, uint32_t repetitions) { - ZEND_ASSERT(ZCG(checksum_skip_list) == NULL); - ZCG(checksum_skip_list_count) = 0; - ZCG(checksum_skip_list_capacity) = 8; - ZCG(checksum_skip_list) = safe_emalloc(ZCG(checksum_skip_list_capacity), sizeof(uint32_t), 0); + ZEND_ASSERT(ZCG(mem_checksum_skip_list) != NULL); + char *base_ptr = (char *) ZCG(current_persistent_script)->mem + sizeof(zend_persistent_script); + ZEND_ASSERT((char *) p >= base_ptr); + zend_accel_skip_list_entry *entry = &ZCG(mem_checksum_skip_list)[ZCG(mem_checksum_skip_list_count)++]; + entry->offset = (char *) p - base_ptr; + entry->checked_area_size = checked_area_size; + entry->repetitions = repetitions; } -static void checksum_skip_list_extend(uint32_t size) +static int mem_checksum_skip_list_compare(const zend_accel_skip_list_entry *l, const zend_accel_skip_list_entry *r) { - ZEND_ASSERT(ZCG(checksum_skip_list) != NULL); - ZCG(checksum_skip_list) = safe_erealloc(ZCG(checksum_skip_list), ZCG(checksum_skip_list_capacity), sizeof(uint32_t), 0); -} - -void checksum_skip_list_add(void *p) -{ - ZEND_ASSERT(ZCG(checksum_skip_list) != NULL); - void *base_btr = ((char*)ZCG(current_persistent_script)->mem) + ZEND_ALIGNED_SIZE(sizeof(zend_persistent_script)); - ZEND_ASSERT(p >= base_btr); - if ((ZCG(checksum_skip_list_count) + 1) >= ZCG(checksum_skip_list_capacity)) { - ZCG(checksum_skip_list_capacity) *= 2; - checksum_skip_list_extend(ZCG(checksum_skip_list_capacity)); - } - ZCG(checksum_skip_list)[ZCG(checksum_skip_list_count)++] = p - base_btr; -} - -static int checksum_skip_list_compare(const uint32_t *l, const uint32_t *r) -{ - if (*l < *r) { + if (l->offset < r->offset) { return -1; - } else if (*l == *r) { + } else if (l->offset == r->offset) { return 0; } else { return 1; } } -static void checksum_skip_list_swap(uint32_t *l, uint32_t *r) +static void mem_checksum_skip_list_swap(zend_accel_skip_list_entry *l, zend_accel_skip_list_entry *r) { - uint32_t tmp = *r; + zend_accel_skip_list_entry tmp = *r; *r = *l; *l = tmp; } -static uint32_t *checksum_skip_list_persist(void) +static zend_accel_skip_list_entry *mem_checksum_skip_list_persist(void) { - zend_sort((void *) ZCG(checksum_skip_list), ZCG(checksum_skip_list_count), sizeof(*ZCG(checksum_skip_list)), (compare_func_t) checksum_skip_list_compare, (swap_func_t) checksum_skip_list_swap); - uint32_t *result = zend_shared_memdup(ZCG(checksum_skip_list), (ZCG(checksum_skip_list_count) + 1) * sizeof(uint32_t)); - result[ZCG(checksum_skip_list_count)] = 0; - efree(ZCG(checksum_skip_list)); - ZCG(checksum_skip_list) = NULL; + zend_sort(ZCG(mem_checksum_skip_list), ZCG(mem_checksum_skip_list_count), sizeof(zend_accel_skip_list_entry), (compare_func_t) mem_checksum_skip_list_compare, (swap_func_t) mem_checksum_skip_list_swap); + zend_accel_skip_list_entry *result = zend_shared_memdup(ZCG(mem_checksum_skip_list), (ZCG(mem_checksum_skip_list_count) + 1) * sizeof(zend_accel_skip_list_entry)); + zend_accel_skip_list_entry *last_sentinel = &result[ZCG(mem_checksum_skip_list_count)]; + last_sentinel->offset = 0; + last_sentinel->checked_area_size = 0; + last_sentinel->repetitions = 0; return result; } @@ -581,6 +568,11 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc zend_op *end = new_opcodes + op_array->last; int offset = 0; + if (ZCG(mem_checksum_skip_list)) { + /* There is already one skip with 0 repetitions, so we have to subtract one */ + mem_checksum_skip_list_add(new_opcodes, sizeof(zend_op) - sizeof(op_array->opcodes[0].handler), op_array->last - 1); + } + for (; opline < end ; opline++, offset++) { #if ZEND_USE_ABS_CONST_ADDR if (opline->op1_type == IS_CONST) { @@ -814,6 +806,10 @@ static void zend_persist_class_method(zval *zv, zend_class_entry *ce) return; } op_array = Z_PTR_P(zv) = zend_shared_memdup_put(op_array, sizeof(zend_op_array)); + if (ZCG(mem_checksum_skip_list)) { + /* There is already one skip with 0 repetitions, so we have to subtract one */ + mem_checksum_skip_list_add(&op_array->reserved, 0, sizeof(op_array->reserved) / sizeof(op_array->reserved[0]) - 1); + } zend_persist_op_array_ex(op_array, NULL); if (ce->ce_flags & ZEND_ACC_IMMUTABLE) { op_array->fn_flags |= ZEND_ACC_IMMUTABLE; @@ -920,8 +916,8 @@ zend_class_entry *zend_persist_class_entry(zend_class_entry *orig_ce) ce->ce_flags |= ZEND_ACC_FILE_CACHED; } ce->inheritance_cache = NULL; - if (ZCG(checksum_skip_list) != NULL) { - checksum_skip_list_add(&ce->inheritance_cache); + if (ZCG(mem_checksum_skip_list)) { + mem_checksum_skip_list_add(&ce->inheritance_cache, 0, 0); } if (!(ce->ce_flags & ZEND_ACC_CACHED)) { @@ -1339,6 +1335,10 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script { Bucket *p; + /* The skip list count is still set by the persist_calc routines, which always precedes a call to this function. */ + ZCG(mem_checksum_skip_list) = safe_emalloc(ZCG(mem_checksum_skip_list_count), sizeof(zend_accel_skip_list_entry), 0); + ZCG(mem_checksum_skip_list_count) = 0; + script->mem = ZCG(mem); ZEND_ASSERT(((zend_uintptr_t)ZCG(mem) & 0x7) == 0); /* should be 8 byte aligned */ @@ -1347,8 +1347,6 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script script->corrupted = 0; ZCG(current_persistent_script) = script; - checksum_skip_list_init(); - if (!for_shm) { /* script is not going to be saved in SHM */ script->corrupted = 1; @@ -1405,7 +1403,9 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script } #endif - script->checksum_skip_list = checksum_skip_list_persist(); + script->mem_checksum_skip_list = mem_checksum_skip_list_persist(); + efree(ZCG(mem_checksum_skip_list)); + ZCG(mem_checksum_skip_list) = NULL; script->corrupted = false; ZCG(current_persistent_script) = NULL; diff --git a/ext/opcache/zend_persist.h b/ext/opcache/zend_persist.h index 4d73cb5ec185c..4c7d6f2eeee15 100644 --- a/ext/opcache/zend_persist.h +++ b/ext/opcache/zend_persist.h @@ -30,6 +30,5 @@ 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); -void checksum_skip_list_add(void *p); #endif /* ZEND_PERSIST_H */ diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 327821e27f966..ae58fbf65da17 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -27,11 +27,6 @@ #include "zend_operators.h" #include "zend_attributes.h" -#ifdef HAVE_JIT -# include "Optimizer/zend_func_info.h" -# include "jit/zend_jit.h" -#endif - #define ADD_DUP_SIZE(m,s) ZCG(current_persistent_script)->size += zend_shared_memdup_size((void*)m, s) #define ADD_SIZE(m) ZCG(current_persistent_script)->size += ZEND_ALIGNED_SIZE(m) @@ -241,6 +236,7 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array) zend_shared_alloc_register_xlat_entry(op_array->opcodes, op_array->opcodes); ADD_SIZE(sizeof(zend_op) * op_array->last); + ZCG(mem_checksum_skip_list_count)++; if (op_array->filename) { ADD_STRING(op_array->filename); @@ -302,36 +298,6 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array) } ADD_SIZE(ZEND_ALIGNED_SIZE(zend_extensions_op_array_persist_calc(op_array))); - -#ifdef HAVE_JIT - if (JIT_G(on)) { - if (JIT_G(hot_loop)) { - zend_cfg cfg; - - if (zend_jit_build_cfg(op_array, &cfg) == SUCCESS) { - for (uint32_t i = 0; i < cfg.blocks_count; i++) { - if (cfg.blocks[i].flags & ZEND_BB_REACHABLE) { - if (cfg.blocks[i].flags & ZEND_BB_LOOP_HEADER) { - // opcode.handler - ZCG(checksum_skip_list_count)++; - } - } - } - } - } - - if (JIT_G(hot_func)) { - zend_op *opline = op_array->opcodes; - if (!(op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS)) { - while (opline->opcode == ZEND_RECV || opline->opcode == ZEND_RECV_INIT) { - opline++; - } - } - // opcode.handler - ZCG(checksum_skip_list_count)++; - } - } -#endif } static void zend_persist_op_array_calc(zval *zv) @@ -375,6 +341,7 @@ static void zend_persist_class_method_calc(zval *zv) if (!old_op_array) { ADD_SIZE(sizeof(zend_op_array)); zend_persist_op_array_calc_ex(Z_PTR_P(zv)); + ZCG(mem_checksum_skip_list_count)++; zend_shared_alloc_register_xlat_entry(op_array, Z_PTR_P(zv)); } else { /* If op_array is shared, the function name refcount is still incremented for each use, @@ -434,10 +401,7 @@ void zend_persist_class_entry_calc(zend_class_entry *ce) ADD_SIZE(sizeof(zend_class_entry)); - if (ZCG(current_persistent_script)) { - // inheritance_cache - ZCG(checksum_skip_list_count)++; - } + ZCG(mem_checksum_skip_list_count)++; if (!(ce->ce_flags & ZEND_ACC_CACHED)) { ADD_INTERNED_STRING(ce->name); @@ -612,7 +576,8 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s { Bucket *p; - ZCG(checksum_skip_list_count) = 0; + /* Account for the final zero element that acts as a terminator. */ + ZCG(mem_checksum_skip_list_count) = 1; new_persistent_script->mem = NULL; new_persistent_script->size = 0; @@ -649,10 +614,7 @@ uint32_t zend_accel_script_persist_calc(zend_persistent_script *new_persistent_s zend_persist_warnings_calc( new_persistent_script->num_warnings, new_persistent_script->warnings); - // null terminator - ZCG(checksum_skip_list_count)++; - ADD_SIZE(ZCG(checksum_skip_list_count) * sizeof(uint32_t)); - ZCG(checksum_skip_list_count) = 0; + ADD_SIZE(ZCG(mem_checksum_skip_list_count) * sizeof(zend_accel_skip_list_entry)); new_persistent_script->corrupted = false; From b5c8b918111055f8ca58eca7423a6e432e309c1c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Feb 2023 13:43:25 +0100 Subject: [PATCH 3/6] Add capacity check (only in debug mode) --- ext/opcache/ZendAccelerator.c | 3 +++ ext/opcache/ZendAccelerator.h | 7 +++++++ ext/opcache/zend_persist.c | 6 ++++++ 3 files changed, 16 insertions(+) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index d8eb8c4225c4f..52804217221ac 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4765,6 +4765,9 @@ static int accel_finish_startup(void) ZCG(mem_checksum_skip_list) = NULL; ZCG(mem_checksum_skip_list_count) = 0; +#if ZEND_DEBUG + ZCG(mem_checksum_skip_list_capacity) = 0; +#endif if (accel_preload(ZCG(accel_directives).preload, in_child) != SUCCESS) { ret = FAILURE; diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 08dda07a800d8..47608f46d1c0a 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -245,6 +245,13 @@ typedef struct _zend_accel_globals { zend_accel_skip_list_entry *mem_checksum_skip_list; uint32_t mem_checksum_skip_list_count; + /* We don't actually need the capacity because the skip list is preallocated to the right size. + * We only use this in a debug build to check for bugs. If an assertion failure would ever trigger + * for this field, then it's signifies a bug in the persistence code because the calculation and + * actual size don't match. */ +#if ZEND_DEBUG + uint32_t mem_checksum_skip_list_capacity; +#endif } zend_accel_globals; typedef struct _zend_string_table { diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 816cb0b2c4271..cb10d213024b1 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -93,6 +93,9 @@ static void mem_checksum_skip_list_add(void *p, uint32_t checked_area_size, uint ZEND_ASSERT(ZCG(mem_checksum_skip_list) != NULL); char *base_ptr = (char *) ZCG(current_persistent_script)->mem + sizeof(zend_persistent_script); ZEND_ASSERT((char *) p >= base_ptr); +#if ZEND_DEBUG + ZEND_ASSERT(ZCG(mem_checksum_skip_list_count) < ZCG(mem_checksum_skip_list_capacity)); +#endif zend_accel_skip_list_entry *entry = &ZCG(mem_checksum_skip_list)[ZCG(mem_checksum_skip_list_count)++]; entry->offset = (char *) p - base_ptr; entry->checked_area_size = checked_area_size; @@ -1337,6 +1340,9 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script /* The skip list count is still set by the persist_calc routines, which always precedes a call to this function. */ ZCG(mem_checksum_skip_list) = safe_emalloc(ZCG(mem_checksum_skip_list_count), sizeof(zend_accel_skip_list_entry), 0); +#if ZEND_DEBUG + ZCG(mem_checksum_skip_list_capacity) = ZCG(mem_checksum_skip_list_count); +#endif ZCG(mem_checksum_skip_list_count) = 0; script->mem = ZCG(mem); From f3027d583383122e3caa148aeccbd68a015029f7 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Feb 2023 13:49:24 +0100 Subject: [PATCH 4/6] Only add opcode handlers to the skip list if the JIT is compiled in and on The JIT is the only place where the handlers are overwritten. --- ext/opcache/zend_persist.c | 4 +++- ext/opcache/zend_persist_calc.c | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index cb10d213024b1..8c3e793fdbe3b 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -571,10 +571,12 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc zend_op *end = new_opcodes + op_array->last; int offset = 0; - if (ZCG(mem_checksum_skip_list)) { +#ifdef HAVE_JIT + if (ZCG(mem_checksum_skip_list) && JIT_G(on)) { /* There is already one skip with 0 repetitions, so we have to subtract one */ mem_checksum_skip_list_add(new_opcodes, sizeof(zend_op) - sizeof(op_array->opcodes[0].handler), op_array->last - 1); } +#endif for (; opline < end ; opline++, offset++) { #if ZEND_USE_ABS_CONST_ADDR diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index ae58fbf65da17..071cfa4f227d5 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -27,6 +27,10 @@ #include "zend_operators.h" #include "zend_attributes.h" +#ifdef HAVE_JIT +#include "jit/zend_jit.h" +#endif + #define ADD_DUP_SIZE(m,s) ZCG(current_persistent_script)->size += zend_shared_memdup_size((void*)m, s) #define ADD_SIZE(m) ZCG(current_persistent_script)->size += ZEND_ALIGNED_SIZE(m) @@ -236,7 +240,11 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array) zend_shared_alloc_register_xlat_entry(op_array->opcodes, op_array->opcodes); ADD_SIZE(sizeof(zend_op) * op_array->last); - ZCG(mem_checksum_skip_list_count)++; +#ifdef HAVE_JIT + if (JIT_G(on)) { + ZCG(mem_checksum_skip_list_count)++; + } +#endif if (op_array->filename) { ADD_STRING(op_array->filename); From bb68202caac5a4dc96e3f07547ef50abc401c8d5 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 1 Mar 2023 21:08:04 +0100 Subject: [PATCH 5/6] Use size_t instead of uint32_t for the offset --- ext/opcache/ZendAccelerator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 47608f46d1c0a..aecc8b6ef93ef 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -112,7 +112,7 @@ typedef enum _zend_accel_restart_reason { typedef struct _zend_accel_skip_list_entry { /* The offset indicates which byte offset within the memory block must be skipped. * The size of the skip is equal to the system's pointer size. */ - uint32_t offset; + size_t offset; /* To prevent creating a huge list with a lot of entries, we use a compression scheme * based on the following two fields. If repetitions > 0, then the checksum algorithm * will repeat `repetitions` times checksumming `checked_area_size` bytes, followed From ef4d15e20bd1150f90cdb9c9433584f617c3c989 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 1 Mar 2023 21:08:16 +0100 Subject: [PATCH 6/6] Already subtract a pointer size from the checked area --- ext/opcache/zend_persist.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 8c3e793fdbe3b..fa5979b7c6ced 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -98,7 +98,7 @@ static void mem_checksum_skip_list_add(void *p, uint32_t checked_area_size, uint #endif zend_accel_skip_list_entry *entry = &ZCG(mem_checksum_skip_list)[ZCG(mem_checksum_skip_list_count)++]; entry->offset = (char *) p - base_ptr; - entry->checked_area_size = checked_area_size; + entry->checked_area_size = checked_area_size - sizeof(char*); entry->repetitions = repetitions; } @@ -574,7 +574,7 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc #ifdef HAVE_JIT if (ZCG(mem_checksum_skip_list) && JIT_G(on)) { /* There is already one skip with 0 repetitions, so we have to subtract one */ - mem_checksum_skip_list_add(new_opcodes, sizeof(zend_op) - sizeof(op_array->opcodes[0].handler), op_array->last - 1); + mem_checksum_skip_list_add(new_opcodes, sizeof(zend_op), op_array->last - 1); } #endif @@ -813,7 +813,7 @@ static void zend_persist_class_method(zval *zv, zend_class_entry *ce) op_array = Z_PTR_P(zv) = zend_shared_memdup_put(op_array, sizeof(zend_op_array)); if (ZCG(mem_checksum_skip_list)) { /* There is already one skip with 0 repetitions, so we have to subtract one */ - mem_checksum_skip_list_add(&op_array->reserved, 0, sizeof(op_array->reserved) / sizeof(op_array->reserved[0]) - 1); + mem_checksum_skip_list_add(&op_array->reserved, sizeof(char*), sizeof(op_array->reserved) / sizeof(op_array->reserved[0]) - 1); } zend_persist_op_array_ex(op_array, NULL); if (ce->ce_flags & ZEND_ACC_IMMUTABLE) {