Added rudimentary leak detection

Added rudimentary support for leak detection. When memory::tracker_enabled
is true, an alloc_tracker object (see alloctracker.{cc,hh}) object keeps
track of still-living memory allocations and the call chain that led to each.
Using a new command in gdb, "osv leak show", a developer can analyze the
remaining allocations, with the aim of finding memory leaks (noticing that
memory leaks often result in repeptitive allocations from the same call chain).

This implementation is a good start (and already found 8 leaks in our code),
but it's far from being perfect. It severely slows down the workload, the
analysis in gdb is not yet friendly enough (requiring manual inspection to
look for the more serious leaks), and the backtrace() implementation also
appears to be fragile: In more than one occasion, we noticed a yet-unexplained
crash when backtrace() unwinds the stack, calls dl_iterate_hphdr() which throws
exception and unwinds the stack again. So this code will probably need more
work in the future.

build.mak

@@ -218,6 +218,7 @@ objects += core/eventlist.o
 objects += core/debug.o
 objects += drivers/pci.o
 objects += core/mempool.o
+objects += core/alloctracker.o
 objects += arch/x64/elf-dl.o
 objects += linux.o
 objects += core/sched.o

core/alloctracker.cc

+#include <string.h>
+#include <stdlib.h>
+#include <execinfo.h>
+
+#include "alloctracker.hh"
+
+namespace memory {
+
+void alloc_tracker::remember(void *addr, int size)
+{
+    std::lock_guard<mutex> guard(lock);
+    // If the code in this method causes new allocations, they will in turn
+    // cause a nested call to this function. To avoid endless recursion, we
+    // do not track these deeper memory allocations. Remember that the
+    // purpose of the allocation tracking is finding memory leaks in the
+    // application code, not in the code in this file.
+    if (lock.getdepth() > 1) {
+        return;
+    }
+
+    struct alloc_info *a = nullptr;
+    for (size_t i = 0; i < size_allocations; i++) {
+        if(!allocations[i].addr){
+            // found a free spot, reuse it
+            a = &allocations[i];
+         }
+    }
+    if (!a) {
+        // Grow the vector to make room for more allocation records.
+        int old_size = size_allocations;
+        size_allocations = size_allocations ? 2*size_allocations : 1024;
+        struct alloc_info *old_allocations = allocations;
+        allocations = (struct alloc_info *) malloc(
+                size_allocations * sizeof(struct alloc_info));
+        if (old_allocations)
+            memcpy(allocations, old_allocations,
+                size_allocations * sizeof(struct alloc_info));
+        for (size_t i = old_size; i < size_allocations; i++)
+            allocations[i].addr=0;
+        a = &allocations[old_size];
+    }
+
+    a->seq = current_seq++;
+    a->addr = addr;
+    a->size = size;
+
+    // Do the backtrace. If we ask for only a small number of call levels
+    // we'll get only the deepest (most recent) levels, but we are more
+    // interested in the highest level functions, so we ask for 1024 levels
+    // (assuming we'll never have deeper recursion than that), and later only
+    // save the highest levels.
+    static void *bt[1024];
+    int n = backtrace(bt, sizeof(bt)/sizeof(*bt));
+
+    // When backtrace is too deep, save only the MAX_BACKTRACE most high
+    // level functions (not the first MAX_BACKTRACE functions!).
+    // We can ignore two functions at the start (always remember() itself
+    // and then malloc/alloc_page) and and at the end (typically some assembly
+    // code or ununderstood address).
+    static constexpr int N_USELESS_FUNCS_START = 2;
+    static constexpr int N_USELESS_FUNCS_END = 1;
+    void **bt_from = bt+N_USELESS_FUNCS_START;
+    n -= N_USELESS_FUNCS_START+N_USELESS_FUNCS_END;
+    if(n > MAX_BACKTRACE) {
+        bt_from += n - MAX_BACKTRACE;
+        n = MAX_BACKTRACE;
+    }
+    a->nbacktrace = n;
+    for (int i = 0; i < n; i++) {
+        a->backtrace[i] = bt_from[i];
+    }
+}
+
+void alloc_tracker::forget(void *addr)
+{
+    if (!addr)
+        return;
+    std::lock_guard<mutex> guard(lock);
+    for (size_t i = 0; i < size_allocations; i++){
+        if (allocations[i].addr == addr) {
+            allocations[i].addr = 0;
+            return;
+        }
+    }
+}
+
+}

core/mempool.cc

@@ -9,11 +9,31 @@
 #include "libc/libc.hh"
 #include "align.hh"
 #include <debug.hh>
+#include "alloctracker.hh"
 
 namespace memory {
 
 size_t phys_mem_size;
 
+// Optionally track living allocations, and the call chain which led to each
+// allocation. Don't set tracker_enabled before tracker is fully constructed.
+alloc_tracker tracker;
+bool tracker_enabled = false;
+static inline void tracker_remember(void *addr, size_t size)
+{
+    // Check if tracker_enabled is true, but expect (be quicker in the case)
+    // that it is false.
+    if (__builtin_expect(tracker_enabled, false)) {
+        tracker.remember(addr, size);
+    }
+}
+static inline void tracker_forget(void *addr)
+{
+    if (__builtin_expect(tracker_enabled, false)) {
+        tracker.forget(addr);
+    }
+}
+
 // Memory allocation strategy
 //
 // The chief requirement is to be able to deduce the object size.
@@ -33,7 +53,6 @@ size_t phys_mem_size;
 // from the same pool as large objects, except they don't have a header
 // (since we know the size already).
 
-
 pool::pool(unsigned size)
     : _size(size)
     , _free()
@@ -243,11 +262,13 @@ void* alloc_page()
     auto p = &*free_page_ranges.begin();
     if (p->size == page_size) {
         free_page_ranges.erase(*p);
+        tracker_remember(p, page_size);
         return p;
     } else {
         p->size -= page_size;
         void* v = p;
         v += p->size;
+        tracker_remember(v, page_size);
         return v;
     }
 }
@@ -255,6 +276,7 @@ void* alloc_page()
 void free_page(void* v)
 {
     free_page_range(v, page_size);
+    tracker_forget(v);
 }
 
 /* Allocate a huge page of a given size N (which must be a power of two)
@@ -292,6 +314,9 @@ void* alloc_huge_page(size_t N)
         }
         // Return the middle 2MB part
         return (void*) ret;
+        // TODO: consider using tracker.remember() for each one of the small
+        // pages allocated. However, this would be inefficient, and since we
+        // only use alloc_huge_page in one place, maybe not worth it.
     }
     // TODO: instead of aborting, tell the caller of this failure and have
     // it fall back to small pages instead.
@@ -361,14 +386,16 @@ void* malloc(size_t size)
 {
     if ((ssize_t)size < 0)
         return libc_error_ptr<void *>(ENOMEM);
-    
+    void *ret;
     if (size <= memory::pool::max_object_size) {
         size = std::max(size, memory::pool::min_object_size);
         unsigned n = ilog2_roundup(size);
-        return memory::malloc_pools[n].alloc();
+        ret = memory::malloc_pools[n].alloc();
     } else {
-        return memory::malloc_large(size);
+        ret = memory::malloc_large(size);
     }
+    memory::tracker_remember(ret, size);
+    return ret;
 }
 
 void* calloc(size_t nmemb, size_t size)
@@ -414,6 +441,7 @@ void free(void* object)
     if (!object) {
         return;
     }
+    memory::tracker_forget(object);
     if (reinterpret_cast<uintptr_t>(object) & (memory::page_size - 1)) {
         return memory::pool::from_object(object)->free(object);
     } else {

include/alloctracker.hh

+// alloc_tracker is used to track living allocations, and the call chain which
+// lead to each allocation. This is useful for leak detection.
+//
+// alloc_tracker implements only two methods: remember(addr, size), to
+// remember a new allocation, and forget(addr), to forget a previous
+// allocation. There is currently no programmatic API for querying this
+// data structure from within OSV. Instead, our gdb python extension (see
+// "loader.py") will find the memory::tracker object, inspect it, and report
+// the suspected leaks and/or other statistics about the leaving allocations.
+#ifndef INCLUDED_ALLOCTRACKER_H
+#define INCLUDED_ALLOCTRACKER_H
+#include <mutex.hh>
+#include <cstdint>
+
+namespace memory {
+
+class alloc_tracker {
+public:
+    void remember(void *addr, int size);
+    void forget(void *addr);
+private:
+    static constexpr int MAX_BACKTRACE = 20;
+    struct alloc_info {
+        // sequential number of allocation (to know how "old" this allocation
+        // is):
+        unsigned int seq;
+        // number of bytes allocated:
+        unsigned int size;
+        // allocation's address (addr==0 is a deleted item)
+        void *addr;
+        // the backtrace - MAX_BACKTRACE highest-level functions in the call
+        // chain that led to this allocation.
+        int nbacktrace;
+        void *backtrace[MAX_BACKTRACE];
+    };
+
+    // The current implementation for searching allocated blocks and remembering
+    // new ones is with a slow linear search. This is very slow when there are
+    // a lot of living allocations. It should be changed to a hash table!
+    struct alloc_info *allocations = 0;
+    size_t size_allocations = 0;
+    unsigned long current_seq = 0;
+    mutex lock;
+};
+#endif
+
+}

scripts/loader.py

@@ -411,12 +411,118 @@ def dump_trace():
                                               )
                       )
 
+def set_leak(val):
+    gdb.parse_and_eval('memory::tracker_enabled=%s' % val)
+
+def show_leak():
+    tracker = gdb.parse_and_eval('memory::tracker')
+    size_allocations = tracker['size_allocations']
+    allocations = tracker['allocations']
+    # Build a list of allocations to be sorted lexicographically by call chain
+    # and summarize allocations with the same call chain:
+    allocs = [];
+    for i in range(size_allocations) :
+        a = allocations[i]
+        addr = ulong(a['addr'])
+        if addr == 0 :
+            continue
+        nbacktrace = a['nbacktrace']
+        backtrace = a['backtrace']
+        callchain = []
+        for j in range(nbacktrace) :
+            callchain.append(backtrace[nbacktrace-1-j])
+        allocs.append((i, callchain))
+    allocs.sort(key=lambda entry: entry[1])
+    gdb.write('Allocations still in memory at this time (seq=%d):\n\n' % tracker['current_seq'])
+    total_size = 0
+    cur_n = 0
+    cur_total_size = 0
+    cur_total_seq = 0
+    cur_first_seq = -1
+    cur_last_seq = -1
+    cur_max_size = -1
+    cur_min_size = -1
+    for k, alloc in enumerate(allocs) :
+        i = alloc[0]
+        callchain = alloc[1]
+        seq = ulong(allocations[i]['seq'])
+        size = ulong(allocations[i]['size'])
+        total_size += size
+        cur_n += 1
+        cur_total_size += size
+        cur_total_seq += seq
+        if cur_first_seq<0 or seq<cur_first_seq :
+            cur_first_seq = seq
+        if cur_last_seq<0 or seq>cur_last_seq :
+            cur_last_seq = seq
+        if cur_min_size<0 or size<cur_min_size :
+            cur_min_size = size
+        if cur_max_size<0 or size>cur_max_size :
+            cur_max_size = size
+        # If the next entry has the same call chain, just continue summing
+        if k!=len(allocs)-1 and callchain==allocs[k+1][1] :
+            continue;
+        # We're done with a bunch of allocations with same call chain:
+        gdb.write('Found %d bytes in %d allocations [size ' % (cur_total_size, cur_n))
+        if cur_min_size != cur_max_size :
+            gdb.write('%d/%.1f/%d' % (cur_min_size, cur_total_size/cur_n, cur_max_size))
+        else :
+            gdb.write('%d' % cur_min_size)
+        gdb.write(', birth ')
+        if cur_first_seq != cur_last_seq :
+            gdb.write('%d/%.1f/%d' % (cur_first_seq, cur_total_seq/cur_n, cur_last_seq))
+        else :
+            gdb.write('%d' % cur_first_seq)
+        gdb.write(']\nfrom:\n')
+        cur_n = 0
+        cur_total_size = 0
+        cur_total_seq = 0
+        cur_first_seq = -1
+        cur_last_seq = -1
+        cur_max_size = -1
+        cur_min_size = -1
+        if len(callchain)==20 :
+            gdb.write('\t<deeper stack trace not remembered>\n')
+        for f in reversed(callchain) :
+            func_name = f
+            sal = gdb.find_pc_line(ulong(f))
+            try :
+                source = ' (%s:%s)' % (sal.symtab.filename, sal.line)
+            except :
+                source = ''
+            gdb.write('\t%s%s\n' % (func_name, source));
+        gdb.write('\n')
+
 class osv_trace(gdb.Command):
     def __init__(self):
         gdb.Command.__init__(self, 'osv trace', gdb.COMMAND_USER, gdb.COMPLETE_NONE)
     def invoke(self, arg, from_tty):
         dump_trace()
 
+class osv_leak(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, 'osv leak', gdb.COMMAND_USER,
+                             gdb.COMPLETE_COMMAND, True)
+
+class osv_leak_show(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, 'osv leak show', gdb.COMMAND_USER, gdb.COMPLETE_NONE)
+    def invoke(self, arg, from_tty):
+        show_leak()
+
+class osv_leak_on(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, 'osv leak on', gdb.COMMAND_USER, gdb.COMPLETE_NONE)
+    def invoke(self, arg, from_tty):
+        set_leak('true')
+
+class osv_leak_off(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, 'osv leak off', gdb.COMMAND_USER, gdb.COMPLETE_NONE)
+    def invoke(self, arg, from_tty):
+        set_leak('false')
+
+
 osv()
 osv_heap()
 osv_syms()
@@ -426,5 +532,9 @@ osv_thread()
 osv_thread_apply()
 osv_thread_apply_all()
 osv_trace()
+osv_leak()
+osv_leak_show()
+osv_leak_on()
+osv_leak_off()
 
 setup_libstdcxx()