Merge branch 'ly/load-bitmap-leakfix'

Leakfix with a new and a bit invasive test.

* ly/load-bitmap-leakfix:
  pack-bitmap: add load corrupt bitmap test
  pack-bitmap: reword comments in test_bitmap_commits()
  pack-bitmap: fix memory leak if load_bitmap() failed
maint
Junio C Hamano 2025-07-15 15:18:18 -07:00
commit f31d155266
4 changed files with 103 additions and 24 deletions

View File

@ -31,6 +31,7 @@ struct stored_bitmap {
struct object_id oid;
struct ewah_bitmap *root;
struct stored_bitmap *xor;
size_t map_pos;
int flags;
};

@ -314,13 +315,14 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index,
struct ewah_bitmap *root,
const struct object_id *oid,
struct stored_bitmap *xor_with,
int flags)
int flags, size_t map_pos)
{
struct stored_bitmap *stored;
khiter_t hash_pos;
int ret;

stored = xmalloc(sizeof(struct stored_bitmap));
stored->map_pos = map_pos;
stored->root = root;
stored->xor = xor_with;
stored->flags = flags;
@ -376,10 +378,12 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
struct stored_bitmap *xor_bitmap = NULL;
uint32_t commit_idx_pos;
struct object_id oid;
size_t entry_map_pos;

if (index->map_size - index->map_pos < 6)
return error(_("corrupt ewah bitmap: truncated header for entry %d"), i);

entry_map_pos = index->map_pos;
commit_idx_pos = read_be32(index->map, &index->map_pos);
xor_offset = read_u8(index->map, &index->map_pos);
flags = read_u8(index->map, &index->map_pos);
@ -402,8 +406,9 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
if (!bitmap)
return -1;

recent_bitmaps[i % MAX_XOR_OFFSET] = store_bitmap(
index, bitmap, &oid, xor_bitmap, flags);
recent_bitmaps[i % MAX_XOR_OFFSET] =
store_bitmap(index, bitmap, &oid, xor_bitmap, flags,
entry_map_pos);
}

return 0;
@ -630,41 +635,28 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
bitmap_git->ext_index.positions = kh_init_oid_pos();

if (load_reverse_index(r, bitmap_git))
goto failed;
return -1;

if (!(bitmap_git->commits = read_bitmap_1(bitmap_git)) ||
!(bitmap_git->trees = read_bitmap_1(bitmap_git)) ||
!(bitmap_git->blobs = read_bitmap_1(bitmap_git)) ||
!(bitmap_git->tags = read_bitmap_1(bitmap_git)))
goto failed;
return -1;

if (!bitmap_git->table_lookup && load_bitmap_entries_v1(bitmap_git) < 0)
goto failed;
return -1;

if (bitmap_git->base) {
if (!bitmap_is_midx(bitmap_git))
BUG("non-MIDX bitmap has non-NULL base bitmap index");
if (load_bitmap(r, bitmap_git->base, 1) < 0)
goto failed;
return -1;
}

if (!recursing)
load_all_type_bitmaps(bitmap_git);

return 0;

failed:
munmap(bitmap_git->map, bitmap_git->map_size);
bitmap_git->map = NULL;
bitmap_git->map_size = 0;

kh_destroy_oid_map(bitmap_git->bitmaps);
bitmap_git->bitmaps = NULL;

kh_destroy_oid_pos(bitmap_git->ext_index.positions);
bitmap_git->ext_index.positions = NULL;

return -1;
}

static int open_pack_bitmap(struct repository *r,
@ -882,6 +874,7 @@ static struct stored_bitmap *lazy_bitmap_for_commit(struct bitmap_index *bitmap_
int xor_flags;
khiter_t hash_pos;
struct bitmap_lookup_table_xor_item *xor_item;
size_t entry_map_pos;

if (is_corrupt)
return NULL;
@ -941,6 +934,7 @@ static struct stored_bitmap *lazy_bitmap_for_commit(struct bitmap_index *bitmap_
goto corrupt;
}

entry_map_pos = bitmap_git->map_pos;
bitmap_git->map_pos += sizeof(uint32_t) + sizeof(uint8_t);
xor_flags = read_u8(bitmap_git->map, &bitmap_git->map_pos);
bitmap = read_bitmap_1(bitmap_git);
@ -948,7 +942,8 @@ static struct stored_bitmap *lazy_bitmap_for_commit(struct bitmap_index *bitmap_
if (!bitmap)
goto corrupt;

xor_bitmap = store_bitmap(bitmap_git, bitmap, &xor_item->oid, xor_bitmap, xor_flags);
xor_bitmap = store_bitmap(bitmap_git, bitmap, &xor_item->oid,
xor_bitmap, xor_flags, entry_map_pos);
xor_items_nr--;
}

@ -982,6 +977,7 @@ static struct stored_bitmap *lazy_bitmap_for_commit(struct bitmap_index *bitmap_
* Instead, we can skip ahead and immediately read the flags and
* ewah bitmap.
*/
entry_map_pos = bitmap_git->map_pos;
bitmap_git->map_pos += sizeof(uint32_t) + sizeof(uint8_t);
flags = read_u8(bitmap_git->map, &bitmap_git->map_pos);
bitmap = read_bitmap_1(bitmap_git);
@ -989,7 +985,8 @@ static struct stored_bitmap *lazy_bitmap_for_commit(struct bitmap_index *bitmap_
if (!bitmap)
goto corrupt;

return store_bitmap(bitmap_git, bitmap, oid, xor_bitmap, flags);
return store_bitmap(bitmap_git, bitmap, oid, xor_bitmap, flags,
entry_map_pos);

corrupt:
free(xor_items);
@ -2852,8 +2849,9 @@ int test_bitmap_commits(struct repository *r)
die(_("failed to load bitmap indexes"));

/*
* As this function is only used to print bitmap selected
* commits, we don't have to read the commit table.
* Since this function needs to print the bitmapped
* commits, bypass the commit lookup table (if one exists)
* by forcing the bitmap to eagerly load its entries.
*/
if (bitmap_git->table_lookup) {
if (load_bitmap_entries_v1(bitmap_git) < 0)
@ -2869,6 +2867,48 @@ int test_bitmap_commits(struct repository *r)
return 0;
}

int test_bitmap_commits_with_offset(struct repository *r)
{
struct object_id oid;
struct stored_bitmap *stored;
struct bitmap_index *bitmap_git;
size_t commit_idx_pos_map_pos, xor_offset_map_pos, flag_map_pos,
ewah_bitmap_map_pos;

bitmap_git = prepare_bitmap_git(r);
if (!bitmap_git)
die(_("failed to load bitmap indexes"));

/*
* Since this function needs to know the position of each individual
* bitmap, bypass the commit lookup table (if one exists) by forcing
* the bitmap to eagerly load its entries.
*/
if (bitmap_git->table_lookup) {
if (load_bitmap_entries_v1(bitmap_git) < 0)
die(_("failed to load bitmap indexes"));
}

kh_foreach (bitmap_git->bitmaps, oid, stored, {
commit_idx_pos_map_pos = stored->map_pos;
xor_offset_map_pos = stored->map_pos + sizeof(uint32_t);
flag_map_pos = xor_offset_map_pos + sizeof(uint8_t);
ewah_bitmap_map_pos = flag_map_pos + sizeof(uint8_t);

printf_ln("%s %"PRIuMAX" %"PRIuMAX" %"PRIuMAX" %"PRIuMAX,
oid_to_hex(&oid),
(uintmax_t)commit_idx_pos_map_pos,
(uintmax_t)xor_offset_map_pos,
(uintmax_t)flag_map_pos,
(uintmax_t)ewah_bitmap_map_pos);
})
;

free_bitmap_index(bitmap_git);

return 0;
}

int test_bitmap_hashes(struct repository *r)
{
struct bitmap_index *bitmap_git = prepare_bitmap_git(r);

View File

@ -81,6 +81,7 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
show_reachable_fn show_reachable);
void test_bitmap_walk(struct rev_info *revs);
int test_bitmap_commits(struct repository *r);
int test_bitmap_commits_with_offset(struct repository *r);
int test_bitmap_hashes(struct repository *r);
int test_bitmap_pseudo_merges(struct repository *r);
int test_bitmap_pseudo_merge_commits(struct repository *r, uint32_t n);

View File

@ -10,6 +10,11 @@ static int bitmap_list_commits(void)
return test_bitmap_commits(the_repository);
}

static int bitmap_list_commits_with_offset(void)
{
return test_bitmap_commits_with_offset(the_repository);
}

static int bitmap_dump_hashes(void)
{
return test_bitmap_hashes(the_repository);
@ -36,6 +41,8 @@ int cmd__bitmap(int argc, const char **argv)

if (argc == 2 && !strcmp(argv[1], "list-commits"))
return bitmap_list_commits();
if (argc == 2 && !strcmp(argv[1], "list-commits-with-offset"))
return bitmap_list_commits_with_offset();
if (argc == 2 && !strcmp(argv[1], "dump-hashes"))
return bitmap_dump_hashes();
if (argc == 2 && !strcmp(argv[1], "dump-pseudo-merges"))
@ -46,6 +53,7 @@ int cmd__bitmap(int argc, const char **argv)
return bitmap_dump_pseudo_merge_objects(atoi(argv[2]));

usage("\ttest-tool bitmap list-commits\n"
"\ttest-tool bitmap list-commits-with-offset\n"
"\ttest-tool bitmap dump-hashes\n"
"\ttest-tool bitmap dump-pseudo-merges\n"
"\ttest-tool bitmap dump-pseudo-merge-commits <n>\n"

View File

@ -495,6 +495,36 @@ test_bitmap_cases () {
grep "ignoring extra bitmap" trace2.txt
)
'

test_expect_success 'load corrupt bitmap' '
rm -fr repo &&
git init repo &&
test_when_finished "rm -fr repo" &&
(
cd repo &&
git config pack.writeBitmapLookupTable '"$writeLookupTable"' &&

test_commit base &&

git repack -adb &&
bitmap="$(ls .git/objects/pack/pack-*.bitmap)" &&
chmod +w $bitmap &&

test-tool bitmap list-commits-with-offset >offsets &&
xor_off=$(head -n1 offsets | awk "{print \$3}") &&
printf '\161' |
dd of=$bitmap count=1 bs=1 conv=notrunc seek=$xor_off &&

git rev-list --objects --no-object-names HEAD >expect.raw &&
git rev-list --objects --use-bitmap-index --no-object-names HEAD \
>actual.raw &&

sort expect.raw >expect &&
sort actual.raw >actual &&

test_cmp expect actual
)
'
}

test_bitmap_cases