Skip to content

Commit

Permalink
cache_entry: add optional validations
Browse files Browse the repository at this point in the history
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 an index_state, 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.

Invalidate cache entry memory on discard

Signed-off-by: Jameson Miller <jamill@microsoft.com>
  • Loading branch information
jamill authored and dscho committed Mar 29, 2018
1 parent ecc8c19 commit c3a2b02
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 6 deletions.
3 changes: 2 additions & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ extern struct cache_entry *make_cache_entry_from_index(struct index_state *, uns
/*
* Create an empty cache entry struct.
*
* istate: The index to associate this cache entry from.
* istate: The index whose memory pool this cache entry should be allocated from.
* len: The length to reserve for the path field of the cache entry.
*/
extern struct cache_entry *make_empty_cache_entry_from_index(struct index_state *istate, size_t len);
Expand Down Expand Up @@ -2000,5 +2000,6 @@ void safe_create_dir(const char *dir, int share);
extern int print_sha1_ellipsis(void);

void cache_entry_free(struct cache_entry *ce);
void validate_cache_entries(const struct index_state *istate);

#endif /* CACHE_H */
7 changes: 7 additions & 0 deletions git.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,14 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)

trace_argv_printf(argv, "trace: built-in: git");

/*
* Validate the state of the cache entries in the index before and
* after running the command. Validation is only performed if the
* appropriate environment variable is set.
*/
validate_cache_entries(&the_index);
exit_code = status = p->fn(argc, argv, prefix);
validate_cache_entries(&the_index);
if (status)
return status;

Expand Down
20 changes: 20 additions & 0 deletions mem-pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,36 @@ void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)
void mem_pool_discard(struct mem_pool *mem_pool)
{
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, 0xCD, ((char *)block_to_free->end) - ((char *)block_to_free->space));

free(block_to_free);
}

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;
Expand Down
2 changes: 2 additions & 0 deletions mem-pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len);
void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);
int mem_pool_contains(struct mem_pool *mem_pool, void *mem);

int should_validate_cache_entries(void);

#endif
54 changes: 49 additions & 5 deletions read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,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 {
Expand All @@ -1259,6 +1261,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
(istate->cache_nr - pos - 1) * sizeof(ce));
set_index_entry(istate, pos, ce);
istate->cache_changed |= CE_ENTRY_ADDED;

validate_cache_entries(istate);
return 0;
}

Expand Down Expand Up @@ -1974,6 +1978,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;
Expand All @@ -1995,6 +2001,44 @@ 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 ||
!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;
Expand Down Expand Up @@ -2777,12 +2821,12 @@ void move_index_extensions(struct index_state *dst, struct index_state *src)
}

/*
* Centralize logic for marking a cache entry as free.
* Indicate that a cache entry is no longer is use
*/
void cache_entry_free(struct cache_entry *ce)
{
/*
* No op for now, but keep function here in case
* we want to take advantage of it.
*/
int invalidate_cache_entry = should_validate_cache_entries();

if (ce && invalidate_cache_entry)
memset(ce, 0xCD, cache_entry_size(ce->ce_namelen));
}

0 comments on commit c3a2b02

Please sign in to comment.