From 71b3e161712e1102761a78e8f30b43b8ec80b886 Mon Sep 17 00:00:00 2001 From: Daan Date: Mon, 31 Mar 2025 10:54:12 -0700 Subject: [PATCH] fix invalid pointer detection in release mode (issue #1051 and #1053) --- include/mimalloc/internal.h | 4 ++-- src/free.c | 23 ++++++----------------- src/init.c | 2 +- src/page-map.c | 4 +--- 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/include/mimalloc/internal.h b/include/mimalloc/internal.h index 0a1ebfc0..9ec28503 100644 --- a/include/mimalloc/internal.h +++ b/include/mimalloc/internal.h @@ -566,14 +566,14 @@ static inline size_t _mi_page_map_index(const void* p, size_t* sub_idx) { static inline mi_page_t* _mi_unchecked_ptr_page(const void* p) { size_t sub_idx; const size_t idx = _mi_page_map_index(p, &sub_idx); - return _mi_page_map[idx][sub_idx]; + return _mi_page_map[idx][sub_idx]; // NULL if p==NULL } static inline mi_page_t* _mi_checked_ptr_page(const void* p) { size_t sub_idx; const size_t idx = _mi_page_map_index(p, &sub_idx); mi_page_t** const sub = _mi_page_map[idx]; - if mi_unlikely(sub == NULL) return (mi_page_t*)&_mi_page_empty; + if mi_unlikely(sub == NULL) return NULL; return sub[sub_idx]; } diff --git a/src/free.c b/src/free.c index 00ecf34d..6b9d3431 100644 --- a/src/free.c +++ b/src/free.c @@ -115,6 +115,7 @@ static inline void mi_block_check_unguard(mi_page_t* page, mi_block_t* block, vo // free a local pointer (page parameter comes first for better codegen) static void mi_decl_noinline mi_free_generic_local(mi_page_t* page, void* p) mi_attr_noexcept { + mi_assert_internal(p!=NULL && page != NULL); mi_block_t* const block = (mi_page_has_aligned(page) ? _mi_page_ptr_unalign(page, p) : (mi_block_t*)p); mi_block_check_unguard(page, block, p); mi_free_block_local(page, block, true /* track stats */, true /* check for a full page */); @@ -122,11 +123,7 @@ static void mi_decl_noinline mi_free_generic_local(mi_page_t* page, void* p) mi_ // 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, void* p) mi_attr_noexcept { - if (p==NULL) return; // a NULL pointer is seen as abandoned (tid==0) with a full flag set - #if !MI_PAGE_MAP_FLAT - if (page==&_mi_page_empty) return; // an invalid pointer may lead to using the empty page - #endif - mi_assert_internal(p!=NULL && page != NULL && page != &_mi_page_empty); + mi_assert_internal(p!=NULL && page != NULL); mi_block_t* const block = _mi_page_ptr_unalign(page, p); // don't check `has_aligned` flag to avoid a race (issue #865) mi_block_check_unguard(page, block, p); mi_free_block_mt(page, block); @@ -150,13 +147,8 @@ static inline mi_page_t* mi_validate_ptr_page(const void* p, const char* msg) return NULL; } mi_page_t* page = _mi_safe_ptr_page(p); - if (page == NULL) { - if (p != NULL) { - _mi_error_message(EINVAL, "%s: invalid pointer: %p\n", msg, p); - } - #if !MI_PAGE_MAP_FLAT - page = (mi_page_t*)&_mi_page_empty; - #endif + if (p != NULL && page == NULL) { + _mi_error_message(EINVAL, "%s: invalid pointer: %p\n", msg, p); } return page; #else @@ -169,12 +161,9 @@ static inline mi_page_t* mi_validate_ptr_page(const void* p, const char* msg) void mi_free(void* p) mi_attr_noexcept { mi_page_t* const page = mi_validate_ptr_page(p,"mi_free"); - - #if MI_PAGE_MAP_FLAT // if not flat, p==NULL leads to `_mi_page_empty` which leads to `mi_free_generic_mt` if mi_unlikely(page==NULL) return; - #endif - mi_assert_internal(page!=NULL); - + mi_assert_internal(p!=NULL && page!=NULL); + const mi_threadid_t xtid = (_mi_prim_thread_id() ^ mi_page_xthread_id(page)); if mi_likely(xtid == 0) { // `tid == mi_page_thread_id(page) && mi_page_flags(page) == 0` // thread-local, aligned, and not a full page diff --git a/src/init.c b/src/init.c index 6d4ce65e..892f4988 100644 --- a/src/init.c +++ b/src/init.c @@ -16,7 +16,7 @@ terms of the MIT license. A copy of the license can be found in the file // Empty page used to initialize the small free pages array const mi_page_t _mi_page_empty = { - MI_ATOMIC_VAR_INIT(MI_PAGE_IN_FULL_QUEUE), // xthread_id (must set flag to catch NULL on a free) + MI_ATOMIC_VAR_INIT(0), // xthread_id NULL, // free 0, // used 0, // capacity diff --git a/src/page-map.c b/src/page-map.c index 44d6de4a..78e8ab8e 100644 --- a/src/page-map.c +++ b/src/page-map.c @@ -210,9 +210,7 @@ bool _mi_page_map_init(void) { if (!mi_page_map_memid.initially_committed) { _mi_os_commit(_mi_page_map[0], os_page_size, NULL); // only first OS page } - _mi_page_map[0][0] = (mi_page_t*)&_mi_page_empty; // caught in `mi_free` - - mi_assert_internal(_mi_ptr_page(NULL)==&_mi_page_empty); + mi_assert_internal(_mi_ptr_page(NULL)==NULL); return true; }