merge from dev

src/alloc.c

@@ -32,10 +32,10 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz
   page->free = mi_block_next(page,block);
   page->used++;
   mi_assert_internal(page->free == NULL || _mi_ptr_page(page->free) == page);
-#if (MI_DEBUG)
+#if (MI_DEBUG!=0)
   if (!page->is_zero) { memset(block, MI_DEBUG_UNINIT, size); }
-#elif (MI_SECURE)
-  block->next = 0;
+#elif (MI_SECURE!=0)
+  block->next = 0;  // don't leak internal data
 #endif
 #if (MI_STAT>1)
   if(size <= MI_LARGE_OBJ_SIZE_MAX) {
@@ -125,10 +125,12 @@ mi_decl_allocator void* mi_zalloc(size_t size) mi_attr_noexcept {
 
 
 // ------------------------------------------------------
-// Check for double free in secure mode
+// Check for double free in secure and debug mode 
+// This is somewhat expensive so only enabled for secure mode 4
 // ------------------------------------------------------
 
-#if MI_SECURE>=4
+#if (MI_ENCODE_FREELIST && (MI_SECURE>=4 || MI_DEBUG!=0))
+// linear check if the free list contains a specific element
 static bool mi_list_contains(const mi_page_t* page, const mi_block_t* list, const mi_block_t* elem) {
   while (list != NULL) {
     if (elem==list) return true;
@@ -137,15 +139,15 @@ static bool mi_list_contains(const mi_page_t* page, const mi_block_t* list, cons
   return false;
 }
 
-static mi_decl_noinline bool mi_check_double_freex(const mi_page_t* page, const mi_block_t* block, const mi_block_t* n) {
+static mi_decl_noinline bool mi_check_is_double_freex(const mi_page_t* page, const mi_block_t* block, const mi_block_t* n) {
   size_t psize;
   uint8_t* pstart = _mi_page_start(_mi_page_segment(page), page, &psize);
   if (n == NULL || ((uint8_t*)n >= pstart && (uint8_t*)n < (pstart + psize))) {
     // Suspicious: the decoded value is in the same page (or NULL).
-    // Walk the free lists to see if it is already freed
+    // Walk the free lists to verify positively if it is already freed
     if (mi_list_contains(page, page->free, block) ||
-      mi_list_contains(page, page->local_free, block) ||
-      mi_list_contains(page, (const mi_block_t*)mi_atomic_read_ptr_relaxed(mi_atomic_cast(void*,&page->thread_free)), block)) 
+        mi_list_contains(page, page->local_free, block) ||
+        mi_list_contains(page, (const mi_block_t*)mi_atomic_read_ptr_relaxed(mi_atomic_cast(void*,&page->thread_free)), block)) 
     {
       _mi_fatal_error("double free detected of block %p with size %zu\n", block, page->block_size);
       return true;
@@ -154,16 +156,23 @@ static mi_decl_noinline bool mi_check_double_freex(const mi_page_t* page, const
   return false;
 }
 
-static inline bool mi_check_double_free(const mi_page_t* page, const mi_block_t* block) {
-  mi_block_t* n = (mi_block_t*)(block->next ^ page->cookie);
-  if (((uintptr_t)n & (MI_INTPTR_SIZE-1))==0 &&       // quick check 
-      (n==NULL || mi_is_in_same_segment(block, n))) 
+static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block_t* block) {
+  mi_block_t* n = mi_block_nextx(page->cookie, block); // pretend it is freed, and get the decoded first field
+  if (((uintptr_t)n & (MI_INTPTR_SIZE-1))==0 &&        // quick check: aligned pointer?
+      (n==NULL || mi_is_in_same_segment(block, n)))    // quick check: in same segment or NULL?
   { 
     // Suspicous: decoded value in block is in the same segment (or NULL) -- maybe a double free?
-    return mi_check_double_freex(page, block, n);
+    // (continue in separate function to improve code generation)
+    return mi_check_is_double_freex(page, block, n);
   }  
   return false;
 }
+#else
+static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block_t* block) {
+  UNUSED(page);
+  UNUSED(block);
+  return false;
+}
 #endif
 
 
@@ -171,7 +180,6 @@ static inline bool mi_check_double_free(const mi_page_t* page, const mi_block_t*
 // Free
 // ------------------------------------------------------
 
-
 // multi-threaded free
 static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* block)
 {
@@ -192,7 +200,10 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc
       page->free = block;
       page->used--;
       page->is_zero = false;
-      _mi_segment_page_free(page,true,&heap->tld->segments);
+      mi_assert(page->used == 0);
+      mi_tld_t* tld = heap->tld;
+      _mi_stat_decrease(&tld->stats.huge, page->block_size);
+      _mi_segment_page_free(page,true,&tld->segments);
     }
     return;
   }
@@ -250,6 +261,7 @@ static inline void _mi_free_block(mi_page_t* page, bool local, mi_block_t* block
   // and push it on the free list
   if (mi_likely(local)) {
     // owning thread can free a block directly
+    if (mi_check_is_double_free(page, block)) return;
     mi_block_set_next(page, block, page->local_free);
     page->local_free = block;
     page->used--;
@@ -293,7 +305,7 @@ void mi_free(void* p) mi_attr_noexcept
   const mi_segment_t* const segment = _mi_ptr_segment(p);
   if (mi_unlikely(segment == NULL)) return;  // checks for (p==NULL)
 
-#if (MI_DEBUG>0)
+#if (MI_DEBUG!=0)
   if (mi_unlikely(!mi_is_in_heap_region(p))) {
     _mi_warning_message("possibly trying to free a pointer that does not point to a valid heap region: 0x%p\n"
       "(this may still be a valid very large allocation (over 64MiB))\n", p);
@@ -302,7 +314,7 @@ void mi_free(void* p) mi_attr_noexcept
     }
   }
 #endif
-#if (MI_DEBUG>0 || MI_SECURE>=4)
+#if (MI_DEBUG!=0 || MI_SECURE>=4)
   if (mi_unlikely(_mi_ptr_cookie(segment) != segment->cookie)) {
     _mi_error_message("trying to free a pointer that does not point to a valid heap space: %p\n", p);
     return;
@@ -324,9 +336,7 @@ void mi_free(void* p) mi_attr_noexcept
   if (mi_likely(tid == 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
     mi_block_t* block = (mi_block_t*)p;
-    #if MI_SECURE>=4 
-    if (mi_check_double_free(page,block)) return;
-    #endif
+    if (mi_check_is_double_free(page,block)) return;    
     mi_block_set_next(page, block, page->local_free);
     page->local_free = block;
     page->used--;

src/init.c

@@ -15,14 +15,14 @@ const mi_page_t _mi_page_empty = {
   0, false, false, false, false, 0, 0,
   { 0 }, false,
   NULL,    // free
-  #if MI_SECURE
+  #if MI_ENCODE_FREELIST
   0,
   #endif
   0,       // used
   NULL, 
   ATOMIC_VAR_INIT(0), ATOMIC_VAR_INIT(0),
   0, NULL, NULL, NULL
-  #if (MI_SECURE==0)
+  #ifndef MI_ENCODE_FREELIST
   , { NULL } // padding
   #endif
 };
@@ -64,8 +64,8 @@ const mi_page_t _mi_page_empty = {
   MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \
   MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \
   MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \
-  MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \
-  MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \
+  MI_STAT_COUNT_NULL(), \
+  { 0, 0 }, { 0, 0 }, { 0, 0 },  \
   { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 } \
   MI_STAT_COUNT_END_NULL()
 

src/memory.c

@@ -461,11 +461,20 @@ void _mi_mem_free(void* p, size_t size, size_t id, mi_stats_t* stats) {
     // reset: 10x slowdown on malloc-large, decommit: 17x slowdown on malloc-large
     if (!is_large) {
       if (mi_option_is_enabled(mi_option_segment_reset)) {
-        _mi_os_reset(p, size, stats);  //
-        // _mi_os_decommit(p,size,stats); // if !is_eager_committed (and clear dirty bits)
+        if (!is_eager_committed &&  // cannot reset large pages
+          (mi_option_is_enabled(mi_option_eager_commit) ||  // cannot reset halfway committed segments, use `option_page_reset` instead
+            mi_option_is_enabled(mi_option_reset_decommits))) // but we can decommit halfway committed segments
+        {
+          _mi_os_reset(p, size, stats);
+          //_mi_os_decommit(p, size, stats);  // todo: and clear dirty bits?
+        }
       }
+<<<<<<< HEAD
       // else { _mi_os_reset(p,size,stats); }
     }
+=======
+    }    
+>>>>>>> dev
     if (!is_eager_committed) {
       // adjust commit statistics as we commit again when re-using the same slot
       _mi_stat_decrease(&stats->committed, mi_good_commit_size(size));

src/options.c

@@ -65,7 +65,7 @@ static mi_option_desc_t options[_mi_option_last] =
   { 0, UNINIT, MI_OPTION(cache_reset) },
   { 0, UNINIT, MI_OPTION(reset_decommits) },     // note: cannot enable this if secure is on
   { 0, UNINIT, MI_OPTION(eager_commit_delay) },  // the first N segments per thread are not eagerly committed
-  { 0, UNINIT, MI_OPTION(segment_reset) },       // reset segment memory on free
+  { 0, UNINIT, MI_OPTION(segment_reset) },       // reset segment memory on free (needs eager commit)
   { 100, UNINIT, MI_OPTION(os_tag) }             // only apple specific for now but might serve more or less related purpose
 };
 
@@ -290,7 +290,9 @@ mi_attr_noreturn void _mi_fatal_error(const char* fmt, ...) {
   va_start(args, fmt);
   mi_vfprintf(NULL, "mimalloc: fatal: ", fmt, args);
   va_end(args);
-  exit(99);
+  #if (MI_SECURE>=0)
+  abort();
+  #endif
 }
 
 // --------------------------------------------------------

src/os.c

@@ -476,7 +476,7 @@ static void* mi_os_mem_alloc(size_t size, size_t try_alignment, bool commit, boo
     int protect_flags = (commit ? (PROT_WRITE | PROT_READ) : PROT_NONE);
     p = mi_unix_mmap(NULL, size, try_alignment, protect_flags, false, allow_large, is_large);
   #endif
-  _mi_stat_increase(&stats->mmap_calls, 1);
+  mi_stat_counter_increase(stats->mmap_calls, 1);
   if (p != NULL) {
     _mi_stat_increase(&stats->reserved, size);
     if (commit) { _mi_stat_increase(&stats->committed, size); }
@@ -631,7 +631,7 @@ static bool mi_os_commitx(void* addr, size_t size, bool commit, bool conservativ
   int err = 0;
   if (commit) {
     _mi_stat_increase(&stats->committed, csize);
-    _mi_stat_increase(&stats->commit_calls, 1);
+    _mi_stat_counter_increase(&stats->commit_calls, 1);
   }
   else {
     _mi_stat_decrease(&stats->committed, csize);

src/page.c

@@ -160,14 +160,21 @@ static void _mi_page_thread_free_collect(mi_page_t* page)
   // return if the list is empty
   if (head == NULL) return;
 
-  // find the tail
+  // find the tail -- also to get a proper count (without data races)
+  uintptr_t max_count = page->capacity; // cannot collect more than capacity
   uintptr_t count = 1;
   mi_block_t* tail = head;
   mi_block_t* next;
-  while ((next = mi_block_next(page,tail)) != NULL) {
+  while ((next = mi_block_next(page,tail)) != NULL && count <= max_count) {
     count++;
     tail = next;
   }
+  // if `count > max_count` there was a memory corruption (possibly infinite list due to double multi-threaded free)
+  if (count > max_count) {
+    _mi_fatal_error("corrupted thread-free list\n");
+    return; // the thread-free items cannot be freed
+  }
+
   // and append the current local free list
   mi_block_set_next(page,tail, page->local_free);
   page->local_free = head;
@@ -377,7 +384,7 @@ void _mi_page_free(mi_page_t* page, mi_page_queue_t* pq, bool force) {
       _mi_stat_decrease(&page->heap->tld->stats.huge, page->block_size);
     }
   }
-
+  
   // remove from the page list
   // (no need to do _mi_heap_delayed_free first as all blocks are already free)
   mi_segments_tld_t* segments_tld = &page->heap->tld->segments;
@@ -405,16 +412,17 @@ void _mi_page_retire(mi_page_t* page) {
   // (or we end up retiring and re-allocating most of the time)
   // NOTE: refine this more: we should not retire if this
   // is the only page left with free blocks. It is not clear
-  // how to check this efficiently though... for now we just check
-  // if its neighbours are almost fully used.
+  // how to check this efficiently though... 
+  // for now, we don't retire if it is the only page left of this size class.
+  mi_page_queue_t* pq = mi_page_queue_of(page);
   if (mi_likely(page->block_size <= (MI_SMALL_SIZE_MAX/4))) {
-    if (mi_page_mostly_used(page->prev) && mi_page_mostly_used(page->next)) {
+    // if (mi_page_mostly_used(page->prev) && mi_page_mostly_used(page->next)) {
+    if (pq->last==page && pq->first==page) {
       mi_stat_counter_increase(_mi_stats_main.page_no_retire,1);
       return; // dont't retire after all
     }
   }
-  
-  _mi_page_free(page, mi_page_queue_of(page), false);
+  _mi_page_free(page, pq, false);
 }
 
 
@@ -508,7 +516,7 @@ static mi_decl_noinline void mi_page_free_list_extend( mi_page_t* page, size_t e
 ----------------------------------------------------------- */
 
 #define MI_MAX_EXTEND_SIZE    (4*1024)      // heuristic, one OS page seems to work well.
-#if MI_SECURE
+#if (MI_SECURE>0)
 #define MI_MIN_EXTEND         (8*MI_SECURE) // extend at least by this many
 #else
 #define MI_MIN_EXTEND         (1)
@@ -529,7 +537,7 @@ static void mi_page_extend_free(mi_heap_t* heap, mi_page_t* page, mi_stats_t* st
 
   size_t page_size;
   _mi_page_start(_mi_page_segment(page), page, &page_size);
-  mi_stat_increase(stats->pages_extended, 1);
+  mi_stat_counter_increase(stats->pages_extended, 1);
 
   // calculate the extend count
   size_t extend = page->reserved - page->capacity;
@@ -577,7 +585,7 @@ static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t block_size, mi
   mi_assert_internal(page_size <= page->slice_count*MI_SEGMENT_SLICE_SIZE);
   mi_assert_internal(page_size / block_size < (1L<<16));
   page->reserved = (uint16_t)(page_size / block_size);
-  #if MI_SECURE
+  #ifdef MI_ENCODE_FREELIST
   page->cookie = _mi_heap_random(heap) | 1;
   #endif
   page->is_zero = page->is_zero_init;
@@ -590,7 +598,7 @@ static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t block_size, mi
   mi_assert_internal(page->next == NULL);
   mi_assert_internal(page->prev == NULL);
   mi_assert_internal(!mi_page_has_aligned(page));
-  #if MI_SECURE
+  #if (MI_ENCODE_FREELIST)
   mi_assert_internal(page->cookie != 0);
   #endif
   mi_assert_expensive(mi_page_is_valid_init(page));

src/stats.c

@@ -95,15 +95,17 @@ static void mi_stats_add(mi_stats_t* stats, const mi_stats_t* src) {
 
   mi_stat_add(&stats->pages_abandoned, &src->pages_abandoned, 1);
   mi_stat_add(&stats->segments_abandoned, &src->segments_abandoned, 1);
-  mi_stat_add(&stats->mmap_calls, &src->mmap_calls, 1);
-  mi_stat_add(&stats->commit_calls, &src->commit_calls, 1);
   mi_stat_add(&stats->threads, &src->threads, 1);
-  mi_stat_add(&stats->pages_extended, &src->pages_extended, 1);
 
   mi_stat_add(&stats->malloc, &src->malloc, 1);
   mi_stat_add(&stats->segments_cache, &src->segments_cache, 1);
   mi_stat_add(&stats->huge, &src->huge, 1);
   mi_stat_add(&stats->large, &src->large, 1);
+
+  mi_stat_counter_add(&stats->pages_extended, &src->pages_extended, 1);
+  mi_stat_counter_add(&stats->mmap_calls, &src->mmap_calls, 1);
+  mi_stat_counter_add(&stats->commit_calls, &src->commit_calls, 1);
+
   mi_stat_counter_add(&stats->page_no_retire, &src->page_no_retire, 1);
   mi_stat_counter_add(&stats->searches, &src->searches, 1);
   mi_stat_counter_add(&stats->huge_count, &src->huge_count, 1);
@@ -121,6 +123,9 @@ static void mi_stats_add(mi_stats_t* stats, const mi_stats_t* src) {
   Display statistics
 ----------------------------------------------------------- */
 
+// unit > 0 : size in binary bytes 
+// unit == 0: count as decimal
+// unit < 0 : count in binary
 static void mi_printf_amount(int64_t n, int64_t unit, mi_output_fun* out, const char* fmt) {
   char buf[32];
   int  len = 32;
@@ -165,17 +170,24 @@ static void mi_stat_print(const mi_stat_count_t* stat, const char* msg, int64_t
       _mi_fprintf(out, "  ok\n");
   }
   else if (unit<0) {
-    mi_print_amount(stat->peak, 1, out);
-    mi_print_amount(stat->allocated, 1, out);
-    mi_print_amount(stat->freed, 1, out);
-    mi_print_amount(-unit, 1, out);
-    mi_print_count((stat->allocated / -unit), 0, out);
+    mi_print_amount(stat->peak, -1, out);
+    mi_print_amount(stat->allocated, -1, out);
+    mi_print_amount(stat->freed, -1, out);
+    if (unit==-1) {
+      _mi_fprintf(out, "%22s", "");
+    }
+    else {
+      mi_print_amount(-unit, 1, out);
+      mi_print_count((stat->allocated / -unit), 0, out);
+    }
     if (stat->allocated > stat->freed)
       _mi_fprintf(out, "  not all freed!\n");
     else
       _mi_fprintf(out, "  ok\n");
   }
   else {
+    mi_print_amount(stat->peak, 1, out);
+    mi_print_amount(stat->allocated, 1, out);
     _mi_fprintf(out, "\n");
   }
 }
@@ -247,11 +259,11 @@ static void _mi_stats_print(mi_stats_t* stats, double secs, mi_output_fun* out)
   mi_stat_print(&stats->segments_cache, "-cached", -1, out);
   mi_stat_print(&stats->pages, "pages", -1, out);
   mi_stat_print(&stats->pages_abandoned, "-abandoned", -1, out);
-  mi_stat_print(&stats->pages_extended, "-extended", 0, out);
+  mi_stat_counter_print(&stats->pages_extended, "-extended", out);
   mi_stat_counter_print(&stats->page_no_retire, "-noretire", out);
-  mi_stat_print(&stats->mmap_calls, "mmaps", 0, out);
-  mi_stat_print(&stats->commit_calls, "commits", 0, out);
-  mi_stat_print(&stats->threads, "threads", 0, out);
+  mi_stat_counter_print(&stats->mmap_calls, "mmaps", out);
+  mi_stat_counter_print(&stats->commit_calls, "commits", out);
+  mi_stat_print(&stats->threads, "threads", -1, out);
   mi_stat_counter_print_avg(&stats->searches, "searches", out);
 
   if (secs >= 0.0) _mi_fprintf(out, "%10s: %9.3f s\n", "elapsed", secs);