From a8a53e3e85fbe8c8f997078399ee089880614ebf Mon Sep 17 00:00:00 2001 From: Daan Leijen Date: Sun, 24 Mar 2024 14:50:15 -0700 Subject: [PATCH] fix double counting of free-ing for non-thread-local free calls --- include/mimalloc/internal.h | 2 +- src/alloc-aligned.c | 2 +- src/free.c | 27 ++++++++++++++------------- src/options.c | 2 +- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/include/mimalloc/internal.h b/include/mimalloc/internal.h index 21dc9d62..29943357 100644 --- a/include/mimalloc/internal.h +++ b/include/mimalloc/internal.h @@ -200,7 +200,7 @@ void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t size, bool void* _mi_heap_malloc_zero(mi_heap_t* heap, size_t size, bool zero) mi_attr_noexcept; void* _mi_heap_malloc_zero_ex(mi_heap_t* heap, size_t size, bool zero, size_t huge_alignment) mi_attr_noexcept; // called from `_mi_heap_malloc_aligned` void* _mi_heap_realloc_zero(mi_heap_t* heap, void* p, size_t newsize, bool zero) mi_attr_noexcept; -mi_block_t* _mi_page_ptr_unalign(const mi_segment_t* segment, const mi_page_t* page, const void* p); +mi_block_t* _mi_page_ptr_unalign(const mi_page_t* page, const void* p); bool _mi_free_delayed_block(mi_block_t* block); void _mi_free_generic(mi_segment_t* segment, mi_page_t* page, bool is_local, void* p) mi_attr_noexcept; // for runtime integration void _mi_padding_shrink(const mi_page_t* page, const mi_block_t* block, const size_t min_size); diff --git a/src/alloc-aligned.c b/src/alloc-aligned.c index 5f60b2fc..b63c5e43 100644 --- a/src/alloc-aligned.c +++ b/src/alloc-aligned.c @@ -69,7 +69,7 @@ static mi_decl_noinline void* mi_heap_malloc_zero_aligned_at_fallback(mi_heap_t* // todo: expand padding if overallocated ? mi_assert_internal(mi_page_usable_block_size(_mi_ptr_page(p)) >= adjust + size); - mi_assert_internal(p == _mi_page_ptr_unalign(_mi_ptr_segment(aligned_p), _mi_ptr_page(aligned_p), aligned_p)); + mi_assert_internal(p == _mi_page_ptr_unalign(_mi_ptr_page(aligned_p), aligned_p)); mi_assert_internal(((uintptr_t)aligned_p + offset) % alignment == 0); mi_assert_internal(mi_usable_size(aligned_p)>=size); mi_assert_internal(mi_usable_size(p) == mi_usable_size(aligned_p)+adjust); diff --git a/src/free.c b/src/free.c index c66de6f6..39443ccf 100644 --- a/src/free.c +++ b/src/free.c @@ -29,17 +29,17 @@ static mi_decl_noinline void mi_free_block_mt(mi_segment_t* segment, mi_page_t* // regular free of a (thread local) block pointer // fast path written carefully to prevent spilling on the stack -static inline void mi_free_block_local(mi_page_t* page, mi_block_t* block, bool check_full) +static inline void mi_free_block_local(mi_page_t* page, mi_block_t* block, bool track_stats, bool check_full) { // checks if mi_unlikely(mi_check_is_double_free(page, block)) return; mi_check_padding(page, block); - mi_stat_free(page, block); + if (track_stats) { mi_stat_free(page, block); } #if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN memset(block, MI_DEBUG_FREED, mi_page_block_size(page)); #endif - mi_track_free_size(p, mi_page_usable_size_of(page,block)); // faster then mi_usable_size as we already know the page and that p is unaligned - + if (track_stats) { mi_track_free_size(p, mi_page_usable_size_of(page, block)); } // faster then mi_usable_size as we already know the page and that p is unaligned + // actual free: push on the local free list mi_block_set_next(page, block, page->local_free); page->local_free = block; @@ -52,7 +52,7 @@ static inline void mi_free_block_local(mi_page_t* page, mi_block_t* block, bool } // Adjust a block that was allocated aligned, to the actual start of the block in the page. -mi_block_t* _mi_page_ptr_unalign(const mi_segment_t* segment, const mi_page_t* page, const void* p) { +mi_block_t* _mi_page_ptr_unalign(const mi_page_t* page, const void* p) { mi_assert_internal(page!=NULL && p!=NULL); size_t diff = (uint8_t*)p - page->page_start; @@ -69,13 +69,14 @@ mi_block_t* _mi_page_ptr_unalign(const mi_segment_t* segment, const mi_page_t* p // free a local pointer (page parameter comes first for better codegen) static void mi_decl_noinline mi_free_generic_local(mi_page_t* page, mi_segment_t* segment, void* p) mi_attr_noexcept { - mi_block_t* const block = (mi_page_has_aligned(page) ? _mi_page_ptr_unalign(segment, page, p) : (mi_block_t*)p); - mi_free_block_local(page, block, true); + MI_UNUSED(segment); + mi_block_t* const block = (mi_page_has_aligned(page) ? _mi_page_ptr_unalign(page, p) : (mi_block_t*)p); + mi_free_block_local(page, block, true, true); } // free a pointer owned by another thread (page parameter comes first for better codegen) static void mi_decl_noinline mi_free_generic_mt(mi_page_t* page, mi_segment_t* segment, void* p) mi_attr_noexcept { - mi_block_t* const block = _mi_page_ptr_unalign(segment, page, p); // don't check `has_aligned` flag to avoid a race (issue #865) + mi_block_t* const block = _mi_page_ptr_unalign(page, p); // don't check `has_aligned` flag to avoid a race (issue #865) mi_free_block_mt(segment, page, block); } @@ -135,7 +136,7 @@ void mi_free(void* p) mi_attr_noexcept if mi_likely(page->flags.full_aligned == 0) { // and it is not a full page (full pages need to move from the full bin), nor has aligned blocks (aligned blocks need to be unaligned) // thread-local, aligned, and not a full page mi_block_t* const block = (mi_block_t*)p; - mi_free_block_local(page, block, false /* no need to check if the page is full */); + mi_free_block_local(page, block, true, false /* no need to check if the page is full */); } else { // page is full or contains (inner) aligned blocks; use generic path @@ -170,7 +171,7 @@ bool _mi_free_delayed_block(mi_block_t* block) { _mi_page_free_collect(page, false); // and free the block (possibly freeing the page as well since used is updated) - mi_free_block_local(page, block, true); + mi_free_block_local(page, block, false /* stats have already been adjusted */, true); return true; } @@ -287,8 +288,8 @@ static void mi_decl_noinline mi_free_block_mt(mi_segment_t* segment, mi_page_t* // ------------------------------------------------------ // Bytes available in a block -static size_t mi_decl_noinline mi_page_usable_aligned_size_of(const mi_segment_t* segment, const mi_page_t* page, const void* p) mi_attr_noexcept { - const mi_block_t* block = _mi_page_ptr_unalign(segment, page, p); +static size_t mi_decl_noinline mi_page_usable_aligned_size_of(const mi_page_t* page, const void* p) mi_attr_noexcept { + const mi_block_t* block = _mi_page_ptr_unalign(page, p); const size_t size = mi_page_usable_size_of(page, block); const ptrdiff_t adjust = (uint8_t*)p - (uint8_t*)block; mi_assert_internal(adjust >= 0 && (size_t)adjust <= size); @@ -305,7 +306,7 @@ static inline size_t _mi_usable_size(const void* p, const char* msg) mi_attr_noe } else { // split out to separate routine for improved code generation - return mi_page_usable_aligned_size_of(segment, page, p); + return mi_page_usable_aligned_size_of(page, p); } } diff --git a/src/options.c b/src/options.c index f8e928d0..8a84d344 100644 --- a/src/options.c +++ b/src/options.c @@ -91,7 +91,7 @@ static mi_option_desc_t options[_mi_option_last] = { 10, UNINIT, MI_OPTION(arena_purge_mult) }, // purge delay multiplier for arena's { 1, UNINIT, MI_OPTION_LEGACY(purge_extend_delay, decommit_extend_delay) }, - { 1, UNINIT, MI_OPTION(abandoned_reclaim_on_free) }, // reclaim an abandoned segment on a free + { 0, UNINIT, MI_OPTION(abandoned_reclaim_on_free) }, // reclaim an abandoned segment on a free }; static void mi_option_init(mi_option_desc_t* desc);