From ea0bfdada955e3f5de8c38b06c831f6dc64ba0f2 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Tue, 24 Aug 2010 11:35:37 +0000 Subject: [PATCH] Remove the debugging memory allocator - valgrind does a better job 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 | 11 ---- rts/RtsUtils.c | 146 ------------------------------------------------------ 2 files changed, 157 deletions(-) diff --git a/rts/RtsStartup.c b/rts/RtsStartup.c index d7a8d95..6ed837a 100644 --- a/rts/RtsStartup.c +++ b/rts/RtsStartup.c @@ -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(): diff --git a/rts/RtsUtils.c b/rts/RtsUtils.c index e9da59f..3df688f 100644 --- a/rts/RtsUtils.c +++ b/rts/RtsUtils.c @@ -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); } -- 1.7.10.4