This is the mail archive of the glibc-cvs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

GNU C Library master sources branch master updated. glibc-2.26.9000-591-g3381be5


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  3381be5cdef2e43949db12f66a5a3ec23b2c4c90 (commit)
      from  e956075a5a2044d05ce48b905b10270ed4a63e87 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=3381be5cdef2e43949db12f66a5a3ec23b2c4c90

commit 3381be5cdef2e43949db12f66a5a3ec23b2c4c90
Author: Wilco Dijkstra <wdijkstr@arm.com>
Date:   Tue Oct 17 18:55:16 2017 +0100

    Improve malloc initialization sequence
    
    The current malloc initialization is quite convoluted. Instead of
    sometimes calling malloc_consolidate from ptmalloc_init, call
    malloc_init_state early so that the main_arena is always initialized.
    The special initialization can now be removed from malloc_consolidate.
    This also fixes BZ #22159.
    
    Check all calls to malloc_consolidate and remove calls that are
    redundant initialization after ptmalloc_init, like in int_mallinfo
    and __libc_mallopt (but keep the latter as consolidation is required for
    set_max_fast).  Update comments to improve clarity.
    
    Remove impossible initialization check from _int_malloc, fix assert
    in do_check_malloc_state to ensure arena->top != 0.  Fix the obvious bugs
    in do_check_free_chunk and do_check_remalloced_chunk to enable single
    threaded malloc debugging (do_check_malloc_state is not thread safe!).
    
    	[BZ #22159]
    	* malloc/arena.c (ptmalloc_init): Call malloc_init_state.
    	* malloc/malloc.c (do_check_free_chunk): Fix build bug.
    	(do_check_remalloced_chunk): Fix build bug.
    	(do_check_malloc_state): Add assert that checks arena->top.
    	(malloc_consolidate): Remove initialization.
    	(int_mallinfo): Remove call to malloc_consolidate.
    	(__libc_mallopt): Clarify why malloc_consolidate is needed.

diff --git a/ChangeLog b/ChangeLog
index fca7345..6cb971d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2017-10-17  Wilco Dijkstra  <wdijkstr@arm.com>
 
+	[BZ #22159]
+	* malloc/arena.c (ptmalloc_init): Call malloc_init_state.
+	* malloc/malloc.c (do_check_free_chunk): Fix build bug.
+	(do_check_remalloced_chunk): Fix build bug.
+	(do_check_malloc_state): Add assert that checks arena->top.
+	(malloc_consolidate): Remove initialization.
+	(int_mallinfo): Remove call to malloc_consolidate.
+	 (__libc_mallopt): Clarify why malloc_consolidate is needed.
+
+2017-10-17  Wilco Dijkstra  <wdijkstr@arm.com>
+
 	* malloc/malloc.c (FASTCHUNKS_BIT): Remove.
 	(have_fastchunks): Remove.
 	(clear_fastchunks): Remove.
diff --git a/malloc/arena.c b/malloc/arena.c
index 9e5a62d..85b985e 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -307,13 +307,9 @@ ptmalloc_init (void)
 
   thread_arena = &main_arena;
 
-#if HAVE_TUNABLES
-  /* Ensure initialization/consolidation and do it under a lock so that a
-     thread attempting to use the arena in parallel waits on us till we
-     finish.  */
-  __libc_lock_lock (main_arena.mutex);
-  malloc_consolidate (&main_arena);
+  malloc_init_state (&main_arena);
 
+#if HAVE_TUNABLES
   TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
   TUNABLE_GET (top_pad, size_t, TUNABLE_CALLBACK (set_top_pad));
   TUNABLE_GET (perturb, int32_t, TUNABLE_CALLBACK (set_perturb_byte));
@@ -322,13 +318,12 @@ ptmalloc_init (void)
   TUNABLE_GET (mmap_max, int32_t, TUNABLE_CALLBACK (set_mmaps_max));
   TUNABLE_GET (arena_max, size_t, TUNABLE_CALLBACK (set_arena_max));
   TUNABLE_GET (arena_test, size_t, TUNABLE_CALLBACK (set_arena_test));
-#if USE_TCACHE
+# if USE_TCACHE
   TUNABLE_GET (tcache_max, size_t, TUNABLE_CALLBACK (set_tcache_max));
   TUNABLE_GET (tcache_count, size_t, TUNABLE_CALLBACK (set_tcache_count));
   TUNABLE_GET (tcache_unsorted_limit, size_t,
 	       TUNABLE_CALLBACK (set_tcache_unsorted_limit));
-#endif
-  __libc_lock_unlock (main_arena.mutex);
+# endif
 #else
   const char *s = NULL;
   if (__glibc_likely (_environ != NULL))
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 670c9f7..51db44f 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1625,8 +1625,10 @@ static INTERNAL_SIZE_T global_max_fast;
 /*
    Set value of max_fast.
    Use impossibly small value if 0.
-   Precondition: there are no existing fastbin chunks.
-   Setting the value clears fastchunk bit but preserves noncontiguous bit.
+   Precondition: there are no existing fastbin chunks in the main arena.
+   Since do_check_malloc_state () checks this, we call malloc_consolidate ()
+   before changing max_fast.  Note other arenas will leak their fast bin
+   entries if max_fast is reduced.
  */
 
 #define set_max_fast(s) \
@@ -1790,11 +1792,8 @@ static struct malloc_par mp_ =
 /*
    Initialize a malloc_state struct.
 
-   This is called only from within malloc_consolidate, which needs
-   be called in the same contexts anyway.  It is never called directly
-   outside of malloc_consolidate because some optimizing compilers try
-   to inline it at all call points, which turns out not to be an
-   optimization at all. (Inlining it in malloc_consolidate is fine though.)
+   This is called from ptmalloc_init () or from _int_new_arena ()
+   when creating a new arena.
  */
 
 static void
@@ -1972,7 +1971,7 @@ do_check_chunk (mstate av, mchunkptr p)
 static void
 do_check_free_chunk (mstate av, mchunkptr p)
 {
-  INTERNAL_SIZE_T sz = p->size & ~(PREV_INUSE | NON_MAIN_ARENA);
+  INTERNAL_SIZE_T sz = chunksize_nomask (p) & ~(PREV_INUSE | NON_MAIN_ARENA);
   mchunkptr next = chunk_at_offset (p, sz);
 
   do_check_chunk (av, p);
@@ -1987,7 +1986,7 @@ do_check_free_chunk (mstate av, mchunkptr p)
       assert ((sz & MALLOC_ALIGN_MASK) == 0);
       assert (aligned_OK (chunk2mem (p)));
       /* ... matching footer field */
-      assert (prev_size (p) == sz);
+      assert (prev_size (next_chunk (p)) == sz);
       /* ... and is fully consolidated */
       assert (prev_inuse (p));
       assert (next == av->top || inuse (next));
@@ -2047,7 +2046,7 @@ do_check_inuse_chunk (mstate av, mchunkptr p)
 static void
 do_check_remalloced_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T s)
 {
-  INTERNAL_SIZE_T sz = p->size & ~(PREV_INUSE | NON_MAIN_ARENA);
+  INTERNAL_SIZE_T sz = chunksize_nomask (p) & ~(PREV_INUSE | NON_MAIN_ARENA);
 
   if (!chunk_is_mmapped (p))
     {
@@ -2123,8 +2122,11 @@ do_check_malloc_state (mstate av)
   /* alignment is a power of 2 */
   assert ((MALLOC_ALIGNMENT & (MALLOC_ALIGNMENT - 1)) == 0);
 
-  /* cannot run remaining checks until fully initialized */
-  if (av->top == 0 || av->top == initial_top (av))
+  /* Check the arena is initialized. */
+  assert (av->top != 0);
+
+  /* No memory has been allocated yet, so doing more tests is not possible.  */
+  if (av->top == initial_top (av))
     return;
 
   /* pagesize is a power of 2 */
@@ -3578,21 +3580,16 @@ _int_malloc (mstate av, size_t bytes)
 
       if ((victim = last (bin)) != bin)
         {
-          if (victim == 0) /* initialization check */
-            malloc_consolidate (av);
-          else
-            {
-              bck = victim->bk;
-	      if (__glibc_unlikely (bck->fd != victim))
-		malloc_printerr
-		  ("malloc(): smallbin double linked list corrupted");
-              set_inuse_bit_at_offset (victim, nb);
-              bin->bk = bck;
-              bck->fd = bin;
-
-              if (av != &main_arena)
-		set_non_main_arena (victim);
-              check_malloced_chunk (av, victim, nb);
+          bck = victim->bk;
+	  if (__glibc_unlikely (bck->fd != victim))
+	    malloc_printerr ("malloc(): smallbin double linked list corrupted");
+          set_inuse_bit_at_offset (victim, nb);
+          bin->bk = bck;
+          bck->fd = bin;
+
+          if (av != &main_arena)
+	    set_non_main_arena (victim);
+          check_malloced_chunk (av, victim, nb);
 #if USE_TCACHE
 	  /* While we're here, if we see other chunks of the same size,
 	     stash them in the tcache.  */
@@ -3619,10 +3616,9 @@ _int_malloc (mstate av, size_t bytes)
 		}
 	    }
 #endif
-              void *p = chunk2mem (victim);
-              alloc_perturb (p, bytes);
-              return p;
-            }
+          void *p = chunk2mem (victim);
+          alloc_perturb (p, bytes);
+          return p;
         }
     }
 
@@ -4320,10 +4316,6 @@ _int_free (mstate av, mchunkptr p, int have_lock)
   purpose since, among other things, it might place chunks back onto
   fastbins.  So, instead, we need to use a minor variant of the same
   code.
-
-  Also, because this routine needs to be called the first time through
-  malloc anyway, it turns out to be the perfect place to trigger
-  initialization code.
 */
 
 static void malloc_consolidate(mstate av)
@@ -4344,84 +4336,73 @@ static void malloc_consolidate(mstate av)
   mchunkptr       bck;
   mchunkptr       fwd;
 
-  /*
-    If max_fast is 0, we know that av hasn't
-    yet been initialized, in which case do so below
-  */
-
-  if (get_max_fast () != 0) {
-    atomic_store_relaxed (&av->have_fastchunks, false);
-
-    unsorted_bin = unsorted_chunks(av);
+  atomic_store_relaxed (&av->have_fastchunks, false);
 
-    /*
-      Remove each chunk from fast bin and consolidate it, placing it
-      then in unsorted bin. Among other reasons for doing this,
-      placing in unsorted bin avoids needing to calculate actual bins
-      until malloc is sure that chunks aren't immediately going to be
-      reused anyway.
-    */
+  unsorted_bin = unsorted_chunks(av);
 
-    maxfb = &fastbin (av, NFASTBINS - 1);
-    fb = &fastbin (av, 0);
-    do {
-      p = atomic_exchange_acq (fb, NULL);
-      if (p != 0) {
-	do {
-	  check_inuse_chunk(av, p);
-	  nextp = p->fd;
-
-	  /* Slightly streamlined version of consolidation code in free() */
-	  size = chunksize (p);
-	  nextchunk = chunk_at_offset(p, size);
-	  nextsize = chunksize(nextchunk);
-
-	  if (!prev_inuse(p)) {
-	    prevsize = prev_size (p);
-	    size += prevsize;
-	    p = chunk_at_offset(p, -((long) prevsize));
-	    unlink(av, p, bck, fwd);
-	  }
+  /*
+    Remove each chunk from fast bin and consolidate it, placing it
+    then in unsorted bin. Among other reasons for doing this,
+    placing in unsorted bin avoids needing to calculate actual bins
+    until malloc is sure that chunks aren't immediately going to be
+    reused anyway.
+  */
 
-	  if (nextchunk != av->top) {
-	    nextinuse = inuse_bit_at_offset(nextchunk, nextsize);
+  maxfb = &fastbin (av, NFASTBINS - 1);
+  fb = &fastbin (av, 0);
+  do {
+    p = atomic_exchange_acq (fb, NULL);
+    if (p != 0) {
+      do {
+	check_inuse_chunk(av, p);
+	nextp = p->fd;
+
+	/* Slightly streamlined version of consolidation code in free() */
+	size = chunksize (p);
+	nextchunk = chunk_at_offset(p, size);
+	nextsize = chunksize(nextchunk);
+
+	if (!prev_inuse(p)) {
+	  prevsize = prev_size (p);
+	  size += prevsize;
+	  p = chunk_at_offset(p, -((long) prevsize));
+	  unlink(av, p, bck, fwd);
+	}
 
-	    if (!nextinuse) {
-	      size += nextsize;
-	      unlink(av, nextchunk, bck, fwd);
-	    } else
-	      clear_inuse_bit_at_offset(nextchunk, 0);
+	if (nextchunk != av->top) {
+	  nextinuse = inuse_bit_at_offset(nextchunk, nextsize);
 
-	    first_unsorted = unsorted_bin->fd;
-	    unsorted_bin->fd = p;
-	    first_unsorted->bk = p;
+	  if (!nextinuse) {
+	    size += nextsize;
+	    unlink(av, nextchunk, bck, fwd);
+	  } else
+	    clear_inuse_bit_at_offset(nextchunk, 0);
 
-	    if (!in_smallbin_range (size)) {
-	      p->fd_nextsize = NULL;
-	      p->bk_nextsize = NULL;
-	    }
+	  first_unsorted = unsorted_bin->fd;
+	  unsorted_bin->fd = p;
+	  first_unsorted->bk = p;
 
-	    set_head(p, size | PREV_INUSE);
-	    p->bk = unsorted_bin;
-	    p->fd = first_unsorted;
-	    set_foot(p, size);
+	  if (!in_smallbin_range (size)) {
+	    p->fd_nextsize = NULL;
+	    p->bk_nextsize = NULL;
 	  }
 
-	  else {
-	    size += nextsize;
-	    set_head(p, size | PREV_INUSE);
-	    av->top = p;
-	  }
+	  set_head(p, size | PREV_INUSE);
+	  p->bk = unsorted_bin;
+	  p->fd = first_unsorted;
+	  set_foot(p, size);
+	}
 
-	} while ( (p = nextp) != 0);
+	else {
+	  size += nextsize;
+	  set_head(p, size | PREV_INUSE);
+	  av->top = p;
+	}
 
-      }
-    } while (fb++ != maxfb);
-  }
-  else {
-    malloc_init_state(av);
-    check_malloc_state(av);
-  }
+      } while ( (p = nextp) != 0);
+
+    }
+  } while (fb++ != maxfb);
 }
 
 /*
@@ -4688,7 +4669,7 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
 static int
 mtrim (mstate av, size_t pad)
 {
-  /* Ensure initialization/consolidation */
+  /* Ensure all blocks are consolidated.  */
   malloc_consolidate (av);
 
   const size_t ps = GLRO (dl_pagesize);
@@ -4819,10 +4800,6 @@ int_mallinfo (mstate av, struct mallinfo *m)
   int nblocks;
   int nfastblocks;
 
-  /* Ensure initialization */
-  if (av->top == 0)
-    malloc_consolidate (av);
-
   check_malloc_state (av);
 
   /* Account for top */
@@ -5071,11 +5048,13 @@ __libc_mallopt (int param_number, int value)
   if (__malloc_initialized < 0)
     ptmalloc_init ();
   __libc_lock_lock (av->mutex);
-  /* Ensure initialization/consolidation */
-  malloc_consolidate (av);
 
   LIBC_PROBE (memory_mallopt, 2, param_number, value);
 
+  /* We must consolidate main arena before changing max_fast
+     (see definition of set_max_fast).  */
+  malloc_consolidate (av);
+
   switch (param_number)
     {
     case M_MXFAST:

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog       |   11 +++
 malloc/arena.c  |   13 +---
 malloc/malloc.c |  197 ++++++++++++++++++++++++------------------------------
 3 files changed, 103 insertions(+), 118 deletions(-)


hooks/post-receive
-- 
GNU C Library master sources


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]