Skip to content

Commit

Permalink
t-reftable-basics: stop assuming that malloc is not a constant
Browse files Browse the repository at this point in the history
As indicated by the `#undef malloc` line in `reftable/basics.h`, it is
quite common to compile in allocators other than the default one by
defining `malloc` constants and friends.

This pattern is used e.g. in Git for Windows, which uses the powerful
and performant `mimalloc` allocator.

Furthermore, in `reftable/basics.c` this `#undef malloc` is
_specifically_ disabled by virtue of defining the
`REFTABLE_ALLOW_BANNED_ALLOCATORS` constant before including
`reftable/basic.h`.

However, in 8db127d (reftable: avoid leaks on realloc error,
2024-12-28) and in 2cca185 (reftable: fix allocation count on
realloc error, 2024-12-28), `reftable_set_alloc()` function calls were
introduced that pass `malloc`, `realloc` and `free` function pointers as
parameters _after_ `reftable/basics.h` ensured that they were no longer
`#define`d.

This causes problems because those calls happen after the initial
allocator has already been used to initialize an array, which is
subsequently resized using the overridden default `realloc()` allocator.

You cannot mix and match allocators like that, which leads to a
`STATUS_HEAP_CORRUPTION` (C0000374), and when running this unit test
through shell and/or `prove` (which only support 7-bit status codes), it
surfaces as exit code 127.

It is totally unnecessary to pass those function pointers to
`malloc`/`realloc`/`free` in, though: The `reftable` code goes out of
its way to fall back to the initial allocator when passing `NULL`
parameters instead. So let's do that instead of causing heap
corruptions.

Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
dscho committed Jan 7, 2025
1 parent b866815 commit 1350d79
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions t/unit-tests/t-reftable-basics.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)

old_alloc = alloc;
old_arr = arr;
reftable_set_alloc(malloc, realloc_stub, free);
reftable_set_alloc(NULL, realloc_stub, NULL);
check(REFTABLE_ALLOC_GROW(arr, old_alloc + 1, alloc));
check(arr == old_arr);
check_uint(alloc, ==, old_alloc);

old_alloc = alloc;
reftable_set_alloc(malloc, realloc, free);
reftable_set_alloc(NULL, NULL, NULL);
check(!REFTABLE_ALLOC_GROW(arr, old_alloc + 1, alloc));
check(arr != NULL);
check_uint(alloc, >, old_alloc);
Expand All @@ -188,11 +188,11 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
arr[alloc - 1] = 42;

old_alloc = alloc;
reftable_set_alloc(malloc, realloc_stub, free);
reftable_set_alloc(NULL, realloc_stub, NULL);
REFTABLE_ALLOC_GROW_OR_NULL(arr, old_alloc + 1, alloc);
check(arr == NULL);
check_uint(alloc, ==, 0);
reftable_set_alloc(malloc, realloc, free);
reftable_set_alloc(NULL, NULL, NULL);

reftable_free(arr);
}
Expand Down

0 comments on commit 1350d79

Please sign in to comment.