[PATCH] do not place remainders into fastbins

[Michal Schmidt <mschmidt at redhat dot com>]

While working with a program that reads many short strings using
sscanf(..., "%ms", &s), I noticed it used more memory than I had
expected. sscanf starts by allocating 100 bytes, and later
reallocs to the exact size. It resulted in a heap layout like this:
 +-------+------------------+-------+------------------+-------+---
 |string1|       free       |string2|       free       |string3| ...
 +-------+------------------+-------+------------------+-------+---

When testing with fastbins disabled (by setting M_MXFAST to 0),
the resulting heap was nicely condensed instead (and obviously smaller).

I cannot say it's definitely a bug, but IMHO it would be better not to
create fastbins in this testcase. The reasoning goes:

When freeing a chunk that was just created as a remainder from
a shrinking realloc etc., there is no reason to expect that the
application will later malloc chunks of this exact size.
Thus placing the chunk in fastbin is not likely to be helpful.

Let's avoid placing remaindered chunks in fastbins.

Note: I have not signed the FSF copyright Assignment, but I do not
consider this change to be legally significant.

2013-06-06  Michal Schmidt  <mschmidt@redhat.com>

	* malloc/malloc.c (_int_free): Add 'remaindered' argument to avoid
	placing chunks in fastbins.
	(sysmalloc): Adjust call to _int_free.
	(__libc_free): Likewise.
	(__libc_realloc): Likewise.
	(_int_malloc): Likewise.
	(_int_realloc): Likewise.
	(_int_memalign): Likewise.
	* malloc/arena.c (free_atfork): Likewise.
	* malloc/hooks.c (free_check): Likewise.

diff --git a/malloc/arena.c b/malloc/arena.c
index 12a48ad..a51f07a 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -207,7 +207,7 @@ free_atfork(void* mem, const void *caller)
 
   ar_ptr = arena_for_chunk(p);
   tsd_getspecific(arena_key, vptr);
-  _int_free(ar_ptr, p, vptr == ATFORK_ARENA_PTR);
+  _int_free(ar_ptr, p, vptr == ATFORK_ARENA_PTR, 0);
 }
 
 
diff --git a/malloc/hooks.c b/malloc/hooks.c
index 8e4a6ed..e8dbc6c 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -282,7 +282,7 @@ free_check(void* mem, const void *caller)
     munmap_chunk(p);
     return;
   }
-  _int_free(&main_arena, p, 1);
+  _int_free(&main_arena, p, 1, 0);
   (void)mutex_unlock(&main_arena.mutex);
 }
 
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 70b9329..894ce73 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1050,7 +1050,7 @@ typedef struct malloc_chunk* mchunkptr;
 /* Internal routines.  */
 
 static void*  _int_malloc(mstate, size_t);
-static void     _int_free(mstate, mchunkptr, int);
+static void     _int_free(mstate, mchunkptr, int, int);
 static void*  _int_realloc(mstate, mchunkptr, INTERNAL_SIZE_T,
 			   INTERNAL_SIZE_T);
 static void*  _int_memalign(mstate, size_t, size_t);
@@ -2404,7 +2404,7 @@ static void* sysmalloc(INTERNAL_SIZE_T nb, mstate av)
 	set_head(chunk_at_offset(old_top, old_size), (2*SIZE_SZ)|PREV_INUSE);
 	set_foot(chunk_at_offset(old_top, old_size), (2*SIZE_SZ));
 	set_head(old_top, old_size|PREV_INUSE|NON_MAIN_ARENA);
-	_int_free(av, old_top, 1);
+	_int_free(av, old_top, 1, 1);
       } else {
 	set_head(old_top, (old_size + 2*SIZE_SZ)|PREV_INUSE);
 	set_foot(old_top, (old_size + 2*SIZE_SZ));
@@ -2660,7 +2660,7 @@ static void* sysmalloc(INTERNAL_SIZE_T nb, mstate av)
 
 	  /* If possible, release the rest. */
 	  if (old_size >= MINSIZE) {
-	    _int_free(av, old_top, 1);
+	    _int_free(av, old_top, 1, 1);
 	  }
 
 	}
@@ -2900,7 +2900,7 @@ __libc_free(void* mem)
   }
 
   ar_ptr = arena_for_chunk(p);
-  _int_free(ar_ptr, p, 0);
+  _int_free(ar_ptr, p, 0, 0);
 }
 libc_hidden_def (__libc_free)
 
@@ -2990,7 +2990,7 @@ __libc_realloc(void* oldmem, size_t bytes)
       if (newp != NULL)
 	{
 	  MALLOC_COPY (newp, oldmem, oldsize - SIZE_SZ);
-	  _int_free(ar_ptr, oldp, 0);
+	  _int_free(ar_ptr, oldp, 0, 0);
 	}
     }
 
@@ -3721,7 +3721,7 @@ _int_malloc(mstate av, size_t bytes)
 */
 
 static void
-_int_free(mstate av, mchunkptr p, int have_lock)
+_int_free(mstate av, mchunkptr p, int have_lock, int remaindered)
 {
   INTERNAL_SIZE_T size;        /* its size */
   mfastbinptr*    fb;          /* associated fastbin */
@@ -3763,10 +3763,13 @@ _int_free(mstate av, mchunkptr p, int have_lock)
 
   /*
     If eligible, place chunk on a fastbin so it can be found
-    and used quickly in malloc.
+    and used quickly in malloc. Chunks that we have just created
+    as remainders are not eligible, because there's no reason
+    to expect the application to ask for this size.
   */
 
   if ((unsigned long)(size) <= (unsigned long)(get_max_fast ())
+      && !remaindered
 
 #if TRIM_FASTBINS
       /*
@@ -4235,7 +4238,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 	  }
 	}
 
-	_int_free(av, oldp, 1);
+	_int_free(av, oldp, 1, 0);
 	check_inuse_chunk(av, newp);
 	return chunk2mem(newp);
       }
@@ -4259,7 +4262,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 	     (av != &main_arena ? NON_MAIN_ARENA : 0));
     /* Mark remainder as inuse so free() won't complain */
     set_inuse_bit_at_offset(remainder, remainder_size);
-    _int_free(av, remainder, 1);
+    _int_free(av, remainder, 1, 1);
   }
 
   check_inuse_chunk(av, newp);
@@ -4346,7 +4349,7 @@ _int_memalign(mstate av, size_t alignment, size_t bytes)
 	     (av != &main_arena ? NON_MAIN_ARENA : 0));
     set_inuse_bit_at_offset(newp, newsize);
     set_head_size(p, leadsize | (av != &main_arena ? NON_MAIN_ARENA : 0));
-    _int_free(av, p, 1);
+    _int_free(av, p, 1, 1);
     p = newp;
 
     assert (newsize >= nb &&
@@ -4362,7 +4365,7 @@ _int_memalign(mstate av, size_t alignment, size_t bytes)
       set_head(remainder, remainder_size | PREV_INUSE |
 	       (av != &main_arena ? NON_MAIN_ARENA : 0));
       set_head_size(p, nb);
-      _int_free(av, remainder, 1);
+      _int_free(av, remainder, 1, 1);
     }
   }
 
-- 
1.8.1.4