Add an option (controlled by an environment variable) perform extra validations on mem_pool allocated cache entries. When set: 1) Invalidate cache_entry memory when discarding cache_entry. 2) When discarding index_state struct, verify that all cache_entries were allocated from expected mem_pool. 3) When discarding mem_pools, invalidate mem_pool memory. This should provide extra checks that mem_pools and their allocated cache_entries are being used as expected. Signed-off-by: Jameson Miller <jamill@xxxxxxxxxxxxx> --- cache.h | 7 +++++++ git.c | 3 +++ mem-pool.c | 24 +++++++++++++++++++++++- mem-pool.h | 2 ++ read-cache.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 1 deletion(-) diff --git a/cache.h b/cache.h index 7ed68f28e0..8f10f0649b 100644 --- a/cache.h +++ b/cache.h @@ -369,6 +369,13 @@ void discard_index_cache_entry(struct cache_entry *ce); */ void discard_transient_cache_entry(struct cache_entry *ce); +/* + * Validate the cache entries in the index. This is an internal + * consistency check that the cache_entry structs are allocated from + * the expected memory pool. + */ +void validate_cache_entries(const struct index_state *istate); + #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS #define active_cache (the_index.cache) #define active_nr (the_index.cache_nr) diff --git a/git.c b/git.c index f598fae7b7..28ec7a6c4f 100644 --- a/git.c +++ b/git.c @@ -347,7 +347,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) trace_argv_printf(argv, "trace: built-in: git"); + validate_cache_entries(&the_index); status = p->fn(argc, argv, prefix); + validate_cache_entries(&the_index); + if (status) return status; diff --git a/mem-pool.c b/mem-pool.c index a495885c4b..77adb5d5b9 100644 --- a/mem-pool.c +++ b/mem-pool.c @@ -60,21 +60,43 @@ void mem_pool_discard(struct mem_pool *mem_pool) { int i; struct mp_block *block, *block_to_free; + int invalidate_memory = should_validate_cache_entries(); + for (block = mem_pool->mp_block; block;) { block_to_free = block; block = block->next_block; + + if (invalidate_memory) + memset(block_to_free->space, 0xDD, ((char *)block_to_free->end) - ((char *)block_to_free->space)); + free(block_to_free); } - for (i = 0; i < mem_pool->nr; i++) + for (i = 0; i < mem_pool->nr; i++) { + memset(mem_pool->custom[i], 0xDD, ((char *)mem_pool->custom_end[i]) - ((char *)mem_pool->custom[i])); free(mem_pool->custom[i]); + } free(mem_pool->custom); free(mem_pool->custom_end); free(mem_pool); } +int should_validate_cache_entries(void) +{ + static int validate_index_cache_entries = -1; + + if (validate_index_cache_entries < 0) { + if (getenv("GIT_TEST_VALIDATE_INDEX_CACHE_ENTRIES")) + validate_index_cache_entries = 1; + else + validate_index_cache_entries = 0; + } + + return validate_index_cache_entries; +} + void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len) { struct mp_block *p; diff --git a/mem-pool.h b/mem-pool.h index 34df4fa709..b1f9a920ba 100644 --- a/mem-pool.h +++ b/mem-pool.h @@ -63,4 +63,6 @@ void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src); */ int mem_pool_contains(struct mem_pool *mem_pool, void *mem); +int should_validate_cache_entries(void); + #endif diff --git a/read-cache.c b/read-cache.c index 01cd7fea41..e1dc9f7f33 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1270,6 +1270,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti { int pos; + validate_cache_entries(istate); + if (option & ADD_CACHE_JUST_APPEND) pos = istate->cache_nr; else { @@ -1290,6 +1292,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti istate->cache_nr - pos - 1); set_index_entry(istate, pos, ce); istate->cache_changed |= CE_ENTRY_ADDED; + + validate_cache_entries(istate); return 0; } @@ -2013,6 +2017,8 @@ int is_index_unborn(struct index_state *istate) int discard_index(struct index_state *istate) { + validate_cache_entries(istate); + resolve_undo_clear_index(istate); istate->cache_nr = 0; istate->cache_changed = 0; @@ -2035,6 +2041,43 @@ int discard_index(struct index_state *istate) return 0; } + +/* + * Validate the cache entries of this index. + * All cache entries associated with this index + * should have been allocated by the memory pool + * associated with this index, or by a referenced + * split index. + */ +void validate_cache_entries(const struct index_state *istate) +{ + int i; + int validate_index_cache_entries = should_validate_cache_entries(); + + if (!validate_index_cache_entries) + return; + + if (!istate || !istate->initialized) + return; + + for (i = 0; i < istate->cache_nr; i++) { + if (!istate) { + die("internal error: cache entry is not allocated from expected memory pool"); + } else if (!istate->ce_mem_pool || + !mem_pool_contains(istate->ce_mem_pool, istate->cache[i])) { + if (!istate->split_index || + !istate->split_index->base || + !istate->split_index->base->ce_mem_pool || + !mem_pool_contains(istate->split_index->base->ce_mem_pool, istate->cache[i])) { + die("internal error: cache entry is not allocated from expected memory pool"); + } + } + } + + if (istate->split_index) + validate_cache_entries(istate->split_index->base); +} + int unmerged_index(const struct index_state *istate) { int i; @@ -2828,6 +2871,10 @@ void move_index_extensions(struct index_state *dst, struct index_state *src) */ void discard_index_cache_entry(struct cache_entry *ce) { + int invalidate_cache_entry = should_validate_cache_entries(); + + if (ce && invalidate_cache_entry) + memset(ce, 0xCD, cache_entry_size(ce->ce_namelen)); } void discard_transient_cache_entry(struct cache_entry *ce) -- 2.14.3