mtrace: record backtrace of memory allocation/deallocation

Message ID 154217315247.27039.4496343883523381963.stgit@kyeongy-dl.ws.atlnz.lc
State Superseded
Headers

Commit Message

Kyeong Yoo Nov. 14, 2018, 5:26 a.m. UTC
  When a memory leak is detected by mtrace, it is still difficult
to identify the exact source code causing that issue if the program
uses libraries or common functions.

Now a backtrace of the calling functions for memory allocation and
deallocation is also recorded. This new backtrace is displayed in
a separate line with '#' prefix prior to the original trace record
with '@' prefix.

This extra backtrace is turned off by default and only turned on
if valid number is set to the environment variable MALLOC_TRACE_LEVEL.
Valid value in MALLOC_TRACE_LEVEL is between 1 and 15, which indicates
the maximum number of recent function calls to display.

The companion mtrace.pl script is also updated to parse backtrace info.
And a new command argument "Maps" is added to mtrace.pl, which should
be a copy of the program's maps file ("/proc/PID/maps"). This helps
to interpret addresses from shared libraries.
---
 ChangeLog        |   17 ++++++++++
 malloc/mtrace.c  |   54 +++++++++++++++++++++++++++++++
 malloc/mtrace.pl |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 162 insertions(+), 3 deletions(-)
  

Comments

Joseph Myers Nov. 14, 2018, 5:30 p.m. UTC | #1
Some general observations:

* We'll need a copyright assignment (and employer assignment / disclaimer 
if applicable) on file at the FSF to consider this patch.

* A new feature needs documenting both in the glibc manual (memory.texi in 
this case) and with a NEWS file entry.

* A new feature should also have testcases added.

* An mtrace.pl option would be needed to specify the addr2line program to 
use, for cross-compilation cases.  That option would need to be used in 
the testcases.  See bug 23681 for discussion of the proper form configure 
tests for such binutils programs should take.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 149f991b70..07a40e2dfd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@ 
+2018-11-14  Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
+
+	* malloc/mtrace.c: New environment variable MALLOC_TRACE_LEVEL is
+	defined to enable backtrace output from mtrace(). If the value set
+	in MALLOC_TRACE_LEVEL is between 1 and 15 when mtrace() is called,
+	extra backtrace info is recored to the mtrace output.
+	* malloc/mtrace.c (tr_backtrace): Print backtrace of the function
+	calls up to the specified call level.
+	* malloc/mtrace.c (tr_freehook, tr_mallochook,tr_reallochook,
+	tr_memalignhook): Record backtrace if needed.
+	* malloc/mtrace.c (mtrace): Parse MALLOC_TRACE_LEVEL environment
+	variable. Valid number is between 1 and 15.
+	* malloc/mtrace.c (muntrace): Reset backtrace level to 0.
+	* malloc/mtrace.pl: Adjust to print backtrace of function calls.
+	* malloc/mtrace.pl: Accept new command argument for "Maps" which makes
+	possible to resolve addresses from shared libraries.
+
 2018-11-14  Samuel Thibault  <samuel.thibault@ens-lyon.org>
 
 	* sysdeps/mach/hurd/dl-sysdep.c (check_no_hidden): Use
diff --git a/malloc/mtrace.c b/malloc/mtrace.c
index 9064f209ec..10e6109dbc 100644
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -24,6 +24,7 @@ 
 # include <mcheck.h>
 # include <libc-lock.h>
 #endif
+#include <execinfo.h>
 
 #include <dlfcn.h>
 #include <fcntl.h>
@@ -44,8 +45,12 @@ 
 
 #define TRACE_BUFFER_SIZE 512
 
+#define MAX_BACKTRACE_LEVEL     15
+
 static FILE *mallstream;
 static const char mallenv[] = "MALLOC_TRACE";
+static const char mall_level_env[] = "MALLOC_TRACE_LEVEL";
+static int mall_trace_level = 0;
 static char *malloc_trace_buffer;
 
 __libc_lock_define_initialized (static, lock);
@@ -108,6 +113,26 @@  tr_where (const void *caller, Dl_info *info)
     }
 }
 
+static void
+tr_backtrace (void)
+{
+  void *bt_addrs[MAX_BACKTRACE_LEVEL + 2];
+  size_t bt_size;
+
+  bt_size = __backtrace (bt_addrs, mall_trace_level + 2);
+
+  /* Print backtrace (skip the first two) */
+  if (bt_size > 2)
+    {
+      size_t i;
+
+      fprintf (mallstream, "# %p", bt_addrs[2]);
+      for (i = 3; i < bt_size; i++)
+        fprintf (mallstream, ",%p", bt_addrs[i]);
+      fprintf (mallstream, "\n");
+    }
+}
+
 static Dl_info *
 lock_and_info (const void *caller, Dl_info *mem)
 {
@@ -127,6 +152,10 @@  tr_freehook (void *ptr, const void *caller)
   if (ptr == NULL)
     return;
 
+  /* Print backtrace */
+  if (mall_trace_level > 0)
+    tr_backtrace ();
+
   Dl_info mem;
   Dl_info *info = lock_and_info (caller, &mem);
   tr_where (caller, info);
@@ -160,6 +189,11 @@  tr_mallochook (size_t size, const void *caller)
     hdr = (void *) (*tr_old_malloc_hook)(size, caller);
   else
     hdr = (void *) malloc (size);
+
+  /* Print backtrace */
+  if (mall_trace_level > 0)
+    tr_backtrace ();
+
   __malloc_hook = tr_mallochook;
 
   tr_where (caller, info);
@@ -192,6 +226,11 @@  tr_reallochook (void *ptr, size_t size, const void *caller)
     hdr = (void *) (*tr_old_realloc_hook)(ptr, size, caller);
   else
     hdr = (void *) realloc (ptr, size);
+
+  /* Collect backtrace */
+  if (mall_trace_level > 0)
+    tr_backtrace ();
+
   __free_hook = tr_freehook;
   __malloc_hook = tr_mallochook;
   __realloc_hook = tr_reallochook;
@@ -236,6 +275,11 @@  tr_memalignhook (size_t alignment, size_t size, const void *caller)
     hdr = (void *) (*tr_old_memalign_hook)(alignment, size, caller);
   else
     hdr = (void *) memalign (alignment, size);
+
+  /* Collect backtrace */
+  if (mall_trace_level > 0)
+    tr_backtrace ();
+
   __memalign_hook = tr_memalignhook;
   __malloc_hook = tr_mallochook;
 
@@ -321,6 +365,15 @@  mtrace (void)
 			    __dso_handle);
             }
 #endif
+
+          /* Check backtrace level */
+          const char *level_str = getenv (mall_level_env);
+          if (level_str != NULL)
+            {
+              int num = atoi (level_str);
+              if (0 < num && num <= MAX_BACKTRACE_LEVEL)
+                mall_trace_level = num;
+            }
         }
       else
         free (mtb);
@@ -338,6 +391,7 @@  muntrace (void)
      file.  */
   FILE *f = mallstream;
   mallstream = NULL;
+  mall_trace_level = 0;
   __free_hook = tr_old_free_hook;
   __malloc_hook = tr_old_malloc_hook;
   __realloc_hook = tr_old_realloc_hook;
diff --git a/malloc/mtrace.pl b/malloc/mtrace.pl
index fe9f546000..69cb9b5480 100644
--- a/malloc/mtrace.pl
+++ b/malloc/mtrace.pl
@@ -25,8 +25,17 @@  $PKGVERSION = "@PKGVERSION@";
 $REPORT_BUGS_TO = '@REPORT_BUGS_TO@';
 $progname = $0;
 
+use Class::Struct;
+use File::Basename;
+
+struct Map => {
+    start   => '$',         # start address
+    end     => '$',         # end address
+    path    => '$',         # library path
+};
+
 sub usage {
-    print "Usage: mtrace [OPTION]... [Binary] MtraceData\n";
+    print "Usage: mtrace [OPTION]... [Binary] MtraceData [Maps]\n";
     print "  --help       print this help, then exit\n";
     print "  --version    print version number, then exit\n";
     print "\n";
@@ -35,9 +44,10 @@  sub usage {
     exit 0;
 }
 
-# We expect two arguments:
+# We expect three arguments:
 #   #1: the complete path to the binary
 #   #2: the mtrace data filename
+#   #3: the memory map of the process (/proc/PID/maps)
 # The usual options are also recognized.
 
 arglist: while (@ARGV) {
@@ -66,7 +76,7 @@  arglist: while (@ARGV) {
 if ($#ARGV == 0) {
     $binary="";
     $data=$ARGV[0];
-} elsif ($#ARGV == 1) {
+} elsif ($#ARGV == 1 or $#ARGV == 2) {
     $binary=$ARGV[0];
     $data=$ARGV[1];
 
@@ -84,10 +94,71 @@  if ($#ARGV == 0) {
 	}
 	close (LOCS);
     }
+
+    # Parse maps file
+    if (defined $ARGV[2]) {
+        open(MAPS, "<$ARGV[2]") || die "Cannot open maps file";
+        my $prog_base = basename($prog);
+        while (<MAPS>) {
+            chop;
+            my @cols = split (' ');
+            # Skip non-existing file or the program itself
+            if ($#cols == 5 and -e $cols[5] and basename($cols[5]) ne $prog_base) {
+                # Find executable memory map
+                if ($cols[1] =~ /^..x.$/ and $cols[0] =~ /^(.*)-(.*)$/) {
+                    my $m = Map->new();
+                    $m->start(hex($1));
+                    $m->end(hex($2));
+                    $m->path($cols[5]);
+                    push(@maps, $m);
+                }
+            }
+        }
+        close (MAPS);
+    }
 } else {
     die "Wrong number of arguments, run $progname --help for help.";
 }
 
+sub find_address_map {
+    my $addr = pop(@_);
+    foreach my $m (@maps) {
+        return $m if ($m->start <= $addr and $addr <= $m->end);
+    }
+    return undef;
+}
+
+sub print_backtrace {
+    foreach my $addr (split (',', pop(@_))) {
+        printf ("  # %-16s   %s\n", $addr, &addr2line(hex($addr)));
+    }
+}
+
+sub addr2line {
+    my $addr = pop(@_);
+    return $cache{$addr} if (exists $cache{$addr});
+
+    my $executable;
+    my $searchaddr;
+    my $m = find_address_map($addr);
+    if ($m) {
+        $executable = $m->path;
+        $searchaddr = sprintf ("%#x", $addr - $m->start);
+    } else {
+        $executable = $binary;
+        $searchaddr = sprintf ("%#x", $addr);
+    }
+    if ($executable ne "" && open (ADDR, "addr2line -e $executable $searchaddr|")) {
+        my $line = <ADDR>;
+        chomp $line;
+        close (ADDR);
+        $cache{$addr} = $line;
+        return $cache{$addr};
+    }
+    $cache{$addr} = "unknown";
+    return $cache{$addr};
+}
+
 sub location {
     my $str = pop(@_);
     return $str if ($str eq "");
@@ -144,6 +215,7 @@  sub location {
 }
 
 $nr=0;
+$bt="";
 open(DATA, "<$data") || die "Cannot open mtrace data file";
 while (<DATA>) {
     my @cols = split (' ');
@@ -167,9 +239,11 @@  while (<DATA>) {
 		printf ("+ %#0@XXX@x Alloc %d duplicate: %s %s\n",
 			hex($allocaddr), $nr, &location($addrwas{$allocaddr}),
 			$where);
+		&print_backtrace ($bt) if ($bt ne "");
 	    } elsif ($allocaddr =~ /^0x/) {
 		$allocated{$allocaddr}=$howmuch;
 		$addrwas{$allocaddr}=$where;
+		$backtrace{$allocaddr}=$bt if ($bt ne "");
 	    }
 	    last SWITCH;
 	}
@@ -177,9 +251,11 @@  while (<DATA>) {
 	    if (defined $allocated{$allocaddr}) {
 		undef $allocated{$allocaddr};
 		undef $addrwas{$allocaddr};
+		undef $backtrace{$allocaddr};
 	    } else {
 		printf ("- %#0@XXX@x Free %d was never alloc'd %s\n",
 			hex($allocaddr), $nr, &location($where));
+		&print_backtrace ($bt) if ($bt ne "");
 	    }
 	    last SWITCH;
 	}
@@ -187,9 +263,11 @@  while (<DATA>) {
 	    if (defined $allocated{$allocaddr}) {
 		undef $allocated{$allocaddr};
 		undef $addrwas{$allocaddr};
+		undef $backtrace{$allocaddr};
 	    } else {
 		printf ("- %#0@XXX@x Realloc %d was never alloc'd %s\n",
 			hex($allocaddr), $nr, &location($where));
+		&print_backtrace ($bt) if ($bt ne "");
 	    }
 	    last SWITCH;
 	}
@@ -198,9 +276,11 @@  while (<DATA>) {
 		printf ("+ %#0@XXX@x Realloc %d duplicate: %#010x %s %s\n",
 			hex($allocaddr), $nr, $allocated{$allocaddr},
 			&location($addrwas{$allocaddr}), &location($where));
+		&print_backtrace ($bt) if ($bt ne "");
 	    } else {
 		$allocated{$allocaddr}=$howmuch;
 		$addrwas{$allocaddr}=$where;
+		$backtrace{$allocaddr}=$bt if ($bt ne "");
 	    }
 	    last SWITCH;
 	}
@@ -213,6 +293,13 @@  while (<DATA>) {
 	    last SWITCH;
 	}
     }
+
+    # Save backtrace info (if any) for the next line
+    if ($cols[0] eq "#") {
+	$bt=$cols[1];
+    } else {
+	$bt="";
+    }
 }
 close (DATA);
 
@@ -229,6 +316,7 @@  if ($#addrs >= 0) {
 	    }
 	    printf ("%#0@XXX@x %#8x  at %s\n", hex($addr), $allocated{$addr},
 		    &location($addrwas{$addr}));
+	    &print_backtrace ($backtrace{$addr}) if (defined $backtrace{$addr});
 	}
     }
 }