* tmalloc: fixed memory leak (the irony!); refactoring; small improvements.

diff --git a/tmalloc/tmalloc.c b/tmalloc/tmalloc.c
index 8d4bdd9062da490db6968337f9a8db43a69b8b76..8269b5c80f004b3611b56901fad94d3e55129ce3 100644 (file)
--- a/tmalloc/tmalloc.c
+++ b/tmalloc/tmalloc.c
@@ -94,7 +94,7 @@ static inline void log_hex(uint64_t n) {
     write(tmalloc_log_fd, b, sizeof buf - (b - buf));
 }
 
-static inline void log_xdump(const void *s, size_t n) {
+static inline void log_xdump(const char *pfx, const void *s, size_t n) {
     const unsigned char *p = s;
     char buf[68];
     char *b = buf;
@@ -109,6 +109,7 @@ static inline void log_xdump(const void *s, size_t n) {
         ++i;
         if (i % 16 == 0) {
             *a++ = '\n';
+            log_str(pfx);
             write(tmalloc_log_fd, buf, a - buf);
             memset(buf, ' ', sizeof buf);
             b = buf;
@@ -122,6 +123,7 @@ static inline void log_xdump(const void *s, size_t n) {
     }
     if (b != buf) {
         *a++ = '\n';
+        log_str(pfx);
         write(tmalloc_log_fd, buf, a - buf);
     }
 }
@@ -147,7 +149,7 @@ static inline void tm_panic(void) {
  * Hash table to store information about allocated blocks of memory.
  */
 
-#define TM_HASHTAB_SIZE 1024
+#define TM_HASHTAB_SIZE 2048
 
 struct tm_info_struct {
     void *addr;
@@ -182,34 +184,14 @@ static inline uint32_t tm_hash(uint64_t k) {
     return hash % TM_HASHTAB_SIZE;
 }
 
-static inline int tm_ht_insert(void *addr, size_t size,
-                               void *raddr, size_t rsize, void *caller) {
-    struct tm_info_struct *e = __libc_malloc(sizeof *e);
-    uint32_t h;
+static inline struct tm_info_struct *tm_ht_lookup(void *addr) {
+    uint32_t h = tm_hash((uint64_t)addr);
 
-    if (!e) {
-        log_str("ERROR: failed to allocate internal block info struct");
-        tm_panic();
-    }
-    h = tm_hash((uint64_t)addr);
     for (struct tm_info_struct *p = tm_htab[h]; p; p = p->next) {
-        if (p->addr == addr) {
-            log_str("ERROR: block at");
-            log_hex((uint64_t)p->addr);  log_str(" (");
-            log_hex((uint64_t)p->raddr);  log_str(") ");
-            log_str("already listed as allocated");
-            tm_panic();
-            return -1;
-        }
+        if (p->addr == addr)
+            return p;
     }
-    e->addr = addr;
-    e->size = size;
-    e->raddr = raddr;
-    e->rsize = rsize;
-    e->caller = caller;
-    e->next = tm_htab[h];
-    tm_htab[h] = e;
-    return 0;
+    return NULL;
 }
 
 static inline int tm_ht_check_entry(struct tm_info_struct *p) {
@@ -240,26 +222,57 @@ static inline int tm_ht_check_entry(struct tm_info_struct *p) {
     return res;
 }
 
-static inline int tm_ht_update(void *addr, size_t size,
+static inline int tm_ht_insert(void *addr, size_t size,
                                void *raddr, size_t rsize, void *caller) {
     struct tm_info_struct *p;
     uint32_t h;
 
+    p = tm_ht_lookup(addr);
+    if (p) {
+        log_str("ERROR: block at");
+        log_hex((uint64_t)p->addr);  log_str(" (");
+        log_hex((uint64_t)p->raddr);  log_str(") ");
+        log_str("already listed as allocated");
+        tm_panic();
+        return -1;
+    }
+    p = __libc_malloc(sizeof *p);
+    if (!p) {
+        log_str("ERROR: failed to allocate internal block info struct");
+        tm_panic();
+        return -1;
+    }
+    memcpy(raddr, &tm_sentinel, TM_SENTSZ);
+    memcpy(raddr + rsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ);
     h = tm_hash((uint64_t)addr);
-    for (p = tm_htab[h]; p; p = p->next) {
-        if (p->addr == addr) {
-            p->size = size;
-            p->raddr = raddr;
-            p->rsize = rsize;
-            p->caller = caller;
-            return 0;
-        }
+    p->addr = addr;
+    p->size = size;
+    p->raddr = raddr;
+    p->rsize = rsize;
+    p->caller = caller;
+    p->next = tm_htab[h];
+    tm_htab[h] = p;
+    return 0;
+}
+
+static inline int tm_ht_update(void *addr, size_t size,
+                               void *raddr, size_t rsize, void *caller) {
+    struct tm_info_struct *p;
+
+    p = tm_ht_lookup(addr);
+    if (!p) {
+        log_str(TM_LOG_PFX"ERROR: attempt to update block at ");
+        log_hex((uint64_t)addr);
+        log_str(", not listed as allocated!\n");
+        tm_panic();
+        return -1;
     }
-    log_str(TM_LOG_PFX"ERROR: attempt to update block at ");
-    log_hex((uint64_t)addr);
-    log_str(", not listed as allocated!\n");
-    tm_panic();
-    return -1;
+    memcpy(raddr + rsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ);
+    p->size = size;
+    p->raddr = raddr;
+    p->rsize = rsize;
+    p->caller = caller;
+    return 0;
 }
 
 static inline int tm_ht_remove(void *addr, int wipe) {
@@ -269,12 +282,16 @@ static inline int tm_ht_remove(void *addr, int wipe) {
     h = tm_hash((uint64_t)addr);
     for (p = tm_htab[h]; p; p = p->next) {
         if (p->addr == addr) {
-            if (prev == NULL)
+            if (!prev)
                 tm_htab[h] = p->next;
             else
                 prev->next = p->next;
-            if (wipe)
+            if (wipe) {
+                /* NOTE: Only check when wiping, else there be dragons! */
+                tm_ht_check_entry(p);
                 memset(p->raddr, '\x55', p->rsize);
+            }
+            __libc_free(p);
             return 0;
         }
         prev = p;
@@ -286,36 +303,18 @@ static inline int tm_ht_remove(void *addr, int wipe) {
     return -1;
 }
 
-static inline int tm_ht_check(void *addr) {
-    struct tm_info_struct *p;
-    uint32_t h;
-
-    h = tm_hash((uint64_t)addr);
-    for (p = tm_htab[h]; p; p = p->next) {
-        if (p->addr == addr) {
-            return tm_ht_check_entry(p);
-        }
-    }
-    log_str(TM_LOG_PFX"ERROR: attempt to check block at ");
-    log_hex((uint64_t)addr);
-    log_str(", not listed as allocated!\n");
-    tm_panic();
-    return -1;
-}
-
 static inline int tm_ht_checkall(void) {
-    size_t i, n;
+    size_t i;
+    int res = 0;
     struct tm_info_struct *p;
 
-    for (i = n = 0; i < TM_HASHTAB_SIZE; ++i) {
-        for (p = tm_htab[i]; p; p = p->next) {
-            if (tm_ht_check_entry(p) != 0)
-                ++n;
-        }
+    for (i = 0; i < TM_HASHTAB_SIZE; ++i) {
+        for (p = tm_htab[i]; p; p = p->next)
+            res -= tm_ht_check_entry(p);
     }
     log_str(TM_LOG_PFX"CHECK: ");
-    log_dec(n);  log_str(" corrupted block(s) found.\n");
-    return n ? -1 : 0;
+    log_dec(res);  log_str(" corrupted block(s) found.\n");
+    return res;
 }
 
 static inline void tm_ht_dump(void) {
@@ -334,7 +333,7 @@ static inline void tm_ht_dump(void) {
                 log_str("\n");
             }
             if (tmalloc_log_level >= TM_LOG_DEBUG) {
-                log_xdump(p->addr, p->size > 64 ? 64 : p->size);
+                log_xdump("  ", p->addr, p->size > 64 ? 64 : p->size);
             }
             tm_ht_check_entry(p);
             ++n;
@@ -355,7 +354,7 @@ static inline void tm_ht_dump(void) {
 #ifdef __GNUC__
     #define ret_address(l_) __builtin_return_address(l_)
 #else
-    #define ret_address(l_) ((void *)0xdeadbeefcafebabeULL)
+    #define ret_address(l_) ((void *)0xdefecatedeadf00dULL)
 #endif
 
 void *malloc(size_t size) {
@@ -363,18 +362,16 @@ void *malloc(size_t size) {
     void *addr;
     size_t xsize = size + (TM_SENTSZ * 2);
 
-    TM_HT_LOCK();
     addr = __libc_malloc(xsize);
     if (addr) {
-        memcpy(addr, &tm_sentinel, TM_SENTSZ);
-        memcpy(addr + xsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ);
+        TM_HT_LOCK();
         tm_ht_insert(addr + TM_SENTSZ, size, addr, xsize, caller);
+        TM_HT_UNLOCK();
         addr += TM_SENTSZ;
     }
     else if (tmalloc_log_level >= TM_LOG_WARNING) {
         log_str(TM_LOG_PFX"WARNING: __libc_malloc() returned NULL pointer!\n");
     }
-    TM_HT_UNLOCK();
 
     if (tmalloc_log_level >= TM_LOG_TRACE) {
         log_str(TM_LOG_PFX"TRACE: malloc(");  log_dec(size);
@@ -390,18 +387,16 @@ void *calloc(size_t nmemb, size_t size) {
     void *addr;
     size_t xnmemb = nmemb + ((TM_SENTSZ - 1) / size + 1) * 2;
 
-    TM_HT_LOCK();
     addr = __libc_calloc(xnmemb, size);
     if (addr) {
-        memcpy(addr, &tm_sentinel, TM_SENTSZ);
-        memcpy(addr + xnmemb * size - TM_SENTSZ, &tm_sentinel, sizeof tm_sentinel);
+        TM_HT_LOCK();
         tm_ht_insert(addr + TM_SENTSZ, nmemb * size, addr, xnmemb * size, caller);
+        TM_HT_UNLOCK();
         addr += TM_SENTSZ;
     }
     else if (tmalloc_log_level >= TM_LOG_WARNING) {
         log_str(TM_LOG_PFX"WARNING: __libc_calloc() returned NULL pointer!\n");
     }
-    TM_HT_UNLOCK();
 
     if (tmalloc_log_level >= TM_LOG_TRACE) {
         log_str(TM_LOG_PFX"TRACE: calloc(");
@@ -419,25 +414,25 @@ void *realloc(void *ptr, size_t size) {
     void *oaddr = ptr ? ptr - TM_SENTSZ : NULL;
     size_t xsize = size + (TM_SENTSZ * 2);
 
-    TM_HT_LOCK();
     addr = __libc_realloc(oaddr, xsize);
     if (addr) {
+        TM_HT_LOCK();
         if (addr == oaddr) {
             tm_ht_update(ptr, size, addr, xsize, caller);
         }
         else {
-            if (ptr)
+            if (ptr) {
+                /* NOTE: Do not wipe, it's not ours anymore! */
                 tm_ht_remove(ptr, 0);
+            }
             tm_ht_insert(addr + TM_SENTSZ, size, addr, xsize, caller);
-            memcpy(addr, &tm_sentinel, TM_SENTSZ);
         }
-        memcpy(addr + xsize - TM_SENTSZ, &tm_sentinel, sizeof tm_sentinel);
+        TM_HT_UNLOCK();
         addr += TM_SENTSZ;
     }
     else if (tmalloc_log_level >= TM_LOG_WARNING) {
         log_str(TM_LOG_PFX"WARNING: __libc_realloc() returned NULL pointer!\n");
     }
-    TM_HT_UNLOCK();
 
     if (tmalloc_log_level >= TM_LOG_TRACE) {
         log_str(TM_LOG_PFX"TRACE: realloc(");
@@ -460,15 +455,14 @@ void free(void *ptr) {
 
     if (ptr) {
         TM_HT_LOCK();
-        tm_ht_check(ptr);
         if (tm_ht_remove(ptr, tmalloc_wipe_on_free) != 0) {
             log_str(TM_LOG_PFX"ERROR: possible double free(");
             log_hex((uint64_t)ptr);
             log_str("): *** HEAP IS VERY LIKELY CORRUPT! ***\n");
             tm_panic();
         }
-        __libc_free(ptr - TM_SENTSZ);
         TM_HT_UNLOCK();
+        __libc_free(ptr - TM_SENTSZ);
     }
     else if (tmalloc_log_level >= TM_LOG_INFO) {
         log_str(TM_LOG_PFX"INFO: free() called with NULL pointer argument!\n");
@@ -497,10 +491,13 @@ void tmalloc_set_wipe_on_free(int w) {
     tmalloc_wipe_on_free = w;
 }
 
-void tmalloc_check(void) {
+int tmalloc_check(void) {
+    int res = 0;
+
     TM_HT_LOCK();
-    tm_ht_checkall();
+    res = tm_ht_checkall();
     TM_HT_UNLOCK();
+    return res;
 }
 
 void tmalloc_stat(void) {

diff --git a/tmalloc/tmalloc.h b/tmalloc/tmalloc.h
index 2c404c4d10d152b9c1775202d71b0161851191cf..7dbef6ca11cd2a21a36b76a43e504c2b73639742 100644 (file)
--- a/tmalloc/tmalloc.h
+++ b/tmalloc/tmalloc.h
@@ -54,8 +54,10 @@ void tmalloc_set_wipe_on_free(int w);
 /*
  * Check all known allocated blocks for intact sentinel values
  * and report any problems detected.
+ *
+ * Returns the number of corrupt blocks found.
  */
-extern void tmalloc_check(void);
+extern int tmalloc_check(void);
 
 /*
  * Print an overview of all known allocated blocks.