avr-libc-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[avr-libc-commit] [2131] bug #27235, malloc: Several things go wrong


From: Joerg Wunsch
Subject: [avr-libc-commit] [2131] bug #27235, malloc: Several things go wrong
Date: Mon, 07 Jun 2010 18:09:17 +0000

Revision: 2131
          http://svn.sv.gnu.org/viewvc/?view=rev&root=avr-libc&revision=2131
Author:   joerg_wunsch
Date:     2010-06-07 18:09:17 +0000 (Mon, 07 Jun 2010)
Log Message:
-----------
bug #27235, malloc: Several things go wrong
* libc/stdlib/malloc.c (malloc): Revert the patch
from r2123, as it is no longer needed.
* libc/stdlib/malloc.c (free): When freeing the
topmost chunk of the heap, reduce __brkval again,
returning the area from the heap entirely.
* doc/api/malloc.dox: Mention that the heap is now
reduced if possible.
* tests/simulate/regression/bug-27242.c: Adapt testscript
to match the new behaviour.
* tests/simulate/stdlib/malloc-1.c: (Ditto.)
* tests/simulate/stdlib/malloc-6.c: (Ditto.)
* tests/simulate/stdlib/malloc-7.c: (Ditto.)
* tests/simulate/stdlib/realloc-2.c: New file (test whether
subsequent allocation does not extend the heap).
* tests/simulate/regression/bug-28135.c: New file, another
regression test (actual bugfix from bug #28135 is no longer
needed)

Ticket Links:
:-----------
    http://savannah.gnu.org/bugs/?27235
    http://savannah.gnu.org/bugs/?28135

Modified Paths:
--------------
    trunk/avr-libc/ChangeLog
    trunk/avr-libc/NEWS
    trunk/avr-libc/doc/api/malloc.dox
    trunk/avr-libc/libc/stdlib/malloc.c
    trunk/avr-libc/tests/simulate/regression/bug-27242.c
    trunk/avr-libc/tests/simulate/stdlib/malloc-1.c
    trunk/avr-libc/tests/simulate/stdlib/malloc-6.c
    trunk/avr-libc/tests/simulate/stdlib/malloc-7.c

Added Paths:
-----------
    trunk/avr-libc/tests/simulate/regression/bug-28135.c
    trunk/avr-libc/tests/simulate/stdlib/realloc-2.c

Modified: trunk/avr-libc/ChangeLog
===================================================================
--- trunk/avr-libc/ChangeLog    2010-06-07 16:27:16 UTC (rev 2130)
+++ trunk/avr-libc/ChangeLog    2010-06-07 18:09:17 UTC (rev 2131)
@@ -1,5 +1,26 @@
 2010-06-07  Joerg Wunsch <address@hidden>
 
+       bug #27235, malloc: Several things go wrong
+       * libc/stdlib/malloc.c (malloc): Revert the patch
+       from r2123, as it is no longer needed.
+       * libc/stdlib/malloc.c (free): When freeing the
+       topmost chunk of the heap, reduce __brkval again,
+       returning the area from the heap entirely.
+       * doc/api/malloc.dox: Mention that the heap is now
+       reduced if possible.
+       * tests/simulate/regression/bug-27242.c: Adapt testscript
+       to match the new behaviour.
+       * tests/simulate/stdlib/malloc-1.c: (Ditto.)
+       * tests/simulate/stdlib/malloc-6.c: (Ditto.)
+       * tests/simulate/stdlib/malloc-7.c: (Ditto.)
+       * tests/simulate/stdlib/realloc-2.c: New file (test whether
+       subsequent allocation does not extend the heap).
+       * tests/simulate/regression/bug-28135.c: New file, another
+       regression test (actual bugfix from bug #28135 is no longer
+       needed)
+
+2010-06-07  Joerg Wunsch <address@hidden>
+
        More testscripts: test free() works as expected.
        * tests/simulate/stdlib/malloc-5.c: New file.
        * tests/simulate/stdlib/malloc-6.c: (Ditto.)

Modified: trunk/avr-libc/NEWS
===================================================================
--- trunk/avr-libc/NEWS 2010-06-07 16:27:16 UTC (rev 2130)
+++ trunk/avr-libc/NEWS 2010-06-07 18:09:17 UTC (rev 2131)
@@ -103,7 +103,7 @@
   [#26840] Missing #define in iom32m1.h
   [#26876] include io.h into fuse.h
   [#27201] _WORDREGISTER in xmega headers does not work in C99 mode
-  [#27235] malloc: Several things go wrong (part 1)
+  [#27235] malloc: Several things go wrong
   [#27242] realloc: serious error when size shrinks
   [#27243] malloc: small error in comment
   [#27367] RAMSTART missing in elder device header files

Modified: trunk/avr-libc/doc/api/malloc.dox
===================================================================
--- trunk/avr-libc/doc/api/malloc.dox   2010-06-07 16:27:16 UTC (rev 2130)
+++ trunk/avr-libc/doc/api/malloc.dox   2010-06-07 18:09:17 UTC (rev 2131)
@@ -214,7 +214,8 @@
 attempt is then made to aggregate the new entry with possible adjacent
 entries, yielding a single larger entry available for further
 allocations.  That way, the potential for heap fragmentation is hopefully
-reduced.
+reduced.  When deallocating the topmost chunk of memory, the size of the
+heap is reduced.
 
 A call to realloc() first determines whether the operation is about to
 grow or shrink the current allocation.  When shrinking, the case is

Modified: trunk/avr-libc/libc/stdlib/malloc.c
===================================================================
--- trunk/avr-libc/libc/stdlib/malloc.c 2010-06-07 16:27:16 UTC (rev 2130)
+++ trunk/avr-libc/libc/stdlib/malloc.c 2010-06-07 18:09:17 UTC (rev 2131)
@@ -164,22 +164,7 @@
         * segment as dynamically requested from the operating system.
         * Since we don't have an operating system, just make sure
         * that we don't collide with the stack.
-        * But first we check if there is a free chunk at the end of
-        * the allocation area. Brkval is corrected downwards then,
-        * so the free chunk gets added to the new chunk.
-        * (fp2 still points to the last list entry from step 1)
         */
-       if (fp2 && (char*)fp2 + fp2->sz + sizeof(size_t) == __brkval) {
-       __brkval = (char*)fp2;
-               for (fp1 = __flp, fp2 = 0;
-                    fp1->nx;
-                fp2 = fp1, fp1 = fp1->nx)
-                       {}
-               if (fp2)
-                       fp2->nx = 0;
-               else
-                       __flp = 0;
-       }
        if (__brkval == 0)
                __brkval = __malloc_heap_start;
        cp = __malloc_heap_end;
@@ -225,10 +210,14 @@
 
        /*
         * Trivial case first: if there's no freelist yet, our entry
-        * will be the only one on it.
+        * will be the only one on it.  If this is the last entry, we
+        * can reduce __brkval instead.
         */
        if (__flp == 0) {
-               __flp = fpnew;
+               if ((char *)p + fpnew->sz == __brkval)
+                       __brkval = cpnew;
+               else
+                       __flp = fpnew;
                return;
        }
 
@@ -269,6 +258,22 @@
                fp2->sz += fpnew->sz + sizeof(size_t);
                fp2->nx = fpnew->nx;
        }
+       /*
+        * If there's a new topmost chunk, lower __brkval instead.
+        */
+       while (fp2->nx != NULL) {
+               fp1 = fp2;
+               fp2 = fp2->nx;
+       }
+       cp2 = (char *)&(fp2->nx);
+       if (cp2 + fp2->sz == __brkval) {
+               if (fp1 == NULL)
+                       /* Freelist is empty now. */
+                       __flp = NULL;
+               else
+                       fp1->nx = NULL;
+               __brkval = cp2 - sizeof(size_t);
+       }
 }
 
 #ifdef MALLOC_TEST

Modified: trunk/avr-libc/tests/simulate/regression/bug-27242.c
===================================================================
--- trunk/avr-libc/tests/simulate/regression/bug-27242.c        2010-06-07 
16:27:16 UTC (rev 2130)
+++ trunk/avr-libc/tests/simulate/regression/bug-27242.c        2010-06-07 
18:09:17 UTC (rev 2131)
@@ -53,11 +53,11 @@
        /*
         * The patch for bug #25723 introduces a regression to realloc()
         * when called to shrink a memory chunk. We simply verify if
-        * the new free chunk starts immediately after the last allocated
+        * the new __brkval starts immediately after the last allocated
         * chunk.
         */
        p = p1 + 10;
-       if ((char*)__flp != p)
+       if (__brkval != p)
                return 3;
 
        /* use the last free chunk */
@@ -73,13 +73,14 @@
        if (!p)
                return 6;
 
-       /* force creation of a new minimal free chunk (sz = 3) */
+       /* force creation of a new minimal free chunk (sz = 3), which
+           will be returned from heap then */
        p1 = realloc(p, 5);
        if (p != p1)
                return 7;
 
        p = p1 + 5;
-       if ((char*)__flp != p)
+       if (__brkval != p)
                return 8;
 
        /* merge with the free chunk (incr = 4 > sz = 3) */
@@ -87,9 +88,9 @@
        if (p != p1)
                return 9;
 
-       /* chunk size should be 10 */
+       /* chunk size should be 9 */
        p -= sizeof(size_t);
-       if (*(size_t *)p != 10)
+       if (*(size_t *)p != 9)
                return 10;
 
        /* should be empty */

Added: trunk/avr-libc/tests/simulate/regression/bug-28135.c
===================================================================
--- trunk/avr-libc/tests/simulate/regression/bug-28135.c                        
        (rev 0)
+++ trunk/avr-libc/tests/simulate/regression/bug-28135.c        2010-06-07 
18:09:17 UTC (rev 2131)
@@ -0,0 +1,126 @@
+/* Copyright (c) 2009 Aleksandar Kanchev
+   All rights reserved.
+
+   Redistribution and use in source and binary forms, with or without
+   modification, are permitted provided that the following conditions are met:
+
+   * Redistributions of source code must retain the above copyright
+     notice, this list of conditions and the following disclaimer.
+
+   * Redistributions in binary form must reproduce the above copyright
+     notice, this list of conditions and the following disclaimer in
+     the documentation and/or other materials provided with the
+     distribution.
+
+   * Neither the name of the copyright holders nor the names of
+     contributors may be used to endorse or promote products derived
+     from this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+  POSSIBILITY OF SUCH DAMAGE.
+*/
+
+/* $Id$ */
+
+#include <stdlib.h>
+
+#include "../../libc/stdlib/stdlib_private.h"
+
+
+/* Test code from bug # */
+int main(void)
+{
+       char *p, *p1;
+
+       /* Step 1: test with just one free chunk */
+
+       p = malloc(20);
+       if (!p)
+               return 1;
+
+       /* releasing 10 bytes returns them from the heap */
+       p1 = realloc(p, 10);
+       if (p != p1)
+               return 2;
+
+       if (__flp)
+               return 3;
+
+       /* should extend the free chunk */
+       p = malloc(15);
+       if (!p)
+               return 4;
+
+       /* should be empty */
+       if (__flp)
+               return 5;
+
+       /* should be the following chunk */
+       p1 += 10 + sizeof(size_t);
+       if (p != p1)
+               return 6;
+
+       /* should point right after the last chunk */
+       p += 15;
+       if (p != __brkval)
+               return 7;
+
+       /* Step 2: Test with 2 free chunks. */
+
+       p = malloc(8);
+       if (!p)
+               return 8;
+
+       p1 = malloc(9);
+       if (!p1)
+               return 9;
+
+       free(p);
+
+       p = malloc(20);
+       if (!p)
+               return 10;
+
+       if (!__flp)
+               return 11;
+
+       p1 = realloc(p, 10);
+       if (p1 != p)
+               return 12;
+
+       if (!__flp)
+               return 13;
+
+       if (__flp->nx)
+               return 14;
+
+       p = malloc(15);
+       if (!p)
+               return 15;
+
+       if (!__flp)
+               return 16;
+
+       if (__flp->nx)
+               return 17;
+
+       p1 += 10 + sizeof(size_t);
+       if (p != p1)
+               return 18;
+
+       p += 15;
+       if (p != __brkval)
+               return 19;
+
+       return 0;
+}
+


Property changes on: trunk/avr-libc/tests/simulate/regression/bug-28135.c
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + Author Id Date
Added: svn:eol-style
   + native

Modified: trunk/avr-libc/tests/simulate/stdlib/malloc-1.c
===================================================================
--- trunk/avr-libc/tests/simulate/stdlib/malloc-1.c     2010-06-07 16:27:16 UTC 
(rev 2130)
+++ trunk/avr-libc/tests/simulate/stdlib/malloc-1.c     2010-06-07 18:09:17 UTC 
(rev 2131)
@@ -93,11 +93,8 @@
         free(ptrs[i]);
     }
 
-    /* after freeing everything, the freelist must contain one
-       chunk of the full TARGETVAL size (minus the size for one
-       pointer), and the "nx" pointer must be NULL */
-    if (__flp->nx != NULL) return __LINE__;
-    if (__flp->sz != TARGETVAL - 2) return __LINE__;
+    /* after freeing everything, the freelist must be empty */
+    if (__flp != NULL) return __LINE__;
 
     return 0;
 }

Modified: trunk/avr-libc/tests/simulate/stdlib/malloc-6.c
===================================================================
--- trunk/avr-libc/tests/simulate/stdlib/malloc-6.c     2010-06-07 16:27:16 UTC 
(rev 2130)
+++ trunk/avr-libc/tests/simulate/stdlib/malloc-6.c     2010-06-07 18:09:17 UTC 
(rev 2131)
@@ -67,11 +67,9 @@
     }
 
     free(ptrs[5]);
-    /* There must be exactly one entry in freelist. */
-    if ((char *)(ptrs[5]) - 2 != (void *)__flp) return __LINE__;
-    if (__flp->sz != 10) return __LINE__;
-    if (__flp->nx != NULL) return __LINE__;
-    if ((char *)(ptrs[5]) + 10 != (void *)__brkval) return __LINE__;
+    /* Freelist must be still empty, and __brkval reduced. */
+    if (__flp != NULL) return __LINE__;
+    if ((char *)(ptrs[5]) - 2 != __brkval) return __LINE__;
 
     return 0;
 }

Modified: trunk/avr-libc/tests/simulate/stdlib/malloc-7.c
===================================================================
--- trunk/avr-libc/tests/simulate/stdlib/malloc-7.c     2010-06-07 16:27:16 UTC 
(rev 2130)
+++ trunk/avr-libc/tests/simulate/stdlib/malloc-7.c     2010-06-07 18:09:17 UTC 
(rev 2131)
@@ -67,39 +67,32 @@
     }
 
     free(ptrs[5]);
-    /* There must be exactly one entry in freelist. */
-    if ((char *)(ptrs[5]) - 2 != (void *)__flp) return __LINE__;
-    if (__flp->sz != 10) return __LINE__;
-    if (__flp->nx != NULL) return __LINE__;
-    if ((char *)(ptrs[5]) + 10 != (void *)__brkval) return __LINE__;
+    /* Freelist must be still empty, and __brkval reduced. */
+    if (__flp != NULL) return __LINE__;
+    if ((char *)(ptrs[5]) - 2 != __brkval) return __LINE__;
 
     free(ptrs[4]);
-    /* Still one entry expected. */
-    if ((char *)(ptrs[4]) - 2 != (void *)__flp) return __LINE__;
-    if (__flp->sz != 22) return __LINE__;
-    if (__flp->nx != NULL) return __LINE__;
-    if ((char *)(ptrs[4]) + 22 != (void *)__brkval) return __LINE__;
+    /* Still no entry, and __brkval further down. */
+    if (__flp != NULL) return __LINE__;
+    if ((char *)(ptrs[4]) - 2 != __brkval) return __LINE__;
 
     struct __freelist *ofp = __flp;
 
     free(ptrs[1]);
-    /* Another entry added. */
+    /* One entry added. */
     if ((char *)(ptrs[1]) - 2 != (void *)__flp) return __LINE__;
     if (__flp->sz != 10) return __LINE__;
     if (__flp->nx != (void *)ofp) return __LINE__;
 
     free(ptrs[3]);
-    /* Second entry must have been extended. */
-    if ((char *)(ptrs[3]) - 2 != (void *)__flp->nx) return __LINE__;
-    if (__flp->nx->sz != 34) return __LINE__;
-    if (__flp->nx->nx != NULL) return __LINE__;
-    if ((char *)(ptrs[3]) + 34 != (void *)__brkval) return __LINE__;
+    /* __brkval lowered again. */
+    if (__flp->nx != NULL) return __LINE__;
+    if ((char *)(ptrs[3]) - 2 != __brkval) return __LINE__;
 
     free(ptrs[2]);
-    /* Both chunks must have been combined. */
-    if (__flp->sz != 58) return __LINE__;
+    /* ...and again. */
     if (__flp->nx != NULL) return __LINE__;
-    if ((char *)(ptrs[1]) + 58 != (void *)__brkval) return __LINE__;
+    if ((char *)(ptrs[1]) - 2 != __brkval) return __LINE__;
 
     return 0;
 }

Added: trunk/avr-libc/tests/simulate/stdlib/realloc-2.c
===================================================================
--- trunk/avr-libc/tests/simulate/stdlib/realloc-2.c                            
(rev 0)
+++ trunk/avr-libc/tests/simulate/stdlib/realloc-2.c    2010-06-07 18:09:17 UTC 
(rev 2131)
@@ -0,0 +1,119 @@
+/* Copyright (c) 2010  Joerg Wunsch
+   All rights reserved.
+
+   Redistribution and use in source and binary forms, with or without
+   modification, are permitted provided that the following conditions are met:
+
+   * Redistributions of source code must retain the above copyright
+     notice, this list of conditions and the following disclaimer.
+   * Redistributions in binary form must reproduce the above copyright
+     notice, this list of conditions and the following disclaimer in
+     the documentation and/or other materials provided with the
+     distribution.
+   * Neither the name of the copyright holders nor the names of
+     contributors may be used to endorse or promote products derived
+     from this software without specific prior written permission.
+
+   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+   AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+   IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+   ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+   LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+   CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+   SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+   INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+   CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+   ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+   POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/* $Id$ */
+
+/* Like malloc-1.c, but after freeing everything, try to subsequently
+   allocate everything through a single pointer, and realloc(). */
+
+#ifndef        __AVR__
+
+/* There is no sense to check on host computer. */
+int main ()
+{
+    return 0;
+}
+
+#else
+
+#include <stdint.h>
+#include <stdlib.h>
+
+struct __freelist {
+        size_t sz;
+        struct __freelist *nx;
+};
+
+extern char *__brkval;          /* first location not yet allocated */
+extern struct __freelist *__flp; /* freelist pointer (head of freelist) */
+extern char *__malloc_heap_start;
+extern char *__malloc_heap_end;
+
+#if defined(__AVR_ATmega128__)
+static const int sizes[8] =
+{
+    5, 8, 2, 122, 256, 1, 32, 25 /* 451 + 1 [padding] + 2 * 8 [ptrs] = 468 */
+};
+#define TARGETVAL 468
+#elif defined(__AVR_AT90S8515__)
+static const int sizes[8] =
+{
+    5, 8, 2, 22, 256, 1, 12, 25 /* 331 + 1 [padding] + 2 * 8 [ptrs] = 348 */
+};
+#define TARGETVAL 348
+#else
+#  error "Unknown MCU type"
+#endif
+
+
+int main(void)
+{
+    void *ptrs[8];
+    uint8_t i;
+
+    for (i = 0; i < 8; i++)
+    {
+        void *p = malloc(sizes[i]);
+        /* first test: all allocations are supposed to fit */
+        if (p == NULL) return __LINE__;
+        ptrs[i] = p;
+    }
+    /* second test: the amount of memory allocated to the heap must
+       match the expected TARGETVAL */
+    if (__brkval - __malloc_heap_start != TARGETVAL) return __LINE__;
+
+    char *obrk = __brkval;
+
+    for (i = 0; i < 8; i++)
+    {
+        free(ptrs[i]);
+    }
+
+    /* after freeing everything, the freelist must be empty */
+    if (__flp != NULL) return __LINE__;
+    if (__brkval != __malloc_heap_start) return __LINE__;
+
+    size_t s = sizes[0];
+    void *p = malloc(s);
+    if (p == NULL) return __LINE__;
+    for (i = 1; i < 8; i++)
+    {
+        s += sizes[i];
+        void *q = realloc(p, s);
+        if (q == NULL) return __LINE__;
+        if (q != p) return __LINE__;
+    }
+
+    /* Verify the allocation length did not increase. */
+    if (__brkval > obrk) return __LINE__;
+
+    return 0;
+}
+
+#endif  /* !AVR */


Property changes on: trunk/avr-libc/tests/simulate/stdlib/realloc-2.c
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + Author Id Date
Added: svn:eol-style
   + native




reply via email to

[Prev in Thread] Current Thread [Next in Thread]