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 <jam...@microsoft.com>
---
cache.h | 6 ++++++
git.c | 3 +++
mem-pool.c | 7 ++++++-
mem-pool.h | 2 +-
read-cache.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 7aae9c8db0..2916e953ad 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,12 @@ extern struct cache_entry
*make_empty_transient_cache_entry(size_t name_len);
*/
void discard_cache_entry(struct cache_entry *ce);
+/*
+ * Check configuration if we should perform extra validation on cache
+ * entries.
+ */
+int should_validate_cache_entries(void);
+
/*
* Duplicate a cache_entry. Allocate memory for the new entry from a
* memory_pool. Takes into account cache_entry fields that are meant
diff --git a/git.c b/git.c
index bab6bbfded..7ce65eab78 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 cc7d3a7ab1..6770b4f740 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -63,13 +63,18 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
initial_size)
*mem_pool = pool;
}
-void mem_pool_discard(struct mem_pool *mem_pool)
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
{
struct mp_block *block, *block_to_free;
+
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);
}
diff --git a/mem-pool.h b/mem-pool.h
index 5c892d3bdb..68d8428902 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -29,7 +29,7 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
initial_size);
/*
* Discard a memory pool and free all the memory it is responsible for.
*/
-void mem_pool_discard(struct mem_pool *mem_pool);
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory);
/*
* Alloc memory from the mem_pool.
diff --git a/read-cache.c b/read-cache.c
index 02fe5d333c..fb2cec6ac6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2023,8 +2023,10 @@ int discard_index(struct index_state *istate)
* Cache entries in istate->cache[] should have been allocated
* from the memory pool associated with this index, or from an
* associated split_index. There is no need to free individual
- * cache entries.
+ * cache entries. validate_cache_entries can detect when this
+ * assertion does not hold.
*/
+ validate_cache_entries(istate);
resolve_undo_clear_index(istate);
istate->cache_nr = 0;
@@ -2041,13 +2043,45 @@ int discard_index(struct index_state *istate)
istate->untracked = NULL;
if (istate->ce_mem_pool) {
- mem_pool_discard(istate->ce_mem_pool);
+ mem_pool_discard(istate->ce_mem_pool,
should_validate_cache_entries());
istate->ce_mem_pool = NULL;
}
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;
+
+ if (!should_validate_cache_entries() ||!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;
@@ -2854,8 +2888,25 @@ struct cache_entry *dup_cache_entry(const struct
cache_entry *ce,
*/
void discard_cache_entry(struct cache_entry *ce)
{
+ if (ce && should_validate_cache_entries())
+ memset(ce, 0xCD, cache_entry_size(ce->ce_namelen));
+
if (ce && ce->mem_pool_allocated)
return;
free(ce);
}
+
+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;
+}
--
2.14.3
