Re: [PATCH][BZ 15089] Fixes malloc_trim always trims for large padding

[Fernando J V da Silva <fernandojvdasilva at gmail dot com>]

Hi Will!

Ah, Ok! I just got confused... Thanks for explaining!
Here is the patch again, now with the changes you requested in the
first e-mail and with ChangeLog inside the commit message.

Cheers,

Fernando J V da Silva


2013/11/21 Will Newton <will.newton@linaro.org>:
> On 21 November 2013 03:00, Fernando J V da Silva
> <fernandojvdasilva@gmail.com> wrote:
>> Hi Will and OndÅej!
>>
>> Thanks for your reviews!
>>
>> Just to make sure I understood... The concern regarding copyright is
>> because of the fix was based on suggestion by Thiago Ize on Bugzilla,
>> as I mentioned on the commit message, right? If that is the case, I
>> should put only an "Ideas by" entry for him at the copyright part of
>> the .c file, is that correct?
>
> I think it is fine as it is, your patch looks like it is different
> from the code snippets in the bug report so the copyright is yours.
> The copyright issue I mentioned was that glibc requires assignment of
> copyright on "significant changes" (see point 8 on
> https://sourceware.org/glibc/wiki/Contribution%20checklist), however
> the link that OndÅej posted defines a "significant change" as 15 lines
> or more so your change should be fine without any of this extra
> paperwork.
>
> --
> Will Newton
> Toolchain Working Group, Linaro

From 78601c256b722897e2edbedad6ebe2e44bdc7c94 Mon Sep 17 00:00:00 2001
From: Fernando J. V. da Silva <fernandojvdasilva@gmail.com>
Date: Thu, 21 Nov 2013 21:03:06 -0200
Subject: [PATCH] Fix BZ #15089: malloc_trim always trim for large padding.

At systrim(), if the heap top size is bigger than requested pad, then
doesn't calculate size to call sbrk().

2013-11-21 Fernando J. V. da Silva <fernandojvdasilva@gmail.com>

        [BZ #15089]
        * malloc/malloc.c: Exit systrim() if pad is bigger than heap top size.
---
 malloc/malloc.c |   70 ++++++++++++++++++++++++++++--------------------------
 1 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/malloc/malloc.c b/malloc/malloc.c
index be472b2..a72c629 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -2712,49 +2712,51 @@ static int systrim(size_t pad, mstate av)
   char* current_brk;     /* address returned by pre-check sbrk call */
   char* new_brk;         /* address returned by post-check sbrk call */
   size_t pagesz;
+  long  top_area;
 
   pagesz = GLRO(dl_pagesize);
   top_size = chunksize(av->top);
 
+  top_area = top_size - MINSIZE - 1;
+  if (top_area <= pad)
+    return 0;
+
   /* Release in pagesize units, keeping at least one page */
-  extra = (top_size - pad - MINSIZE - 1) & ~(pagesz - 1);
+  extra = (top_area - pad) & ~(pagesz - 1);
 
-  if (extra > 0) {
+  /*
+  Only proceed if end of memory is where we last set it.
+  This avoids problems if there were foreign sbrk calls.
+  */
+  current_brk = (char*)(MORECORE(0));
+  if (current_brk == (char*)(av->top) + top_size) {
 
     /*
-      Only proceed if end of memory is where we last set it.
-      This avoids problems if there were foreign sbrk calls.
+    Attempt to release memory. We ignore MORECORE return value,
+    and instead call again to find out where new end of memory is.
+    This avoids problems if first call releases less than we asked,
+    of if failure somehow altered brk value. (We could still
+    encounter problems if it altered brk in some very bad way,
+    but the only thing we can do is adjust anyway, which will cause
+    some downstream failure.)
     */
-    current_brk = (char*)(MORECORE(0));
-    if (current_brk == (char*)(av->top) + top_size) {
 
-      /*
-	Attempt to release memory. We ignore MORECORE return value,
-	and instead call again to find out where new end of memory is.
-	This avoids problems if first call releases less than we asked,
-	of if failure somehow altered brk value. (We could still
-	encounter problems if it altered brk in some very bad way,
-	but the only thing we can do is adjust anyway, which will cause
-	some downstream failure.)
-      */
+    MORECORE(-extra);
+    /* Call the `morecore' hook if necessary.  */
+    void (*hook) (void) = force_reg (__after_morecore_hook);
+    if (__builtin_expect (hook != NULL, 0))
+      (*hook) ();
+    new_brk = (char*)(MORECORE(0));
 
-      MORECORE(-extra);
-      /* Call the `morecore' hook if necessary.  */
-      void (*hook) (void) = force_reg (__after_morecore_hook);
-      if (__builtin_expect (hook != NULL, 0))
-	(*hook) ();
-      new_brk = (char*)(MORECORE(0));
-
-      if (new_brk != (char*)MORECORE_FAILURE) {
-	released = (long)(current_brk - new_brk);
-
-	if (released != 0) {
-	  /* Success. Adjust top. */
-	  av->system_mem -= released;
-	  set_head(av->top, (top_size - released) | PREV_INUSE);
-	  check_malloc_state(av);
-	  return 1;
-	}
+    if (new_brk != (char*)MORECORE_FAILURE) {
+      released = (long)(current_brk - new_brk);
+
+      if (released != 0) {
+        /* Success. Adjust top. */
+        av->system_mem -= released;
+        set_head(av->top, (top_size - released) | PREV_INUSE);
+        check_malloc_state(av);
+        return 1;
       }
     }
   }
-- 
1.7.1