From 4a0c53ba5a76ae4b1016f88ddba48eb47674c64a Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 21 Apr 2020 15:07:32 +0300 Subject: [PATCH 01/20] Fix the prototype of malloc_size. Otherwise we'll end with invalid redeclaration if malloc/malloc.h is pulled on Darwin --- src/alloc-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/alloc-override.c b/src/alloc-override.c index 0e908f42..75c7990f 100644 --- a/src/alloc-override.c +++ b/src/alloc-override.c @@ -165,7 +165,7 @@ extern "C" { void cfree(void* p) MI_FORWARD0(mi_free, p); void* reallocf(void* p, size_t newsize) MI_FORWARD2(mi_reallocf,p,newsize); -size_t malloc_size(void* p) MI_FORWARD1(mi_usable_size,p); +size_t malloc_size(const void* p) MI_FORWARD1(mi_usable_size,p); #if !defined(__ANDROID__) size_t malloc_usable_size(void *p) MI_FORWARD1(mi_usable_size,p); #else From 07d72f4fba97c698191a855fdf24efddc4387e74 Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 21 Apr 2020 15:08:27 +0300 Subject: [PATCH 02/20] Do not forget to include malloc zone implementation in the static object --- CMakeLists.txt | 1 + src/static.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e37b5083..d0d03991 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,6 +67,7 @@ if(MI_OVERRIDE MATCHES "ON") # use zone's on macOS message(STATUS " Use malloc zone to override malloc (MI_OSX_ZONE=ON)") list(APPEND mi_sources src/alloc-override-osx.c) + list(APPEND mi_defines MI_OSX_ZONE=1) if(NOT MI_INTERPOSE MATCHES "ON") message(STATUS " (enabling INTERPOSE as well since zone's require this)") set(MI_INTERPOSE "ON") diff --git a/src/static.c b/src/static.c index ec9370eb..bf86166d 100644 --- a/src/static.c +++ b/src/static.c @@ -24,5 +24,8 @@ terms of the MIT license. A copy of the license can be found in the file #include "alloc.c" #include "alloc-aligned.c" #include "alloc-posix.c" +#if MI_OSX_ZONE +#include "alloc-override-osx.c" +#endif #include "init.c" #include "options.c" From fe976caaea6d208f1c8499bdf5e478f35df6e67e Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 21 Apr 2020 15:10:49 +0300 Subject: [PATCH 03/20] Provide zone_size function: free() uses it to find the zone pointer belongs to in order to call the corresponding zone_free function --- src/alloc-override-osx.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/alloc-override-osx.c b/src/alloc-override-osx.c index cc03f5e2..c1c880ca 100644 --- a/src/alloc-override-osx.c +++ b/src/alloc-override-osx.c @@ -41,8 +41,11 @@ extern malloc_zone_t* malloc_default_purgeable_zone(void) __attribute__((weak_im ------------------------------------------------------ */ static size_t zone_size(malloc_zone_t* zone, const void* p) { - UNUSED(zone); UNUSED(p); - return 0; // as we cannot guarantee that `p` comes from us, just return 0 + UNUSED(zone); + if (!mi_is_in_heap_region(p)) + return 0; // not our pointer, bail out + + return mi_usable_size(p); } static void* zone_malloc(malloc_zone_t* zone, size_t size) { From 0d3c141243db42dd4a1a7017ecfdfba0985ed70d Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Apr 2020 17:28:44 -0700 Subject: [PATCH 04/20] add check for if commit fails in segment allocation --- src/segment.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/segment.c b/src/segment.c index 8aa838a2..28594843 100644 --- a/src/segment.c +++ b/src/segment.c @@ -627,8 +627,13 @@ static mi_segment_t* mi_segment_init(mi_segment_t* segment, size_t required, mi_ if (!commit) { // ensure the initial info is committed bool commit_zero = false; - _mi_mem_commit(segment, pre_size, &commit_zero, tld->os); + bool ok = _mi_mem_commit(segment, pre_size, &commit_zero, tld->os); if (commit_zero) is_zero = true; + if (!ok) { + // commit failed; we cannot touch the memory: free the segment directly and return `NULL` + _mi_mem_free(segment, MI_SEGMENT_SIZE, memid, false, false, os_tld); + return NULL; + } } segment->memid = memid; segment->mem_is_fixed = mem_large; From 7123efb397b961450bfa38ae5e69ec3431558c1a Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Apr 2020 20:19:48 -0700 Subject: [PATCH 05/20] pass full commit flag to free, possible fix for issue #218 --- src/arena.c | 5 +++-- src/region.c | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/arena.c b/src/arena.c index 724fc52c..bb9fc174 100644 --- a/src/arena.c +++ b/src/arena.c @@ -36,6 +36,7 @@ of 256MiB in practice. // os.c void* _mi_os_alloc_aligned(size_t size, size_t alignment, bool commit, bool* large, mi_os_tld_t* tld); +void _mi_os_free_ex(void* p, size_t size, bool was_committed, mi_stats_t* stats); void _mi_os_free(void* p, size_t size, mi_stats_t* stats); void* _mi_os_alloc_huge_os_pages(size_t pages, int numa_node, mi_msecs_t max_secs, size_t* pages_reserved, size_t* psize); @@ -213,13 +214,13 @@ void* _mi_arena_alloc(size_t size, bool* commit, bool* large, bool* is_zero, siz Arena free ----------------------------------------------------------- */ -void _mi_arena_free(void* p, size_t size, size_t memid, mi_stats_t* stats) { +void _mi_arena_free(void* p, size_t size, size_t memid, bool all_committed, mi_stats_t* stats) { mi_assert_internal(size > 0 && stats != NULL); if (p==NULL) return; if (size==0) return; if (memid == MI_MEMID_OS) { // was a direct OS allocation, pass through - _mi_os_free(p, size, stats); + _mi_os_free_ex(p, size, all_committed, stats); } else { // allocated in an arena diff --git a/src/region.c b/src/region.c index fd7d4544..9db11e0c 100644 --- a/src/region.c +++ b/src/region.c @@ -49,7 +49,7 @@ bool _mi_os_reset(void* p, size_t size, mi_stats_t* stats); bool _mi_os_unreset(void* p, size_t size, bool* is_zero, mi_stats_t* stats); // arena.c -void _mi_arena_free(void* p, size_t size, size_t memid, mi_stats_t* stats); +void _mi_arena_free(void* p, size_t size, size_t memid, bool all_committed, mi_stats_t* stats); void* _mi_arena_alloc(size_t size, bool* commit, bool* large, bool* is_zero, size_t* memid, mi_os_tld_t* tld); void* _mi_arena_alloc_aligned(size_t size, size_t alignment, bool* commit, bool* large, bool* is_zero, size_t* memid, mi_os_tld_t* tld); @@ -187,7 +187,7 @@ static bool mi_region_try_alloc_os(size_t blocks, bool commit, bool allow_large, const uintptr_t idx = mi_atomic_increment(®ions_count); if (idx >= MI_REGION_MAX) { mi_atomic_decrement(®ions_count); - _mi_arena_free(start, MI_REGION_SIZE, arena_memid, tld->stats); + _mi_arena_free(start, MI_REGION_SIZE, arena_memid, region_commit, tld->stats); _mi_warning_message("maximum regions used: %zu GiB (perhaps recompile with a larger setting for MI_HEAP_REGION_MAX_SIZE)", _mi_divide_up(MI_HEAP_REGION_MAX_SIZE, GiB)); return false; } @@ -391,7 +391,7 @@ void _mi_mem_free(void* p, size_t size, size_t id, bool full_commit, bool any_re mem_region_t* region; if (mi_memid_is_arena(id,®ion,&bit_idx,&arena_memid)) { // was a direct arena allocation, pass through - _mi_arena_free(p, size, arena_memid, tld->stats); + _mi_arena_free(p, size, arena_memid, full_commit, tld->stats); } else { // allocated in a region @@ -454,12 +454,13 @@ void _mi_mem_collect(mi_os_tld_t* tld) { // on success, free the whole region uint8_t* start = mi_atomic_read_ptr(uint8_t,®ions[i].start); size_t arena_memid = mi_atomic_read_relaxed(®ions[i].arena_memid); + uintptr_t commit = mi_atomic_read_relaxed(®ions[i].commit); memset(®ions[i], 0, sizeof(mem_region_t)); // and release the whole region mi_atomic_write(®ion->info, 0); if (start != NULL) { // && !_mi_os_is_huge_reserved(start)) { _mi_abandoned_await_readers(); // ensure no pending reads - _mi_arena_free(start, MI_REGION_SIZE, arena_memid, tld->stats); + _mi_arena_free(start, MI_REGION_SIZE, arena_memid, (~commit == 0), tld->stats); } } } From 74a01d05af7568bb637b94ca09a43ef4af214879 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Apr 2020 20:48:55 -0700 Subject: [PATCH 06/20] check commit/protect if eager_commit is disabled --- src/segment.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/segment.c b/src/segment.c index 28594843..3bbe9d40 100644 --- a/src/segment.c +++ b/src/segment.c @@ -204,9 +204,9 @@ static void mi_segment_protect(mi_segment_t* segment, bool protect, mi_os_tld_t* mi_segment_protect_range((uint8_t*)segment + segment->segment_info_size - os_page_size, os_page_size, protect); if (MI_SECURE <= 1 || segment->capacity == 1) { // and protect the last (or only) page too - mi_assert_internal(segment->page_kind >= MI_PAGE_LARGE); + mi_assert_internal(MI_SECURE <= 1 || segment->page_kind >= MI_PAGE_LARGE); uint8_t* start = (uint8_t*)segment + segment->segment_size - os_page_size; - if (protect && !mi_option_is_enabled(mi_option_eager_page_commit)) { + if (protect && !segment->mem_is_committed) { // ensure secure page is committed _mi_mem_commit(start, os_page_size, NULL, tld); } @@ -730,13 +730,12 @@ static void mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_seg mi_assert_internal(!segment->mem_is_fixed); mi_assert_internal(!page->is_reset); page->is_committed = true; - if (segment->page_kind < MI_PAGE_LARGE - || !mi_option_is_enabled(mi_option_eager_page_commit)) { + if (segment->page_kind < MI_PAGE_LARGE || mi_option_is_enabled(mi_option_eager_page_commit)) { size_t psize; uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); bool is_zero = false; const size_t gsize = (MI_SECURE >= 2 ? _mi_os_page_size() : 0); - _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); + _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); // todo: what if this fails? if (gsize > 0) { mi_segment_protect_range(start + psize, gsize, true); } if (is_zero) { page->is_zero_init = true; } } From a5bf45cd1e065d2f7b4312ade4f6ad8232acc970 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Apr 2020 21:01:06 -0700 Subject: [PATCH 07/20] add commit check on page reclaim --- src/segment.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/segment.c b/src/segment.c index 3bbe9d40..a7f7c289 100644 --- a/src/segment.c +++ b/src/segment.c @@ -236,12 +236,12 @@ static void mi_page_reset(mi_segment_t* segment, mi_page_t* page, size_t size, m void* start = mi_segment_raw_page_start(segment, page, &psize); page->is_reset = true; mi_assert_internal(size <= psize); - size_t reset_size = (size == 0 || size > psize ? psize : size); + size_t reset_size = ((size == 0 || size > psize) ? psize : size); if (size == 0 && segment->page_kind >= MI_PAGE_LARGE && !mi_option_is_enabled(mi_option_eager_page_commit)) { mi_assert_internal(page->xblock_size > 0); reset_size = page->capacity * mi_page_block_size(page); } - _mi_mem_reset(start, reset_size, tld->os); + if (reset_size > 0) _mi_mem_reset(start, reset_size, tld->os); } static void mi_page_unreset(mi_segment_t* segment, mi_page_t* page, size_t size, mi_segments_tld_t* tld) @@ -258,7 +258,7 @@ static void mi_page_unreset(mi_segment_t* segment, mi_page_t* page, size_t size, unreset_size = page->capacity * mi_page_block_size(page); } bool is_zero = false; - _mi_mem_unreset(start, unreset_size, &is_zero, tld->os); + if (unreset_size > 0) _mi_mem_unreset(start, unreset_size, &is_zero, tld->os); if (is_zero) page->is_zero_init = true; } @@ -719,27 +719,30 @@ static bool mi_segment_has_free(const mi_segment_t* segment) { return (segment->used < segment->capacity); } -static void mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_segments_tld_t* tld) { +static bool mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_segments_tld_t* tld) { mi_assert_internal(_mi_page_segment(page) == segment); mi_assert_internal(!page->segment_in_use); - // set in-use before doing unreset to prevent delayed reset mi_pages_reset_remove(page, tld); - page->segment_in_use = true; - segment->used++; + // check commit if (!page->is_committed) { mi_assert_internal(!segment->mem_is_fixed); mi_assert_internal(!page->is_reset); - page->is_committed = true; if (segment->page_kind < MI_PAGE_LARGE || mi_option_is_enabled(mi_option_eager_page_commit)) { size_t psize; uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); bool is_zero = false; const size_t gsize = (MI_SECURE >= 2 ? _mi_os_page_size() : 0); - _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); // todo: what if this fails? + bool ok = _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); + if (!ok) return false; // failed to commit! if (gsize > 0) { mi_segment_protect_range(start + psize, gsize, true); } if (is_zero) { page->is_zero_init = true; } } + page->is_committed = true; } + // set in-use before doing unreset to prevent delayed reset + page->segment_in_use = true; + segment->used++; + // check reset if (page->is_reset) { mi_page_unreset(segment, page, 0, tld); // todo: only unreset the part that was reset? } @@ -750,6 +753,7 @@ static void mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_seg mi_assert_internal(!mi_segment_has_free(segment)); mi_segment_remove_from_free_queue(segment, tld); } + return true; } @@ -1216,8 +1220,8 @@ static mi_page_t* mi_segment_find_free(mi_segment_t* segment, mi_segments_tld_t* for (size_t i = 0; i < segment->capacity; i++) { // TODO: use a bitmap instead of search? mi_page_t* page = &segment->pages[i]; if (!page->segment_in_use) { - mi_segment_page_claim(segment, page, tld); - return page; + bool ok = mi_segment_page_claim(segment, page, tld); + if (ok) return page; } } mi_assert(false); From 798cd6647d4ce52f340694570adcd8e25aa21c73 Mon Sep 17 00:00:00 2001 From: Daan Leijen Date: Fri, 24 Apr 2020 07:43:21 -0700 Subject: [PATCH 08/20] use line-buffered output for statistics (issue #235 --- src/options.c | 12 ++++++++---- src/stats.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/options.c b/src/options.c index 0af4a485..1d221cd6 100644 --- a/src/options.c +++ b/src/options.c @@ -260,13 +260,17 @@ static void mi_recurse_exit(void) { } void _mi_fputs(mi_output_fun* out, void* arg, const char* prefix, const char* message) { - if (!mi_recurse_enter()) return; if (out==NULL || (FILE*)out==stdout || (FILE*)out==stderr) { // TODO: use mi_out_stderr for stderr? + if (!mi_recurse_enter()) return; out = mi_out_get_default(&arg); + if (prefix != NULL) out(prefix, arg); + out(message, arg); + mi_recurse_exit(); + } + else { + if (prefix != NULL) out(prefix, arg); + out(message, arg); } - if (prefix != NULL) out(prefix,arg); - out(message,arg); - mi_recurse_exit(); } // Define our own limited `fprintf` that avoids memory allocation. diff --git a/src/stats.c b/src/stats.c index a1404502..478f8229 100644 --- a/src/stats.c +++ b/src/stats.c @@ -237,9 +237,51 @@ static void mi_stats_print_bins(mi_stat_count_t* all, const mi_stat_count_t* bin #endif + +//------------------------------------------------------------ +// Use an output wrapper for line-buffered output +// (which is nice when using loggers etc.) +//------------------------------------------------------------ +typedef struct buffered_s { + mi_output_fun* out; // original output function + void* arg; // and state + char* buf; // local buffer of at least size `count+1` + size_t used; // currently used chars `used <= count` + size_t count; // total chars available for output +} buffered_t; + +static void mi_buffered_flush(buffered_t* buf) { + buf->buf[buf->used] = 0; + _mi_fputs(buf->out, buf->arg, NULL, buf->buf); + buf->used = 0; +} + +static void mi_buffered_out(const char* msg, void* arg) { + buffered_t* buf = (buffered_t*)arg; + if (msg==NULL || buf==NULL) return; + for (const char* src = msg; *src != 0; src++) { + char c = *src; + if (buf->used >= buf->count) mi_buffered_flush(buf); + mi_assert_internal(buf->used < buf->count); + buf->buf[buf->used++] = c; + if (c == '\n') mi_buffered_flush(buf); + } +} + +//------------------------------------------------------------ +// Print statistics +//------------------------------------------------------------ + static void mi_process_info(mi_msecs_t* utime, mi_msecs_t* stime, size_t* peak_rss, size_t* page_faults, size_t* page_reclaim, size_t* peak_commit); -static void _mi_stats_print(mi_stats_t* stats, mi_msecs_t elapsed, mi_output_fun* out, void* arg) mi_attr_noexcept { +static void _mi_stats_print(mi_stats_t* stats, mi_msecs_t elapsed, mi_output_fun* out0, void* arg0) mi_attr_noexcept { + // wrap the output function to be line buffered + char buf[256]; + buffered_t buffer = { out0, arg0, buf, 0, 255 }; + mi_output_fun* out = &mi_buffered_out; + void* arg = &buffer; + + // and print using that mi_print_header(out,arg); #if MI_STAT>1 mi_stat_count_t normal = { 0,0,0,0 }; @@ -287,7 +329,7 @@ static void _mi_stats_print(mi_stats_t* stats, mi_msecs_t elapsed, mi_output_fun _mi_fprintf(out, arg, ", commit charge: "); mi_printf_amount((int64_t)peak_commit, 1, out, arg, "%s"); } - _mi_fprintf(out, arg, "\n"); + _mi_fprintf(out, arg, "\n"); } static mi_msecs_t mi_time_start; // = 0 From 1f8cc50c6b991aa41c94ac3cacb35d6d03ef7c10 Mon Sep 17 00:00:00 2001 From: Daan Leijen Date: Fri, 24 Apr 2020 07:48:22 -0700 Subject: [PATCH 09/20] disable artifact uploading for now as it exceeds the storage space --- azure-pipelines.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 954ec15d..c81e31bd 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -40,8 +40,8 @@ jobs: cd $(BuildType) ctest displayName: CTest - - upload: $(Build.SourcesDirectory)/$(BuildType) - artifact: mimalloc-windows-$(BuildType) +# - upload: $(Build.SourcesDirectory)/$(BuildType) +# artifact: mimalloc-windows-$(BuildType) - job: displayName: Linux @@ -99,8 +99,8 @@ jobs: displayName: Make - script: make test -C $(BuildType) displayName: CTest - - upload: $(Build.SourcesDirectory)/$(BuildType) - artifact: mimalloc-ubuntu-$(BuildType) +# - upload: $(Build.SourcesDirectory)/$(BuildType) +# artifact: mimalloc-ubuntu-$(BuildType) - job: displayName: macOS @@ -127,5 +127,5 @@ jobs: displayName: Make - script: make test -C $(BuildType) displayName: CTest - - upload: $(Build.SourcesDirectory)/$(BuildType) - artifact: mimalloc-macos-$(BuildType) +# - upload: $(Build.SourcesDirectory)/$(BuildType) +# artifact: mimalloc-macos-$(BuildType) From f40aaad87655b6d82b064b412fe93b29c7ab22ce Mon Sep 17 00:00:00 2001 From: Nicolas Date: Fri, 24 Apr 2020 23:20:28 -0300 Subject: [PATCH 10/20] Fix typo in comment Usuelly->Usually --- src/os.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/os.c b/src/os.c index 1c7e03c8..f33cfbc3 100644 --- a/src/os.c +++ b/src/os.c @@ -618,7 +618,7 @@ static void mi_mprotect_hint(int err) { } // Commit/Decommit memory. -// Usuelly commit is aligned liberal, while decommit is aligned conservative. +// Usually commit is aligned liberal, while decommit is aligned conservative. // (but not for the reset version where we want commit to be conservative as well) static bool mi_os_commitx(void* addr, size_t size, bool commit, bool conservative, bool* is_zero, mi_stats_t* stats) { // page align in the range, commit liberally, decommit conservative From 2f1fc1df5ce6dfaecbde6d161aea3f0be5b74547 Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 21 Apr 2020 11:39:57 +0300 Subject: [PATCH 11/20] Add xmalloc()-like functionality. xmalloc is a non-standard extension forcing malloc() to abort should the memory allocation failed instead of returning a null pointer. Such functionality is quite useful as it provides one single point of error handling if the caller of malloc() does not check the result (as it often does!) and segfault is ocurring somewhere else. If more fine-grained control is necessary one could register a custom error handler, however, this might not be an option while interposing. --- CMakeLists.txt | 6 ++++++ include/mimalloc-types.h | 1 - src/options.c | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e37b5083..35e6f81b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,6 +15,7 @@ option(MI_LOCAL_DYNAMIC_TLS "Use slightly slower, dlopen-compatible TLS mechanis option(MI_BUILD_TESTS "Build test executables" ON) option(MI_CHECK_FULL "Use full internal invariant checking in DEBUG mode (deprecated, use MI_DEBUG_FULL instead)" OFF) option(MI_PADDING "Enable padding to detect heap block overflow (only in debug mode)" ON) +option(MI_XMALLOC "Enable abort() call on memory allocation failure by default" OFF) include("cmake/mimalloc-config-version.cmake") @@ -105,6 +106,11 @@ if(MI_PADDING MATCHES "OFF") list(APPEND mi_defines MI_PADDING=0) endif() +if(MI_XMALLOC MATCHES "ON") + message(STATUS "Enable abort() calls on memory allocation failure (MI_XMALLOC=ON)") + list(APPEND mi_defines MI_XMALLOC=1) +endif() + if(MI_USE_CXX MATCHES "ON") message(STATUS "Use the C++ compiler to compile (MI_USE_CXX=ON)") set_source_files_properties(${mi_sources} PROPERTIES LANGUAGE CXX ) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 7e50a2bc..449e2e41 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -61,7 +61,6 @@ terms of the MIT license. A copy of the license can be found in the file #define MI_ENCODE_FREELIST 1 #endif - // ------------------------------------------------------ // Platform specific values // ------------------------------------------------------ diff --git a/src/options.c b/src/options.c index 0af4a485..9fce0398 100644 --- a/src/options.c +++ b/src/options.c @@ -348,6 +348,11 @@ static void mi_error_default(int err) { abort(); } #endif +#if defined(MI_XMALLOC) + if (err==ENOMEM || err==EOVERFLOW) { // abort on memory allocation fails in xmalloc mode + abort(); + } +#endif } void mi_register_error(mi_error_fun* fun, void* arg) { From 079b886febf6082decfa0a39c2d2d2090d56f7a4 Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 21 Apr 2020 10:27:42 +0300 Subject: [PATCH 12/20] Add cmake option to specify whether warnings / errors are enabled by default. Currently warnings / errors are enabled by default in debug build. Otherwise they could be enabled only via environmental variable or API option call. Add possibility to specify the default during the build time. This simplifies e.g. integration of the library into bigger projects as no source changes would be required. --- CMakeLists.txt | 8 ++++++++ include/mimalloc-types.h | 1 - src/options.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e37b5083..1d0cc4fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,6 +15,7 @@ option(MI_LOCAL_DYNAMIC_TLS "Use slightly slower, dlopen-compatible TLS mechanis option(MI_BUILD_TESTS "Build test executables" ON) option(MI_CHECK_FULL "Use full internal invariant checking in DEBUG mode (deprecated, use MI_DEBUG_FULL instead)" OFF) option(MI_PADDING "Enable padding to detect heap block overflow (only in debug mode)" ON) +option(MI_SHOW_ERRORS "Show error and warning messages by default" OFF) include("cmake/mimalloc-config-version.cmake") @@ -105,6 +106,13 @@ if(MI_PADDING MATCHES "OFF") list(APPEND mi_defines MI_PADDING=0) endif() +if(MI_SHOW_ERRORS MATCHES "ON") + message(STATUS "Enable printing of error and warning messages by default (MI_SHOW_ERRORS=ON)") + list(APPEND mi_defines MI_SHOW_ERRORS=1) +else() + list(APPEND mi_defines MI_SHOW_ERRORS=0) +endif() + if(MI_USE_CXX MATCHES "ON") message(STATUS "Use the C++ compiler to compile (MI_USE_CXX=ON)") set_source_files_properties(${mi_sources} PROPERTIES LANGUAGE CXX ) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 7e50a2bc..449e2e41 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -61,7 +61,6 @@ terms of the MIT license. A copy of the license can be found in the file #define MI_ENCODE_FREELIST 1 #endif - // ------------------------------------------------------ // Platform specific values // ------------------------------------------------------ diff --git a/src/options.c b/src/options.c index 0af4a485..3f651874 100644 --- a/src/options.c +++ b/src/options.c @@ -51,7 +51,7 @@ typedef struct mi_option_desc_s { static mi_option_desc_t options[_mi_option_last] = { // stable options - { MI_DEBUG, UNINIT, MI_OPTION(show_errors) }, + { MI_DEBUG | MI_SHOW_ERRORS, UNINIT, MI_OPTION(show_errors) }, { 0, UNINIT, MI_OPTION(show_stats) }, { 0, UNINIT, MI_OPTION(verbose) }, From 0b440d9543e7d3183d800b0986cf1d20531cf507 Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Tue, 28 Apr 2020 19:09:54 +0300 Subject: [PATCH 13/20] Apparently gcc 6 defines __cpp_aligned_new in C++14 mode, however no std::align_val_t is available there for obvious reasons --- src/alloc-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/alloc-override.c b/src/alloc-override.c index 0e908f42..72e8b45e 100644 --- a/src/alloc-override.c +++ b/src/alloc-override.c @@ -103,7 +103,7 @@ terms of the MIT license. A copy of the license can be found in the file void operator delete[](void* p, std::size_t n) noexcept MI_FORWARD02(mi_free_size,p,n); #endif - #if (__cplusplus > 201402L || defined(__cpp_aligned_new)) && (!defined(__GNUC__) || (__GNUC__ > 5)) + #if (__cplusplus > 201402L && defined(__cpp_aligned_new)) && (!defined(__GNUC__) || (__GNUC__ > 5)) void operator delete (void* p, std::align_val_t al) noexcept { mi_free_aligned(p, static_cast(al)); } void operator delete[](void* p, std::align_val_t al) noexcept { mi_free_aligned(p, static_cast(al)); } void operator delete (void* p, std::size_t n, std::align_val_t al) noexcept { mi_free_size_aligned(p, n, static_cast(al)); }; From 9a33f23b5f96d8eefa5640cb23f53626f16c8ea6 Mon Sep 17 00:00:00 2001 From: daan Date: Tue, 28 Apr 2020 11:11:23 -0700 Subject: [PATCH 14/20] fix MI_SHOW_ERRORS on msvc --- CMakeLists.txt | 2 -- src/options.c | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 733cf0a2..e116236d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -116,8 +116,6 @@ endif() if(MI_SHOW_ERRORS MATCHES "ON") message(STATUS "Enable printing of error and warning messages by default (MI_SHOW_ERRORS=ON)") list(APPEND mi_defines MI_SHOW_ERRORS=1) -else() - list(APPEND mi_defines MI_SHOW_ERRORS=0) endif() if(MI_USE_CXX MATCHES "ON") diff --git a/src/options.c b/src/options.c index 76b84589..1a4633ee 100644 --- a/src/options.c +++ b/src/options.c @@ -51,7 +51,11 @@ typedef struct mi_option_desc_s { static mi_option_desc_t options[_mi_option_last] = { // stable options - { MI_DEBUG | MI_SHOW_ERRORS, UNINIT, MI_OPTION(show_errors) }, +#if MI_DEBUG || defined(MI_SHOW_ERRORS) + { 1, UNINIT, MI_OPTION(show_errors) }, +#else + { 0, UNINIT, MI_OPTION(show_errors) }, +#endif { 0, UNINIT, MI_OPTION(show_stats) }, { 0, UNINIT, MI_OPTION(verbose) }, From 0af9dd6fd2b841bb469ed036cbaa1f7a74c81776 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 30 Apr 2020 17:39:34 -0700 Subject: [PATCH 15/20] fix initialization of union padding; issue found through valgrind by @jasongibson --- src/region.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/region.c b/src/region.c index 9db11e0c..ae3a799a 100644 --- a/src/region.c +++ b/src/region.c @@ -205,6 +205,7 @@ static bool mi_region_try_alloc_os(size_t blocks, bool commit, bool allow_large, // and share it mi_region_info_t info; + info.value = 0; // initialize the full union to zero info.x.valid = true; info.x.is_large = region_large; info.x.numa_node = (short)_mi_os_numa_node(tld); From 07a17dfeae7cc61085d257a225133df844a69d5f Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 30 Apr 2020 17:39:34 -0700 Subject: [PATCH 16/20] fix initialization of union padding; issue found through valgrind by @jasongibson --- src/region.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/region.c b/src/region.c index fd7d4544..46e89df3 100644 --- a/src/region.c +++ b/src/region.c @@ -205,6 +205,7 @@ static bool mi_region_try_alloc_os(size_t blocks, bool commit, bool allow_large, // and share it mi_region_info_t info; + info.value = 0; // initialize the full union to zero info.x.valid = true; info.x.is_large = region_large; info.x.numa_node = (short)_mi_os_numa_node(tld); From 5cfdc39ff131902d3da1960b6dc54c64451e34d0 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 30 Apr 2020 18:23:33 -0700 Subject: [PATCH 17/20] remove on-demand page commit option --- include/mimalloc.h | 3 +-- src/page.c | 9 ++------- src/segment.c | 33 ++++++++++----------------------- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/include/mimalloc.h b/include/mimalloc.h index 28d6d1b7..0af04a94 100644 --- a/include/mimalloc.h +++ b/include/mimalloc.h @@ -308,8 +308,7 @@ typedef enum mi_option_e { mi_option_use_numa_nodes, mi_option_os_tag, mi_option_max_errors, - _mi_option_last, - mi_option_eager_page_commit = mi_option_eager_commit + _mi_option_last } mi_option_t; diff --git a/src/page.c b/src/page.c index 2903b258..61645597 100644 --- a/src/page.c +++ b/src/page.c @@ -570,7 +570,8 @@ static void mi_page_extend_free(mi_heap_t* heap, mi_page_t* page, mi_tld_t* tld) if (page->capacity >= page->reserved) return; size_t page_size; - uint8_t* page_start = _mi_page_start(_mi_page_segment(page), page, &page_size); + //uint8_t* page_start = + _mi_page_start(_mi_page_segment(page), page, &page_size); mi_stat_counter_increase(tld->stats.pages_extended, 1); // calculate the extend count @@ -588,12 +589,6 @@ static void mi_page_extend_free(mi_heap_t* heap, mi_page_t* page, mi_tld_t* tld) mi_assert_internal(extend > 0 && extend + page->capacity <= page->reserved); mi_assert_internal(extend < (1UL<<16)); - // commit on-demand for large and huge pages? - if (_mi_page_segment(page)->page_kind >= MI_PAGE_LARGE && !mi_option_is_enabled(mi_option_eager_page_commit)) { - uint8_t* start = page_start + (page->capacity * bsize); - _mi_mem_commit(start, extend * bsize, NULL, &tld->os); - } - // and append the extend the free list if (extend < MI_MIN_SLICES || MI_SECURE==0) { //!mi_option_is_enabled(mi_option_secure)) { mi_page_free_list_extend(page, bsize, extend, &tld->stats ); diff --git a/src/segment.c b/src/segment.c index a7f7c289..d36cf1c5 100644 --- a/src/segment.c +++ b/src/segment.c @@ -237,10 +237,6 @@ static void mi_page_reset(mi_segment_t* segment, mi_page_t* page, size_t size, m page->is_reset = true; mi_assert_internal(size <= psize); size_t reset_size = ((size == 0 || size > psize) ? psize : size); - if (size == 0 && segment->page_kind >= MI_PAGE_LARGE && !mi_option_is_enabled(mi_option_eager_page_commit)) { - mi_assert_internal(page->xblock_size > 0); - reset_size = page->capacity * mi_page_block_size(page); - } if (reset_size > 0) _mi_mem_reset(start, reset_size, tld->os); } @@ -253,10 +249,6 @@ static void mi_page_unreset(mi_segment_t* segment, mi_page_t* page, size_t size, size_t psize; uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); size_t unreset_size = (size == 0 || size > psize ? psize : size); - if (size == 0 && segment->page_kind >= MI_PAGE_LARGE && !mi_option_is_enabled(mi_option_eager_page_commit)) { - mi_assert_internal(page->xblock_size > 0); - unreset_size = page->capacity * mi_page_block_size(page); - } bool is_zero = false; if (unreset_size > 0) _mi_mem_unreset(start, unreset_size, &is_zero, tld->os); if (is_zero) page->is_zero_init = true; @@ -475,10 +467,7 @@ static void mi_segment_os_free(mi_segment_t* segment, size_t segment_size, mi_se if (any_reset && mi_option_is_enabled(mi_option_reset_decommits)) { fully_committed = false; } - if (segment->page_kind >= MI_PAGE_LARGE && !mi_option_is_enabled(mi_option_eager_page_commit)) { - fully_committed = false; - } - + _mi_mem_free(segment, segment_size, segment->memid, fully_committed, any_reset, tld->os); } @@ -726,17 +715,15 @@ static bool mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_seg // check commit if (!page->is_committed) { mi_assert_internal(!segment->mem_is_fixed); - mi_assert_internal(!page->is_reset); - if (segment->page_kind < MI_PAGE_LARGE || mi_option_is_enabled(mi_option_eager_page_commit)) { - size_t psize; - uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); - bool is_zero = false; - const size_t gsize = (MI_SECURE >= 2 ? _mi_os_page_size() : 0); - bool ok = _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); - if (!ok) return false; // failed to commit! - if (gsize > 0) { mi_segment_protect_range(start + psize, gsize, true); } - if (is_zero) { page->is_zero_init = true; } - } + mi_assert_internal(!page->is_reset); + size_t psize; + uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); + bool is_zero = false; + const size_t gsize = (MI_SECURE >= 2 ? _mi_os_page_size() : 0); + bool ok = _mi_mem_commit(start, psize + gsize, &is_zero, tld->os); + if (!ok) return false; // failed to commit! + if (gsize > 0) { mi_segment_protect_range(start + psize, gsize, true); } + if (is_zero) { page->is_zero_init = true; } page->is_committed = true; } // set in-use before doing unreset to prevent delayed reset From 5c03e9dc793610bad6dbb984d1d5a40606288124 Mon Sep 17 00:00:00 2001 From: Anastasios Andronidis Date: Fri, 1 May 2020 17:07:17 +0100 Subject: [PATCH 18/20] Compile static and obj targets with PIC --- CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e37b5083..79a7a577 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -199,6 +199,7 @@ endif() # static library add_library(mimalloc-static STATIC ${mi_sources}) +set_property(TARGET mimalloc-static PROPERTY POSITION_INDEPENDENT_CODE ON) target_compile_definitions(mimalloc-static PRIVATE ${mi_defines} MI_STATIC_LIB) target_compile_options(mimalloc-static PRIVATE ${mi_cflags}) target_link_libraries(mimalloc-static PUBLIC ${mi_libraries}) @@ -235,6 +236,7 @@ endif() # single object file for more predictable static overriding add_library(mimalloc-obj OBJECT src/static.c) +set_property(TARGET mimalloc-obj PROPERTY POSITION_INDEPENDENT_CODE ON) target_compile_definitions(mimalloc-obj PRIVATE ${mi_defines}) target_compile_options(mimalloc-obj PRIVATE ${mi_cflags}) target_include_directories(mimalloc-obj PUBLIC From 097c007ba354b46faacd1fcd06e4c1e39122a8bf Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 2 May 2020 00:19:20 -0700 Subject: [PATCH 19/20] reduce page retire cycles based on object size --- src/page.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/page.c b/src/page.c index 61645597..a7f95a80 100644 --- a/src/page.c +++ b/src/page.c @@ -381,7 +381,7 @@ void _mi_page_free(mi_page_t* page, mi_page_queue_t* pq, bool force) { } #define MI_MAX_RETIRE_SIZE MI_LARGE_OBJ_SIZE_MAX -#define MI_RETIRE_CYCLES (16) +#define MI_RETIRE_CYCLES (8) // Retire a page with no more used blocks // Important to not retire too quickly though as new @@ -406,7 +406,7 @@ void _mi_page_retire(mi_page_t* page) { if (mi_likely(page->xblock_size <= MI_MAX_RETIRE_SIZE && !mi_page_is_in_full(page))) { if (pq->last==page && pq->first==page) { // the only page in the queue? mi_stat_counter_increase(_mi_stats_main.page_no_retire,1); - page->retire_expire = MI_RETIRE_CYCLES; + page->retire_expire = (page->xblock_size <= MI_SMALL_OBJ_SIZE_MAX ? MI_RETIRE_CYCLES : MI_RETIRE_CYCLES/4); mi_heap_t* heap = mi_page_heap(page); mi_assert_internal(pq >= heap->pages); const size_t index = pq - heap->pages; From a801b8e7f1eee1da22c75719489108b4a48c2c6c Mon Sep 17 00:00:00 2001 From: Daan Date: Sat, 2 May 2020 18:08:31 -0700 Subject: [PATCH 20/20] Update readme with descriptions of secure and debug mode --- readme.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/readme.md b/readme.md index 583d54ed..fd600763 100644 --- a/readme.md +++ b/readme.md @@ -255,6 +255,32 @@ OS will copy the entire 1GiB huge page (or 2MiB large page) which can cause the [linux-huge]: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/tuning_and_optimizing_red_hat_enterprise_linux_for_oracle_9i_and_10g_databases/sect-oracle_9i_and_10g_tuning_guide-large_memory_optimization_big_pages_and_huge_pages-configuring_huge_pages_in_red_hat_enterprise_linux_4_or_5 [windows-huge]: https://docs.microsoft.com/en-us/sql/database-engine/configure-windows/enable-the-lock-pages-in-memory-option-windows?view=sql-server-2017 +## Secure Mode + +_mimalloc_ can be build in secure mode by using the `-DMI_SECURE=ON` flags in `cmake`. This build enables various mitigations +to make mimalloc more robust against exploits. In particular: + +- All internal mimalloc pages are surrounded by guard pages and the heap metadata is behind a guard page as well (so a buffer overflow + exploit cannot reach into the metadata), +- All free list pointers are + [encoded](https://github.com/microsoft/mimalloc/blob/783e3377f79ee82af43a0793910a9f2d01ac7863/include/mimalloc-internal.h#L396) + with per-page keys which is used both to prevent overwrites with a known pointer, as well as to detect heap corruption, +- Double free's are detected (and ignored), +- The free lists are initialized in a random order and allocation randomly chooses between extension and reuse within a page to + mitigate against attacks that rely on a predicable allocation order. Similarly, the larger heap blocks allocated by mimalloc + from the OS are also address randomized. + +As always, evaluate with care as part of an overall security strategy as all of the above are mitigations but not guarantees. + +## Debug Mode + +When _mimalloc_ is built using debug mode, various checks are done at runtime to catch development errors. + +- Statistics are maintained in detail for each object size. They can be shown using `MIMALLOC_SHOW_STATS=1` at runtime. +- All objects have padding at the end to detect (byte precise) heap block overflows. +- Double free's, and freeing invalid heap pointers are detected. +- Corrupted free-lists and some forms of use-after-free are detected. + # Overriding Malloc