From 84c706508c83e49158069dca3f65a853b019f6e4 Mon Sep 17 00:00:00 2001 From: daan Date: Sun, 30 Oct 2022 10:45:51 -0700 Subject: [PATCH] fix false positives from valgrind in rptest --- include/mimalloc-track.h | 2 +- include/mimalloc-types.h | 2 +- src/alloc-aligned.c | 6 ++++-- src/alloc-override.c | 12 ++++++------ src/alloc.c | 31 +++++++++++++++++-------------- test/test-wrong.c | 13 +++++++++++-- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/include/mimalloc-track.h b/include/mimalloc-track.h index 25f800b5..bb9df4fa 100644 --- a/include/mimalloc-track.h +++ b/include/mimalloc-track.h @@ -20,7 +20,7 @@ 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,MI_PADDING_SIZE /*red zone*/,(zero?1:0)) +#define mi_track_malloc(p,size,zero) VALGRIND_MALLOCLIKE_BLOCK(p,size,MI_PADDING_SIZE /*red zone*/,zero) #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) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 66b31069..1455da6f 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -66,7 +66,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) +#if (MI_SECURE>=3 || MI_DEBUG>=1 || MI_PADDING > 0) && !MI_VALGRIND #define MI_ENCODE_FREELIST 1 #endif diff --git a/src/alloc-aligned.c b/src/alloc-aligned.c index e72d363f..10e145b4 100644 --- a/src/alloc-aligned.c +++ b/src/alloc-aligned.c @@ -42,8 +42,10 @@ static mi_decl_noinline void* mi_heap_malloc_zero_aligned_at_fallback(mi_heap_t* mi_assert_internal(((uintptr_t)aligned_p + offset) % alignment == 0); mi_assert_internal(p == _mi_page_ptr_unalign(_mi_ptr_segment(aligned_p), _mi_ptr_page(aligned_p), aligned_p)); - mi_track_free(p); - mi_track_malloc(aligned_p,size,zero); + if (p != aligned_p) { + mi_track_free(p); + mi_track_malloc(aligned_p,size,zero); + } return aligned_p; } diff --git a/src/alloc-override.c b/src/alloc-override.c index e29cb4b2..9534e9d5 100644 --- a/src/alloc-override.c +++ b/src/alloc-override.c @@ -29,7 +29,7 @@ typedef struct mi_nothrow_s { int _tag; } mi_nothrow_t; // Override system malloc // ------------------------------------------------------ -#if (defined(__GNUC__) || defined(__clang__)) && !defined(__APPLE__) && !defined(MI_VALGRIND) +#if (defined(__GNUC__) || defined(__clang__)) && !defined(__APPLE__) && !MI_TRACK_ENABLED // gcc, clang: use aliasing to alias the exported function to one of our `mi_` functions #if (defined(__GNUC__) && __GNUC__ >= 9) #pragma GCC diagnostic ignored "-Wattributes" // or we get warnings that nodiscard is ignored on a forward @@ -44,7 +44,7 @@ typedef struct mi_nothrow_s { int _tag; } mi_nothrow_t; #define MI_FORWARD02(fun,x,y) MI_FORWARD(fun) #else // otherwise use forwarding by calling our `mi_` function - #define MI_FORWARD1(fun,x) { return fun(x); } + #define MI_FORWARD1(fun,x) { return fun(x); } #define MI_FORWARD2(fun,x,y) { return fun(x,y); } #define MI_FORWARD3(fun,x,y,z) { return fun(x,y,z); } #define MI_FORWARD0(fun,x) { fun(x); } @@ -123,10 +123,10 @@ typedef struct mi_nothrow_s { int _tag; } mi_nothrow_t; // we just override new/delete which does work in a static library. #else // On all other systems forward to our API - void* malloc(size_t size) MI_FORWARD1(mi_malloc, size) - void* calloc(size_t size, size_t n) MI_FORWARD2(mi_calloc, size, n) - void* realloc(void* p, size_t newsize) MI_FORWARD2(mi_realloc, p, newsize) - void free(void* p) MI_FORWARD0(mi_free, p) + mi_decl_export void* malloc(size_t size) MI_FORWARD1(mi_malloc, size) + mi_decl_export void* calloc(size_t size, size_t n) MI_FORWARD2(mi_calloc, size, n) + mi_decl_export void* realloc(void* p, size_t newsize) MI_FORWARD2(mi_realloc, p, newsize) + mi_decl_export void free(void* p) MI_FORWARD0(mi_free, p) #endif #if (defined(__GNUC__) || defined(__clang__)) && !defined(__APPLE__) diff --git a/src/alloc.c b/src/alloc.c index 24c1adff..f243e8c5 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -42,18 +42,20 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz // todo: can we optimize this call away for non-zero'd release mode? #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); + mi_assert_internal(track_bsize >= size); + mi_track_mem_undefined(block,track_bsize - MI_PADDING_SIZE); #endif // zero the block? note: we need to zero the full block size (issue #63) if mi_unlikely(zero) { mi_assert_internal(page->xblock_size != 0); // do not call with zero'ing for huge blocks (see _mi_malloc_generic) - const size_t zsize = (page->is_zero ? sizeof(block->next) : page->xblock_size); - _mi_memzero_aligned(block, zsize); + const size_t zsize = (page->is_zero ? sizeof(block->next) + MI_PADDING_SIZE : page->xblock_size); + mi_assert(zsize <= track_bsize && zsize >= MI_PADDING_SIZE); + _mi_memzero_aligned(block, zsize - MI_PADDING_SIZE); } #if (MI_DEBUG>0) - if (!page->is_zero && !zero) { memset(block, MI_DEBUG_UNINIT, size); } + if (!page->is_zero && !zero) { memset(block, MI_DEBUG_UNINIT, size - MI_PADDING_SIZE); } #elif (MI_SECURE!=0) if (!zero) { block->next = 0; } // don't leak internal data #endif @@ -70,7 +72,7 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz } #endif -#if (MI_PADDING > 0) && defined(MI_ENCODE_FREELIST) +#if (MI_PADDING > 0) && defined(MI_ENCODE_FREELIST) && !MI_TRACK_ENABLED 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) @@ -85,7 +87,10 @@ 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,track_bsize); + // mi_track_mem_noaccess(block, track_bsize); + // mi_track_resize(block,track_bsize,size - MI_PADDING_SIZE); + // mi_track_free(block); + //mi_track_malloc(block,size - MI_PADDING_SIZE,zero); return block; } @@ -217,7 +222,7 @@ static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block // Check for heap block overflow by setting up padding at the end of the block // --------------------------------------------------------------------------- -#if (MI_PADDING>0) && defined(MI_ENCODE_FREELIST) +#if (MI_PADDING>0) && defined(MI_ENCODE_FREELIST) && !MI_TRACK_ENABLED 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); @@ -421,7 +426,7 @@ static inline void _mi_free_block(mi_page_t* page, bool local, mi_block_t* block // owning thread can free a block directly if mi_unlikely(mi_check_is_double_free(page, block)) return; mi_check_padding(page, block); - #if (MI_DEBUG!=0) + #if (MI_DEBUG!=0) && !MI_TRACK_ENABLED mi_track_mem_undefined(block, mi_page_block_size(page)); // note: check padding may set parts to noaccess memset(block, MI_DEBUG_FREED, mi_page_block_size(page)); #endif @@ -453,13 +458,11 @@ 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); - //#if MI_TRACK_ENABLED - //const size_t track_bsize = mi_page_block_size(page); - //#endif - //mi_track_mem_defined(block,track_bsize); + const size_t track_bsize = mi_page_block_size(page); + //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 + _mi_free_block(page, local, block); + //mi_track_mem_noaccess(block,track_bsize); } // Get the segment data belonging to a pointer diff --git a/test/test-wrong.c b/test/test-wrong.c index 61edb7ea..bb556003 100644 --- a/test/test-wrong.c +++ b/test/test-wrong.c @@ -10,13 +10,21 @@ int main(int argc, char** argv) { int* p = mi(malloc)(3*sizeof(int)); - int* q = mi(malloc)(sizeof(int)); - + int* r = mi_malloc_aligned(8,16); mi_free(r); + // illegal byte wise read + char* c = (char*)mi(malloc)(3); + printf("invalid byte: over: %d, under: %d\n", c[4], c[-1]); + mi(free)(c); + // undefined access + int* q = mi(malloc)(sizeof(int)); printf("undefined: %d\n", *q); + + // illegal int read + printf("invalid: over: %d, under: %d\n", q[1], q[-1]); *q = 42; @@ -28,6 +36,7 @@ int main(int argc, char** argv) { mi(free)(q); + // double free mi(free)(q);