diff --git a/CMakeLists.txt b/CMakeLists.txt index a1cf41348..8ff15718f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -877,7 +877,7 @@ if (MI_BUILD_TESTS) enable_testing() # static link tests - set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff) + set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff free-null-preinit) if(NOT (MI_DEBUG_TSAN OR MI_TRACK_ASAN OR MI_DEBUG_UBSAN)) list(APPEND mi_static_tests thp-optout) # counts madvise calls by interposing it, which a sanitizer runtime does first endif() diff --git a/src/page-map.c b/src/page-map.c index b4ee0189a..e977eeb12 100644 --- a/src/page-map.c +++ b/src/page-map.c @@ -215,12 +215,19 @@ mi_decl_nodiscard mi_decl_export bool mi_is_in_heap_region(const void* p) mi_att #define MI_PAGE_MAP_SUB_SIZE (MI_PAGE_MAP_SUB_COUNT * sizeof(mi_page_t*)) // Use an initial empty page map so `free(NULL)` works even if mimalloc is not yet initialized (issue #1341) +// The sub-map for the NULL address must itself be a valid (zero initialized) array rather than +// NULL: the default (non-secure, non-debug) build resolves pointers via `_mi_unchecked_ptr_page`, +// which loads `submaps[idx][sub_idx]` without checking the sub-map for NULL. A single entry is +// enough since `_mi_page_map_index(NULL)` is 0 at both levels, and no other pointer can belong +// to mimalloc before it is initialized (any `malloc` initializes the page map first). +static mi_page_t* mi_page_map_empty_sub[1] = { NULL }; // so that `_mi_ptr_page(NULL) == NULL` + static mi_page_map_t mi_page_map_empty = { MI_ATOMIC_VAR_INIT(1), sizeof(mi_page_map_t), MI_MEMID_STATIC, MI_LOCK_INITIALIZER, - { MI_ATOMIC_VAR_INIT(NULL) } + { MI_ATOMIC_VAR_INIT(mi_page_map_empty_sub) } }; mi_decl_hidden mi_decl_cache_align _Atomic(mi_page_map_t*) __mi_page_map = MI_ATOMIC_VAR_INIT(&mi_page_map_empty); diff --git a/test/test-free-null-preinit.c b/test/test-free-null-preinit.c new file mode 100644 index 000000000..2581f8f3f --- /dev/null +++ b/test/test-free-null-preinit.c @@ -0,0 +1,44 @@ +// `free(NULL)` must be a no-op even before mimalloc is initialized (issue #1341). +// +// This is not hypothetical: glibc >= 2.44 calls `free(NULL)` unconditionally at the end of +// `newlocale()`, and the C++ runtime calls `newlocale()` from a static initializer while +// constructing `std::locale::classic()`. With a global malloc override (MI_OVERRIDE) that +// `free` lands in mimalloc before any allocation has happened, so before the page map exists. +// +// The 2-level page map resolves a pointer as `submaps[idx][sub_idx]`; for NULL both indices +// are 0. The default (non-secure, non-debug) build uses `_mi_unchecked_ptr_page`, which does +// not test the sub-map for NULL -- so the static "empty" page map must carry a real (zeroed) +// sub-map at entry 0 rather than a NULL one, or this dereferences address 0. +#include +#include +#include + +static int early_ran = 0; + +#if defined(__GNUC__) || defined(__clang__) +// priority 101 runs before mimalloc's own (default priority) constructor +__attribute__((constructor(101))) +static void free_null_before_init(void) { + mi_free(NULL); // goes to mimalloc whether or not the malloc override is enabled + free(NULL); // and via the override, when it is + early_ran = 1; +} +#endif + +int main(void) { + int failures = 0; + + // and once more now that mimalloc is certainly initialized + mi_free(NULL); + free(NULL); + +#if defined(__GNUC__) || defined(__clang__) + if (!early_ran) { + printf(" FAIL: the early constructor did not run\n"); + failures++; + } +#endif + + printf(failures == 0 ? "ok\n" : "failed\n"); + return (failures == 0 ? 0 : 1); +}