From ec4aa62b656cb11fa984ad36e0d899b8956411b2 Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 09:12:29 -0800 Subject: [PATCH 1/6] use physical memory in kib to avoid overflow of size_t (issue #1010) --- include/mimalloc/prim.h | 18 +++++++++--------- src/os.c | 12 ++++++------ src/prim/unix/prim.c | 2 +- src/prim/windows/prim.c | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/include/mimalloc/prim.h b/include/mimalloc/prim.h index 606b7199..bddd66e9 100644 --- a/include/mimalloc/prim.h +++ b/include/mimalloc/prim.h @@ -22,14 +22,14 @@ terms of the MIT license. A copy of the license can be found in the file // OS memory configuration typedef struct mi_os_mem_config_s { - size_t page_size; // default to 4KiB - size_t large_page_size; // 0 if not supported, usually 2MiB (4MiB on Windows) - size_t alloc_granularity; // smallest allocation size (usually 4KiB, on Windows 64KiB) - size_t physical_memory; // physical memory size - size_t virtual_address_bits; // usually 48 or 56 bits on 64-bit systems. (used to determine secure randomization) - bool has_overcommit; // can we reserve more memory than can be actually committed? - bool has_partial_free; // can allocated blocks be freed partially? (true for mmap, false for VirtualAlloc) - bool has_virtual_reserve; // supports virtual address space reservation? (if true we can reserve virtual address space without using commit or physical memory) + size_t page_size; // default to 4KiB + size_t large_page_size; // 0 if not supported, usually 2MiB (4MiB on Windows) + size_t alloc_granularity; // smallest allocation size (usually 4KiB, on Windows 64KiB) + size_t physical_memory_in_kib; // physical memory size in KiB + size_t virtual_address_bits; // usually 48 or 56 bits on 64-bit systems. (used to determine secure randomization) + bool has_overcommit; // can we reserve more memory than can be actually committed? + bool has_partial_free; // can allocated blocks be freed partially? (true for mmap, false for VirtualAlloc) + bool has_virtual_reserve; // supports virtual address space reservation? (if true we can reserve virtual address space without using commit or physical memory) } mi_os_mem_config_t; // Initialize @@ -124,7 +124,7 @@ void _mi_prim_thread_associate_default_heap(mi_heap_t* heap); //------------------------------------------------------------------- // Access to TLS (thread local storage) slots. // We need fast access to both a unique thread id (in `free.c:mi_free`) and -// to a thread-local heap pointer (in `alloc.c:mi_malloc`). +// to a thread-local heap pointer (in `alloc.c:mi_malloc`). // To achieve this we use specialized code for various platforms. //------------------------------------------------------------------- diff --git a/src/os.c b/src/os.c index 77469775..12cc5da3 100644 --- a/src/os.c +++ b/src/os.c @@ -18,17 +18,17 @@ terms of the MIT license. A copy of the license can be found in the file ----------------------------------------------------------- */ #ifndef MI_DEFAULT_VIRTUAL_ADDRESS_BITS #if MI_INTPTR_SIZE < 8 -#define MI_DEFAULT_VIRTUAL_ADDRESS_BITS 32 +#define MI_DEFAULT_VIRTUAL_ADDRESS_BITS 32 #else -#define MI_DEFAULT_VIRTUAL_ADDRESS_BITS 48 +#define MI_DEFAULT_VIRTUAL_ADDRESS_BITS 48 #endif #endif -#ifndef MI_DEFAULT_PHYSICAL_MEMORY +#ifndef MI_DEFAULT_PHYSICAL_MEMORY_IN_KIB #if MI_INTPTR_SIZE < 8 -#define MI_DEFAULT_PHYSICAL_MEMORY 4*MI_GiB +#define MI_DEFAULT_PHYSICAL_MEMORY_IN_KIB 4*MI_MiB // 4 GiB #else -#define MI_DEFAULT_PHYSICAL_MEMORY 32*MI_GiB +#define MI_DEFAULT_PHYSICAL_MEMORY_IN_KIB 32*MI_MiB // 32 GiB #endif #endif @@ -36,7 +36,7 @@ static mi_os_mem_config_t mi_os_mem_config = { 4096, // page size 0, // large page size (usually 2MiB) 4096, // allocation granularity - MI_DEFAULT_PHYSICAL_MEMORY, + MI_DEFAULT_PHYSICAL_MEMORY_IN_KIB, MI_DEFAULT_VIRTUAL_ADDRESS_BITS, true, // has overcommit? (if true we use MAP_NORESERVE on mmap systems) false, // can we partially free allocated blocks? (on mmap systems we can free anywhere in a mapped range, but on Windows we must free the entire span) diff --git a/src/prim/unix/prim.c b/src/prim/unix/prim.c index c43e6419..da2e2902 100644 --- a/src/prim/unix/prim.c +++ b/src/prim/unix/prim.c @@ -143,7 +143,7 @@ void _mi_prim_mem_init( mi_os_mem_config_t* config ) #if defined(_SC_PHYS_PAGES) long pphys = sysconf(_SC_PHYS_PAGES); if (pphys > 0 && (size_t)pphys < (SIZE_MAX/(size_t)psize)) { - config->physical_memory = (size_t)pphys * (size_t)psize; + config->physical_memory_in_kib = (size_t)pphys * ((size_t)psize / MI_KiB); } #endif } diff --git a/src/prim/windows/prim.c b/src/prim/windows/prim.c index aa79eb91..a248e934 100644 --- a/src/prim/windows/prim.c +++ b/src/prim/windows/prim.c @@ -155,7 +155,7 @@ void _mi_prim_mem_init( mi_os_mem_config_t* config ) ULONGLONG memInKiB = 0; if ((*pGetPhysicallyInstalledSystemMemory)(&memInKiB)) { if (memInKiB > 0 && memInKiB < (SIZE_MAX / MI_KiB)) { - config->physical_memory = (size_t)memInKiB * MI_KiB; + config->physical_memory_in_kib = memInKiB; } } } @@ -643,7 +643,7 @@ static void NTAPI mi_win_main(PVOID module, DWORD reason, LPVOID reserved) { } else if (reason==DLL_THREAD_DETACH && !_mi_is_redirected()) { _mi_thread_done(NULL); - } + } } From 0c8069adab0f85cbae85544746e2578bfda2ebdb Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 09:18:23 -0800 Subject: [PATCH 2/6] use physical memory in kib to avoid overflow of size_t (issue #1010) --- src/prim/unix/prim.c | 5 +++-- src/prim/windows/prim.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/prim/unix/prim.c b/src/prim/unix/prim.c index da2e2902..f9f38f4e 100644 --- a/src/prim/unix/prim.c +++ b/src/prim/unix/prim.c @@ -142,8 +142,9 @@ void _mi_prim_mem_init( mi_os_mem_config_t* config ) config->alloc_granularity = (size_t)psize; #if defined(_SC_PHYS_PAGES) long pphys = sysconf(_SC_PHYS_PAGES); - if (pphys > 0 && (size_t)pphys < (SIZE_MAX/(size_t)psize)) { - config->physical_memory_in_kib = (size_t)pphys * ((size_t)psize / MI_KiB); + const size_t psize_in_kib = (size_t)psize / MI_KiB; + if (psize_in_kib > 0 && pphys > 0 && (size_t)pphys <= (SIZE_MAX/psize_in_kib)) { + config->physical_memory_in_kib = (size_t)pphys * psize_in_kib; } #endif } diff --git a/src/prim/windows/prim.c b/src/prim/windows/prim.c index a248e934..12a7a5d9 100644 --- a/src/prim/windows/prim.c +++ b/src/prim/windows/prim.c @@ -154,7 +154,7 @@ void _mi_prim_mem_init( mi_os_mem_config_t* config ) if (pGetPhysicallyInstalledSystemMemory != NULL) { ULONGLONG memInKiB = 0; if ((*pGetPhysicallyInstalledSystemMemory)(&memInKiB)) { - if (memInKiB > 0 && memInKiB < (SIZE_MAX / MI_KiB)) { + if (memInKiB > 0 && memInKiB <= SIZE_MAX) { config->physical_memory_in_kib = memInKiB; } } From d3897635adc659b3c8c63da2b3e9e031cbe6112d Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 09:22:31 -0800 Subject: [PATCH 3/6] fix compilation warning --- src/prim/windows/prim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prim/windows/prim.c b/src/prim/windows/prim.c index 12a7a5d9..20f833bd 100644 --- a/src/prim/windows/prim.c +++ b/src/prim/windows/prim.c @@ -155,7 +155,7 @@ void _mi_prim_mem_init( mi_os_mem_config_t* config ) ULONGLONG memInKiB = 0; if ((*pGetPhysicallyInstalledSystemMemory)(&memInKiB)) { if (memInKiB > 0 && memInKiB <= SIZE_MAX) { - config->physical_memory_in_kib = memInKiB; + config->physical_memory_in_kib = (size_t)memInKiB; } } } From 69a5fbb1f3f9fdd9361d8a33677d5573c7db5f72 Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 09:32:13 -0800 Subject: [PATCH 4/6] avoid overflow in max address calculation on 32-bit (issue #1010) --- src/page-map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/page-map.c b/src/page-map.c index 74c22e90..44d6de4a 100644 --- a/src/page-map.c +++ b/src/page-map.c @@ -40,7 +40,7 @@ bool _mi_page_map_init(void) { } // Allocate the page map and commit bits - mi_page_map_max_address = (void*)(MI_PU(1) << vbits); + mi_page_map_max_address = (void*)(vbits >= MI_SIZE_BITS ? (SIZE_MAX - MI_ARENA_SLICE_SIZE + 1) : (MI_PU(1) << vbits)); const size_t page_map_size = (MI_ZU(1) << (vbits - MI_ARENA_SLICE_SHIFT)); const bool commit = (page_map_size <= 1*MI_MiB || mi_option_is_enabled(mi_option_pagemap_commit)); // _mi_os_has_overcommit(); // commit on-access on Linux systems? const size_t commit_bits = _mi_divide_up(page_map_size, MI_PAGE_MAP_ENTRIES_PER_COMMIT_BIT); @@ -183,7 +183,7 @@ bool _mi_page_map_init(void) { // Allocate the page map and commit bits mi_assert(MI_MAX_VABITS >= vbits); - mi_page_map_max_address = (void*)(MI_PU(1) << vbits); + mi_page_map_max_address = (void*)(vbits >= MI_SIZE_BITS ? (SIZE_MAX - MI_ARENA_SLICE_SIZE + 1) : (MI_PU(1) << vbits)); const size_t page_map_count = (MI_ZU(1) << (vbits - MI_PAGE_MAP_SUB_SHIFT - MI_ARENA_SLICE_SHIFT)); mi_assert(page_map_count <= MI_PAGE_MAP_COUNT); const size_t os_page_size = _mi_os_page_size(); From 63b8f8f753dae22b5179d639e78f047da07baed6 Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 09:47:03 -0800 Subject: [PATCH 5/6] fix assertion condition --- src/arena-meta.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/arena-meta.c b/src/arena-meta.c index ff50ea60..530e42cb 100644 --- a/src/arena-meta.c +++ b/src/arena-meta.c @@ -25,9 +25,9 @@ terms of the MIT license. A copy of the license can be found in the file #define MI_META_PAGE_SIZE MI_ARENA_SLICE_SIZE #define MI_META_PAGE_ALIGN MI_ARENA_SLICE_ALIGN -#define MI_META_BLOCK_SIZE (128) // large enough such that META_MAX_SIZE > 4k (even on 32-bit) +#define MI_META_BLOCK_SIZE (128) // large enough such that META_MAX_SIZE >= 4k (even on 32-bit) #define MI_META_BLOCK_ALIGN MI_META_BLOCK_SIZE -#define MI_META_BLOCKS_PER_PAGE (MI_ARENA_SLICE_SIZE / MI_META_BLOCK_SIZE) // 1024 +#define MI_META_BLOCKS_PER_PAGE (MI_META_PAGE_SIZE / MI_META_BLOCK_SIZE) // 512 #define MI_META_MAX_SIZE (MI_BCHUNK_SIZE * MI_META_BLOCK_SIZE) typedef struct mi_meta_page_s { @@ -150,7 +150,7 @@ mi_decl_noinline void _mi_meta_free(void* p, size_t size, mi_memid_t memid) { const size_t block_idx = memid.mem.meta.block_index; mi_meta_page_t* mpage = (mi_meta_page_t*)memid.mem.meta.meta_page; mi_assert_internal(mi_meta_page_of_ptr(p,NULL) == mpage); - mi_assert_internal(block_idx + block_count < MI_META_BLOCKS_PER_PAGE); + mi_assert_internal(block_idx + block_count <= MI_META_BLOCKS_PER_PAGE); mi_assert_internal(mi_bbitmap_is_clearN(&mpage->blocks_free, block_idx, block_count)); // we zero on free (and on the initial page allocation) so we don't need a "dirty" map _mi_memzero_aligned(mi_meta_block_start(mpage, block_idx), block_count*MI_META_BLOCK_SIZE); From 44a4c83fbfda403ae25dd436fed4adf3197a62b3 Mon Sep 17 00:00:00 2001 From: daanx Date: Tue, 11 Feb 2025 13:56:58 -0800 Subject: [PATCH 6/6] maintain count in pagequeue for constant time test in free.c --- include/mimalloc/internal.h | 1 + include/mimalloc/types.h | 1 + src/free.c | 5 ++++- src/heap.c | 4 ++++ src/init.c | 2 +- src/page-queue.c | 37 +++++++++++++++++++++++++++++++++++-- 6 files changed, 46 insertions(+), 4 deletions(-) diff --git a/include/mimalloc/internal.h b/include/mimalloc/internal.h index c9f69a26..b45f7565 100644 --- a/include/mimalloc/internal.h +++ b/include/mimalloc/internal.h @@ -213,6 +213,7 @@ void _mi_deferred_free(mi_heap_t* heap, bool force); void _mi_page_free_collect(mi_page_t* page, bool force); void _mi_page_free_collect_partly(mi_page_t* page, mi_block_t* head); void _mi_page_init(mi_heap_t* heap, mi_page_t* page); +bool _mi_page_queue_is_valid(mi_heap_t* heap, const mi_page_queue_t* pq); size_t _mi_bin_size(uint8_t bin); // for stats uint8_t _mi_bin(size_t size); // for stats diff --git a/include/mimalloc/types.h b/include/mimalloc/types.h index 5059ecd1..a743546e 100644 --- a/include/mimalloc/types.h +++ b/include/mimalloc/types.h @@ -389,6 +389,7 @@ typedef struct mi_tld_s mi_tld_t; typedef struct mi_page_queue_s { mi_page_t* first; mi_page_t* last; + size_t count; size_t block_size; } mi_page_queue_t; diff --git a/src/free.c b/src/free.c index 9ca71499..418acd02 100644 --- a/src/free.c +++ b/src/free.c @@ -202,13 +202,16 @@ void mi_free(void* p) mi_attr_noexcept // Multi-threaded Free (`_mt`) // ------------------------------------------------------ static bool mi_page_unown_from_free(mi_page_t* page, mi_block_t* mt_free); -static bool inline mi_page_queue_len_is_atmost( mi_heap_t* heap, size_t block_size, size_t atmost) { +static inline bool mi_page_queue_len_is_atmost( mi_heap_t* heap, size_t block_size, size_t atmost) { mi_page_queue_t* const pq = mi_page_queue(heap,block_size); mi_assert_internal(pq!=NULL); + return (pq->count <= atmost); + /* for(mi_page_t* p = pq->first; p!=NULL; p = p->next, atmost--) { if (atmost == 0) { return false; } } return true; + */ } static void mi_decl_noinline mi_free_try_collect_mt(mi_page_t* page, mi_block_t* mt_free) mi_attr_noexcept { diff --git a/src/heap.c b/src/heap.c index daad8afc..116d0589 100644 --- a/src/heap.c +++ b/src/heap.c @@ -63,6 +63,9 @@ static bool mi_heap_page_is_valid(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_ static bool mi_heap_is_valid(mi_heap_t* heap) { mi_assert_internal(heap!=NULL); mi_heap_visit_pages(heap, &mi_heap_page_is_valid, NULL, NULL); + for (int bin = 0; bin < MI_BIN_COUNT; bin++) { + mi_assert_internal(_mi_page_queue_is_valid(heap, &heap->pages[bin])); + } return true; } #endif @@ -106,6 +109,7 @@ static bool mi_heap_page_collect(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_t static void mi_heap_collect_ex(mi_heap_t* heap, mi_collect_t collect) { if (heap==NULL || !mi_heap_is_initialized(heap)) return; + mi_assert_expensive(mi_heap_is_valid(heap)); const bool force = (collect >= MI_FORCE); _mi_deferred_free(heap, force); diff --git a/src/init.c b/src/init.c index 5bedab85..4cac1c18 100644 --- a/src/init.c +++ b/src/init.c @@ -50,7 +50,7 @@ const mi_page_t _mi_page_empty = { // Empty page queues for every bin -#define QNULL(sz) { NULL, NULL, (sz)*sizeof(uintptr_t) } +#define QNULL(sz) { NULL, NULL, 0, (sz)*sizeof(uintptr_t) } #define MI_PAGE_QUEUES_EMPTY \ { QNULL(1), \ QNULL( 1), QNULL( 2), QNULL( 3), QNULL( 4), QNULL( 5), QNULL( 6), QNULL( 7), QNULL( 8), /* 8 */ \ diff --git a/src/page-queue.c b/src/page-queue.c index 9e3aaacc..5365c0b7 100644 --- a/src/page-queue.c +++ b/src/page-queue.c @@ -49,6 +49,10 @@ static inline bool mi_page_queue_is_special(const mi_page_queue_t* pq) { return (pq->block_size > MI_LARGE_MAX_OBJ_SIZE); } +static inline size_t mi_page_queue_count(const mi_page_queue_t* pq) { + return pq->count; +} + /* ----------------------------------------------------------- Bins ----------------------------------------------------------- */ @@ -142,6 +146,25 @@ static bool mi_heap_contains_queue(const mi_heap_t* heap, const mi_page_queue_t* } #endif +bool _mi_page_queue_is_valid(mi_heap_t* heap, const mi_page_queue_t* pq) { + if (pq==NULL) return false; + size_t count = 0; + mi_page_t* prev = NULL; + for (mi_page_t* page = pq->first; page != NULL; page = page->next) { + mi_assert_internal(page->prev == prev); + mi_assert_internal(mi_page_block_size(page) == pq->block_size); + mi_assert_internal(page->heap == heap); + if (page->next == NULL) { + mi_assert_internal(pq->last == page); + } + count++; + prev = page; + } + mi_assert_internal(pq->count == count); + return true; +} + + static mi_page_queue_t* mi_heap_page_queue_of(mi_heap_t* heap, const mi_page_t* page) { mi_assert_internal(heap!=NULL); uint8_t bin = (mi_page_is_in_full(page) ? MI_BIN_FULL : (mi_page_is_huge(page) ? MI_BIN_HUGE : mi_bin(mi_page_block_size(page)))); @@ -211,6 +234,7 @@ static bool mi_page_queue_is_empty(mi_page_queue_t* queue) { static void mi_page_queue_remove(mi_page_queue_t* queue, mi_page_t* page) { mi_assert_internal(page != NULL); mi_assert_expensive(mi_page_queue_contains(queue, page)); + mi_assert_internal(queue->count >= 1); mi_assert_internal(mi_page_block_size(page) == queue->block_size || (mi_page_is_huge(page) && mi_page_queue_is_huge(queue)) || (mi_page_is_in_full(page) && mi_page_queue_is_full(queue))); @@ -225,6 +249,7 @@ static void mi_page_queue_remove(mi_page_queue_t* queue, mi_page_t* page) { mi_heap_queue_first_update(heap,queue); } heap->page_count--; + queue->count--; page->next = NULL; page->prev = NULL; mi_page_set_in_full(page,false); @@ -253,6 +278,7 @@ static void mi_page_queue_push(mi_heap_t* heap, mi_page_queue_t* queue, mi_page_ else { queue->first = queue->last = page; } + queue->count++; // update direct mi_heap_queue_first_update(heap, queue); @@ -279,6 +305,7 @@ static void mi_page_queue_push_at_end(mi_heap_t* heap, mi_page_queue_t* queue, m else { queue->first = queue->last = page; } + queue->count++; // update direct if (queue->first == page) { @@ -298,6 +325,7 @@ static void mi_page_queue_move_to_front(mi_heap_t* heap, mi_page_queue_t* queue, static void mi_page_queue_enqueue_from_ex(mi_page_queue_t* to, mi_page_queue_t* from, bool enqueue_at_end, mi_page_t* page) { mi_assert_internal(page != NULL); + mi_assert_internal(from->count >= 1); mi_assert_expensive(mi_page_queue_contains(from, page)); mi_assert_expensive(!mi_page_queue_contains(to, page)); const size_t bsize = mi_page_block_size(page); @@ -320,8 +348,10 @@ static void mi_page_queue_enqueue_from_ex(mi_page_queue_t* to, mi_page_queue_t* mi_assert_internal(mi_heap_contains_queue(heap, from)); mi_heap_queue_first_update(heap, from); } + from->count--; // insert into `to` + to->count++; if (enqueue_at_end) { // enqueue at the end page->prev = to->last; @@ -378,15 +408,16 @@ static void mi_page_queue_enqueue_from_full(mi_page_queue_t* to, mi_page_queue_t size_t _mi_page_queue_append(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_queue_t* append) { mi_assert_internal(mi_heap_contains_queue(heap,pq)); mi_assert_internal(pq->block_size == append->block_size); - + if (append->first==NULL) return 0; - + // set append pages to new heap and count size_t count = 0; for (mi_page_t* page = append->first; page != NULL; page = page->next) { mi_page_set_heap(page, heap); count++; } + mi_assert_internal(count == append->count); if (pq->last==NULL) { // take over afresh @@ -403,5 +434,7 @@ size_t _mi_page_queue_append(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_queue append->first->prev = pq->last; pq->last = append->last; } + pq->count += append->count; + return count; }