diff --git a/include/mimalloc-track.h b/include/mimalloc-track.h index 200f2529..25f800b5 100644 --- a/include/mimalloc-track.h +++ b/include/mimalloc-track.h @@ -13,9 +13,6 @@ terms of the MIT license. A copy of the license can be found in the file // or other memory checkers. // ------------------------------------------------------ - -#define MI_VALGRIND 1 - #if MI_VALGRIND #define MI_TRACK_ENABLED 1 @@ -23,9 +20,9 @@ terms of the MIT license. A copy of the license can be found in the file #include #include -#define mi_track_malloc(p,size,zero) VALGRIND_MALLOCLIKE_BLOCK(p,size,0 /*red zone*/,zero) -#define mi_track_resize(p,oldsize,newsize) VALGRIND_RESIZEINPLACE_BLOCK(p,oldsize,newsize,0 /*red zone*/) -#define mi_track_free(p) VALGRIND_FREELIKE_BLOCK(p,0 /*red zone*/) +#define mi_track_malloc(p,size,zero) VALGRIND_MALLOCLIKE_BLOCK(p,size,MI_PADDING_SIZE /*red zone*/,(zero?1:0)) +#define mi_track_resize(p,oldsize,newsize) VALGRIND_RESIZEINPLACE_BLOCK(p,oldsize,newsize,MI_PADDING_SIZE /*red zone*/) +#define mi_track_free(p) VALGRIND_FREELIKE_BLOCK(p,MI_PADDING_SIZE /*red zone*/) #define mi_track_mem_defined(p,size) VALGRIND_MAKE_MEM_DEFINED(p,size) #define mi_track_mem_undefined(p,size) VALGRIND_MAKE_MEM_UNDEFINED(p,size) #define mi_track_mem_noaccess(p,size) VALGRIND_MAKE_MEM_NOACCESS(p,size) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index a3fad92d..66b31069 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -29,6 +29,9 @@ terms of the MIT license. A copy of the license can be found in the file // Define NDEBUG in the release version to disable assertions. // #define NDEBUG +// Define MI_VALGRIND to enable valgrind support +#define MI_VALGRIND 1 + // Define MI_STAT as 1 to maintain statistics; set it to 2 to have detailed statistics (but costs some performance). // #define MI_STAT 1 @@ -56,7 +59,7 @@ terms of the MIT license. A copy of the license can be found in the file // Reserve extra padding at the end of each block to be more resilient against heap block overflows. // The padding can detect byte-precise buffer overflow on free. -#if !defined(MI_PADDING) && (MI_DEBUG>=1) +#if !defined(MI_PADDING) && (MI_DEBUG>=1 || MI_VALGRIND) #define MI_PADDING 1 #endif @@ -64,7 +67,7 @@ terms of the MIT license. A copy of the license can be found in the file // Encoded free lists allow detection of corrupted free lists // and can detect buffer overflows, modify after free, and double `free`s. #if (MI_SECURE>=3 || MI_DEBUG>=1 || MI_PADDING > 0) -#define MI_ENCODE_FREELIST 1 +#define MI_ENCODE_FREELIST 1 #endif diff --git a/src/alloc.c b/src/alloc.c index 33e565bf..24c1adff 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -40,7 +40,10 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz // allow use of the block internally // todo: can we optimize this call away for non-zero'd release mode? - mi_track_mem_undefined(block,mi_page_block_size(page)); + #if MI_TRACK_ENABLED + const size_t track_bsize = mi_page_block_size(page); + if (zero) mi_track_mem_defined(block,track_bsize); else mi_track_mem_undefined(block,track_bsize); + #endif // zero the block? note: we need to zero the full block size (issue #63) if mi_unlikely(zero) { @@ -70,7 +73,10 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz #if (MI_PADDING > 0) && defined(MI_ENCODE_FREELIST) mi_padding_t* const padding = (mi_padding_t*)((uint8_t*)block + mi_page_usable_block_size(page)); ptrdiff_t delta = ((uint8_t*)padding - (uint8_t*)block - (size - MI_PADDING_SIZE)); + #if (MI_DEBUG>1) mi_assert_internal(delta >= 0 && mi_page_usable_block_size(page) >= (size - MI_PADDING_SIZE + delta)); + mi_track_mem_defined(padding,sizeof(mi_padding_t)); // note: re-enable since mi_page_usable_block_size may set noaccess + #endif padding->canary = (uint32_t)(mi_ptr_encode(page,block,page->keys)); padding->delta = (uint32_t)(delta); uint8_t* fill = (uint8_t*)padding - delta; @@ -79,7 +85,7 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz #endif // mark as no-access again - mi_track_mem_noaccess(block,mi_page_block_size(page)); + mi_track_mem_noaccess(block,track_bsize); return block; } @@ -215,10 +221,14 @@ static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block static bool mi_page_decode_padding(const mi_page_t* page, const mi_block_t* block, size_t* delta, size_t* bsize) { *bsize = mi_page_usable_block_size(page); const mi_padding_t* const padding = (mi_padding_t*)((uint8_t*)block + *bsize); - mi_track_mem_defined(padding,sizeof(*padding)); + mi_track_mem_defined(padding,sizeof(mi_padding_t)); *delta = padding->delta; - bool ok = ((uint32_t)mi_ptr_encode(page,block,page->keys) == padding->canary && *delta <= *bsize); - mi_track_mem_noaccess(padding,sizeof(*padding)); + uint32_t canary = padding->canary; + uintptr_t keys[2]; + keys[0] = page->keys[0]; + keys[1] = page->keys[1]; + bool ok = ((uint32_t)mi_ptr_encode(page,block,keys) == canary && *delta <= *bsize); + mi_track_mem_noaccess(padding,sizeof(mi_padding_t)); return ok; } @@ -349,9 +359,9 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc // The padding check may access the non-thread-owned page for the key values. // that is safe as these are constant and the page won't be freed (as the block is not freed yet). mi_check_padding(page, block); - mi_padding_shrink(page, block, sizeof(mi_block_t)); // for small size, ensure we can fit the delayed thread pointers without triggering overflow detection - #if (MI_DEBUG!=0) - mi_track_mem_undefined(block, mi_page_block_size(page)); // note: check padding may set parts to noaccess + mi_track_mem_defined(block, mi_page_block_size(page)); // ensure the padding can be accessed + mi_padding_shrink(page, block, sizeof(mi_block_t)); // for small size, ensure we can fit the delayed thread pointers without triggering overflow detection + #if (MI_DEBUG!=0) && !MI_TRACK_ENABLED // note: when tracking, cannot use mi_usable_size with multi-threading memset(block, MI_DEBUG_FREED, mi_usable_size(block)); #endif @@ -406,6 +416,7 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc static inline void _mi_free_block(mi_page_t* page, bool local, mi_block_t* block) { // and push it on the free list + //const size_t bsize = mi_page_block_size(page); if mi_likely(local) { // owning thread can free a block directly if mi_unlikely(mi_check_is_double_free(page, block)) return; @@ -442,8 +453,13 @@ mi_block_t* _mi_page_ptr_unalign(const mi_segment_t* segment, const mi_page_t* p static void mi_decl_noinline mi_free_generic(const mi_segment_t* segment, bool local, void* p) mi_attr_noexcept { mi_page_t* const page = _mi_segment_page_of(segment, p); mi_block_t* const block = (mi_page_has_aligned(page) ? _mi_page_ptr_unalign(segment, page, p) : (mi_block_t*)p); - mi_stat_free(page, block); + //#if MI_TRACK_ENABLED + //const size_t track_bsize = mi_page_block_size(page); + //#endif + //mi_track_mem_defined(block,track_bsize); + mi_stat_free(page, block); // stat_free may access the padding _mi_free_block(page, local, block); + //mi_track_mem_noaccess(block,track_bsize); // use track_bsize as the page may have been deallocated by now } // Get the segment data belonging to a pointer @@ -485,27 +501,22 @@ void mi_free(void* p) mi_attr_noexcept { mi_segment_t* const segment = mi_checked_ptr_segment(p,"mi_free"); if mi_unlikely(segment == NULL) return; - mi_track_free(p); mi_threadid_t tid = _mi_thread_id(); mi_page_t* const page = _mi_segment_page_of(segment, p); mi_block_t* const block = (mi_block_t*)p; - #if MI_TRACK_ENABLED - const size_t track_bsize = mi_page_block_size(page); - #endif - if mi_likely(tid == mi_atomic_load_relaxed(&segment->thread_id) && page->flags.full_aligned == 0) { // the thread id matches and it is not a full page, nor has aligned blocks // local, and not full or aligned if mi_unlikely(mi_check_is_double_free(page,block)) return; mi_check_padding(page, block); mi_stat_free(page, block); - #if (MI_DEBUG!=0) - mi_track_mem_undefined(block,track_bsize); // note: check padding may set parts to noaccess + #if (MI_DEBUG!=0) && !MI_TRACK_ENABLED memset(block, MI_DEBUG_FREED, mi_page_block_size(page)); #endif mi_block_set_next(page, block, page->local_free); page->local_free = block; + // mi_track_mem_noaccess(block,mi_page_block_size(page)); if mi_unlikely(--page->used == 0) { // using this expression generates better code than: page->used--; if (mi_page_all_free(page)) _mi_page_retire(page); } @@ -513,10 +524,9 @@ void mi_free(void* p) mi_attr_noexcept else { // non-local, aligned blocks, or a full page; use the more generic path // note: recalc page in generic to improve code generation - mi_track_mem_undefined(block,track_bsize); mi_free_generic(segment, tid == segment->thread_id, p); - } - mi_track_mem_noaccess(block,track_bsize); // cannot use mi_page_block_size as the segment might be deallocated by now + } + mi_track_free(p); } bool _mi_free_delayed_block(mi_block_t* block) { diff --git a/src/region.c b/src/region.c index 15b2cce1..54b7be0c 100644 --- a/src/region.c +++ b/src/region.c @@ -330,7 +330,7 @@ static void* mi_region_try_alloc(size_t blocks, bool* commit, bool* large, bool* } mi_assert_internal(!_mi_bitmap_is_any_claimed(®ion->reset, 1, blocks, bit_idx)); - #if (MI_DEBUG>=2) + #if (MI_DEBUG>=2) && !MI_TRACK_ENABLED if (*commit) { ((uint8_t*)p)[0] = 0; } #endif @@ -376,9 +376,9 @@ void* _mi_mem_alloc_aligned(size_t size, size_t alignment, bool* commit, bool* l if (p != NULL) { mi_assert_internal((uintptr_t)p % alignment == 0); -#if (MI_DEBUG>=2) + #if (MI_DEBUG>=2) && !MI_TRACK_ENABLED if (*commit) { ((uint8_t*)p)[0] = 0; } // ensure the memory is committed -#endif + #endif } return p; } diff --git a/src/segment.c b/src/segment.c index 9a76385a..68174bb2 100644 --- a/src/segment.c +++ b/src/segment.c @@ -531,6 +531,7 @@ static mi_segment_t* mi_segment_init(mi_segment_t* segment, size_t required, mi_ // Try to get it from our thread local cache first if (segment != NULL) { // came from cache + mi_track_mem_defined(segment,info_size); mi_assert_internal(segment->segment_size == segment_size); if (page_kind <= MI_PAGE_MEDIUM && segment->page_kind == page_kind && segment->segment_size == segment_size) { pages_still_good = true; @@ -584,15 +585,15 @@ static mi_segment_t* mi_segment_init(mi_segment_t* segment, size_t required, mi_ return NULL; } } + mi_track_mem_undefined(segment,info_size); segment->memid = memid; segment->mem_is_pinned = (mem_large || is_pinned); segment->mem_is_committed = commit; mi_segments_track_size((long)segment_size, tld); } mi_assert_internal(segment != NULL && (uintptr_t)segment % MI_SEGMENT_SIZE == 0); - mi_assert_internal(segment->mem_is_pinned ? segment->mem_is_committed : true); - mi_track_mem_defined(segment,info_size); - + mi_assert_internal(segment->mem_is_pinned ? segment->mem_is_committed : true); + mi_atomic_store_ptr_release(mi_segment_t, &segment->abandoned_next, NULL); // tsan if (!pages_still_good) { // zero the segment info (but not the `mem` fields) diff --git a/test/test-wrong.c b/test/test-wrong.c index b0e59834..61edb7ea 100644 --- a/test/test-wrong.c +++ b/test/test-wrong.c @@ -16,17 +16,20 @@ int main(int argc, char** argv) { mi_free(r); // undefined access - // printf("undefined: %d\n", *q); + printf("undefined: %d\n", *q); *q = 42; // buffer overflow - // q[1] = 43; + q[1] = 43; + + // buffer underflow + q[-1] = 44; mi(free)(q); // double free - // mi(free)(q); + mi(free)(q); // use after free printf("use-after-free: %d\n", *q);