Remove the debugging memory allocator - valgrind does a better job
authorSimon Marlow <marlowsd@gmail.com>
Tue, 24 Aug 2010 11:35:37 +0000 (11:35 +0000)
committerSimon Marlow <marlowsd@gmail.com>
Tue, 24 Aug 2010 11:35:37 +0000 (11:35 +0000)
I got fed up with the constant bogus output from the debugging memory
allocator in RtsUtils.c.  One problem is that we allocate memory in
constructors that then isn't tracked, because the debugging allocator
hasn't been initialised yet.

The bigger problem is that for a given piece of leaking memory it's
impossible to find out where it was allocated; however Valgrind gives
output like this:

==6967== 8 bytes in 1 blocks are still reachable in loss record 1 of 7
==6967==    at 0x4C284A8: malloc (vg_replace_malloc.c:236)
==6967==    by 0x4C28522: realloc (vg_replace_malloc.c:525)
==6967==    by 0x6745E9: stgReallocBytes (RtsUtils.c:213)
==6967==    by 0x68D812: setHeapAlloced (MBlock.c:91)
==6967==    by 0x68D8E2: markHeapAlloced (MBlock.c:116)
==6967==    by 0x68DB56: getMBlocks (MBlock.c:240)
==6967==    by 0x684F55: alloc_mega_group (BlockAlloc.c:305)
==6967==    by 0x6850C8: allocGroup (BlockAlloc.c:358)
==6967==    by 0x69484F: allocNursery (Storage.c:390)
==6967==    by 0x694ABD: allocNurseries (Storage.c:436)
==6967==    by 0x6944F2: initStorage (Storage.c:217)
==6967==    by 0x673E3C: hs_init (RtsStartup.c:160)

which tells us exactly what the leaking bit of memory is.  So I don't
think we need our own debugging allocator.

rts/RtsStartup.c
rts/RtsUtils.c

index d7a8d95..6ed837a 100644 (file)
@@ -119,12 +119,6 @@ hs_init(int *argc, char **argv[])
      */
     stat_startInit();
 
-#if defined(DEBUG)
-    /* Start off by initialising the allocator debugging so we can
-     * use it anywhere */
-    initAllocator();
-#endif
-
     /* Set the RTS flags to default values. */
 
     initRtsFlagsDefaults();
@@ -456,11 +450,6 @@ hs_exit_(rtsBool wait_foreign)
     // heap memory (e.g. by being passed a ByteArray#).
     freeStorage(wait_foreign);
 
-#if defined(DEBUG)
-    /* and shut down the allocator debugging */
-    shutdownAllocator();
-#endif
-
 }
 
 // The real hs_exit():
index e9da59f..3df688f 100644 (file)
@@ -52,139 +52,6 @@ extern char *ctime_r(const time_t *, char *);
 #endif
 
 /* -----------------------------------------------------------------------------
-   Debugging allocator
-   -------------------------------------------------------------------------- */
-
-#if defined(DEBUG)
-
-typedef struct Allocated_ {
-    void *addr;
-    size_t len;
-    struct Allocated_ *next;
-} Allocated;
-
-static Allocated *allocs = NULL;
-
-#ifdef THREADED_RTS
-static Mutex allocator_mutex;
-#endif
-
-void
-initAllocator(void)
-{
-    Allocated *a;
-    size_t alloc_size;
-
-#ifdef THREADED_RTS
-    initMutex(&allocator_mutex);
-#endif
-    alloc_size = sizeof(Allocated);
-    if ((a = (Allocated *) malloc(alloc_size)) == NULL) {
-      /* don't fflush(stdout); WORKAROUND bug in Linux glibc */
-      MallocFailHook((W_) alloc_size, "initialising debugging allocator");
-      stg_exit(EXIT_INTERNAL_ERROR);
-    }
-    a->addr = NULL;
-    a->len = 0;
-    a->next = NULL;
-    allocs = a;
-}
-
-void
-shutdownAllocator(void)
-{
-    Allocated *prev, *a;
-
-    if (allocs == NULL) {
-        barf("Allocator shutdown requested, but not initialised!");
-    }
-
-#ifdef THREADED_RTS
-    closeMutex(&allocator_mutex);
-#endif
-
-    prev = allocs;
-    while (1) {
-        a = prev->next;
-        free(prev);
-        if (a == NULL) return;
-        IF_DEBUG(sanity,
-                 debugBelch("Warning: %ld bytes at %p still allocated at shutdown\n",
-                            (long)a->len, a->addr);)
-        prev = a;
-    }
-}
-
-static void addAllocation(void *addr, size_t len) {
-    Allocated *a;
-    size_t alloc_size;
-
-    if (allocs != NULL) {
-        alloc_size = sizeof(Allocated);
-        if ((a = (Allocated *) malloc(alloc_size)) == NULL) {
-          /* don't fflush(stdout); WORKAROUND bug in Linux glibc */
-          MallocFailHook((W_) alloc_size,
-                         "creating info for debugging allocator");
-          stg_exit(EXIT_INTERNAL_ERROR);
-        }
-        a->addr = addr;
-        a->len = len;
-        ACQUIRE_LOCK(&allocator_mutex);
-        a->next = allocs->next;
-        allocs->next = a;
-        RELEASE_LOCK(&allocator_mutex);
-    }
-    else {
-        /* This doesn't actually help as we haven't looked at the flags
-         * at the time that it matters (while running constructors) */
-        IF_DEBUG(sanity,
-                 debugBelch("Ignoring allocation %p %d as allocs is NULL\n",
-                            addr, (int)len);)
-    }
-}
-
-static void removeAllocation(void *addr, int overwrite_with_aa) {
-    Allocated *prev, *a;
-
-    if (addr == NULL) {
-        barf("Freeing NULL!");
-    }
-
-    if (allocs != NULL) {
-        ACQUIRE_LOCK(&allocator_mutex);
-        prev = allocs;
-        a = prev->next;
-        while (a != NULL) {
-            if (a->addr == addr) {
-                prev->next = a->next;
-                if (overwrite_with_aa) {
-                    memset(addr, 0xaa, a->len);
-                }
-                free(a);
-                RELEASE_LOCK(&allocator_mutex);
-                return;
-            }
-            prev = a;
-            a = a->next;
-        }
-        /* We would like to barf here, but we can't as conc021
-         * allocates some stuff in a constructor which then gets freed
-         * during hs_exit */
-        /* barf("Freeing non-allocated memory at %p", addr); */
-        IF_DEBUG(sanity,
-                 debugBelch("Warning: Freeing non-allocated memory at %p\n",
-                            addr);)
-        RELEASE_LOCK(&allocator_mutex);
-    }
-    else {
-        IF_DEBUG(sanity,
-                 debugBelch("Ignoring free of %p as allocs is NULL\n",
-                            addr);)
-    }
-}
-#endif
-
-/* -----------------------------------------------------------------------------
    Result-checking malloc wrappers.
    -------------------------------------------------------------------------- */
 
@@ -200,9 +67,6 @@ stgMallocBytes (int n, char *msg)
       MallocFailHook((W_) n, msg); /*msg*/
       stg_exit(EXIT_INTERNAL_ERROR);
     }
-#if defined(DEBUG)
-    addAllocation(space, n2);
-#endif
     return space;
 }
 
@@ -218,10 +82,6 @@ stgReallocBytes (void *p, int n, char *msg)
       MallocFailHook((W_) n, msg); /*msg*/
       stg_exit(EXIT_INTERNAL_ERROR);
     }
-#if defined(DEBUG)
-    if (p != NULL) { removeAllocation(p, 0); }
-    addAllocation(space, n2);
-#endif
     return space;
 }
 
@@ -235,9 +95,6 @@ stgCallocBytes (int n, int m, char *msg)
       MallocFailHook((W_) n*m, msg); /*msg*/
       stg_exit(EXIT_INTERNAL_ERROR);
     }
-#if defined(DEBUG)
-    addAllocation(space, (size_t) n * (size_t) m);
-#endif
     return space;
 }
 
@@ -247,9 +104,6 @@ stgCallocBytes (int n, int m, char *msg)
 void
 stgFree(void* p)
 {
-#if defined(DEBUG)
-  removeAllocation(p, 1);
-#endif
   free(p);
 }