Patchwork libgo patch committed: Better backtraces

login
register
mail settings
Submitter Ian Taylor
Date Jan. 30, 2013, 10:24 p.m.
Message ID <mcrsj5iux5f.fsf@google.com>
Download mbox | patch
Permalink /patch/217002/
State New
Headers show

Comments

Ian Taylor - Jan. 30, 2013, 10:24 p.m.
This patch to libgo fixes backtrace information when optimizing.
Previously the backtraces just collected PC values and then converted
them into file/line information.  This of course works poorly when
functions are inlined.  This version uses the facilities of libbacktrace
to get good backtraces in the presence of inlined functions.  It also
skips the split-stack functions, which are not of interest when getting
a Go backtrace.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

Patch

diff -r 6edf35ba05b2 libgo/runtime/go-caller.c
--- a/libgo/runtime/go-caller.c	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/go-caller.c	Wed Jan 30 13:53:47 2013 -0800
@@ -166,16 +166,16 @@ 
 Caller (int skip)
 {
   struct caller_ret ret;
-  uintptr pc;
+  Location loc;
   int32 n;
-  String fn;
 
   runtime_memclr (&ret, sizeof ret);
-  n = runtime_callers (skip + 1, &pc, 1);
+  n = runtime_callers (skip + 1, &loc, 1);
   if (n < 1)
     return ret;
-  ret.pc = pc;
-  __go_file_line (pc, &fn, &ret.file, &ret.line);
+  ret.pc = loc.pc;
+  ret.file = loc.filename;
+  ret.line = loc.lineno;
   ret.ok = 1;
   return ret;
 }
diff -r 6edf35ba05b2 libgo/runtime/go-callers.c
--- a/libgo/runtime/go-callers.c	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/go-callers.c	Wed Jan 30 13:53:47 2013 -0800
@@ -15,20 +15,37 @@ 
 
 struct callers_data
 {
-  uintptr *pcbuf;
+  Location *locbuf;
   int index;
   int max;
 };
 
-/* Callback function for backtrace_simple.  Just collect the PC
-   values.  Return zero to continue, non-zero to stop.  */
+/* Callback function for backtrace_full.  Just collect the locations.
+   Return zero to continue, non-zero to stop.  */
 
 static int
-callback (void *data, uintptr_t pc)
+callback (void *data, uintptr_t pc, const char *filename, int lineno,
+	  const char *function)
 {
   struct callers_data *arg = (struct callers_data *) data;
+  Location *loc;
 
-  arg->pcbuf[arg->index] = pc;
+  /* Skip split stack functions.  */
+  if (function != NULL)
+    {
+      const char *p = function;
+
+      if (__builtin_strncmp (p, "___", 3) == 0)
+	++p;
+      if (__builtin_strncmp (p, "__morestack_", 12) == 0)
+	return 0;
+    }
+
+  loc = &arg->locbuf[arg->index];
+  loc->pc = pc;
+  loc->filename = runtime_gostring ((const byte *) filename);
+  loc->function = runtime_gostring ((const byte *) function);
+  loc->lineno = lineno;
   ++arg->index;
   return arg->index >= arg->max;
 }
@@ -47,15 +64,15 @@ 
 /* Gather caller PC's.  */
 
 int32
-runtime_callers (int32 skip, uintptr *pcbuf, int32 m)
+runtime_callers (int32 skip, Location *locbuf, int32 m)
 {
   struct callers_data data;
 
-  data.pcbuf = pcbuf;
+  data.locbuf = locbuf;
   data.index = 0;
   data.max = m;
-  backtrace_simple (__go_get_backtrace_state (), skip + 1, callback,
-		    error_callback, &data);
+  backtrace_full (__go_get_backtrace_state (), skip + 1, callback,
+		  error_callback, &data);
   return data.index;
 }
 
@@ -65,8 +82,19 @@ 
 int
 Callers (int skip, struct __go_open_array pc)
 {
+  Location *locbuf;
+  int ret;
+  int i;
+
+  locbuf = (Location *) runtime_mal (pc.__count * sizeof (Location));
+
   /* In the Go 1 release runtime.Callers has an off-by-one error,
      which we can not correct because it would break backward
      compatibility.  Adjust SKIP here to be compatible.  */
-  return runtime_callers (skip - 1, (uintptr *) pc.__values, pc.__count);
+  ret = runtime_callers (skip - 1, locbuf, pc.__count);
+
+  for (i = 0; i < ret; i++)
+    ((uintptr *) pc.__values)[i] = locbuf[i].pc;
+
+  return ret;
 }
diff -r 6edf35ba05b2 libgo/runtime/go-traceback.c
--- a/libgo/runtime/go-traceback.c	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/go-traceback.c	Wed Jan 30 13:53:47 2013 -0800
@@ -13,29 +13,25 @@ 
 void
 runtime_traceback ()
 {
-  uintptr pcbuf[100];
+  Location locbuf[100];
   int32 c;
 
-  c = runtime_callers (1, pcbuf, sizeof pcbuf / sizeof pcbuf[0]);
-  runtime_printtrace (pcbuf, c, true);
+  c = runtime_callers (1, locbuf, nelem (locbuf));
+  runtime_printtrace (locbuf, c, true);
 }
 
 void
-runtime_printtrace (uintptr *pcbuf, int32 c, bool current)
+runtime_printtrace (Location *locbuf, int32 c, bool current)
 {
   int32 i;
 
   for (i = 0; i < c; ++i)
     {
-      String fn;
-      String file;
-      intgo line;
-
-      if (__go_file_line (pcbuf[i], &fn, &file, &line)
-	  && runtime_showframe (fn, current))
+      if (runtime_showframe (locbuf[i].function, current))
 	{
-	  runtime_printf ("%S\n", fn);
-	  runtime_printf ("\t%S:%D\n", file, (int64) line);
+	  runtime_printf ("%S\n", locbuf[i].function);
+	  runtime_printf ("\t%S:%D\n", locbuf[i].filename,
+			  (int64) locbuf[i].lineno);
 	}
     }
 }
diff -r 6edf35ba05b2 libgo/runtime/mprof.goc
--- a/libgo/runtime/mprof.goc	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/mprof.goc	Wed Jan 30 13:53:47 2013 -0800
@@ -11,6 +11,7 @@ 
 #include "malloc.h"
 #include "defs.h"
 #include "go-type.h"
+#include "go-string.h"
 
 // NOTE(rsc): Everything here could use cas if contention became an issue.
 static Lock proflock;
@@ -46,7 +47,7 @@ 
 	};
 	uintptr	hash;
 	uintptr	nstk;
-	uintptr	stk[1];
+	Location stk[1];
 };
 enum {
 	BuckHashSize = 179999,
@@ -58,9 +59,9 @@ 
 
 // Return the bucket for stk[0:nstk], allocating new bucket if needed.
 static Bucket*
-stkbucket(int32 typ, uintptr *stk, int32 nstk, bool alloc)
+stkbucket(int32 typ, Location *stk, int32 nstk, bool alloc)
 {
-	int32 i;
+	int32 i, j;
 	uintptr h;
 	Bucket *b;
 
@@ -72,7 +73,7 @@ 
 	// Hash stack.
 	h = 0;
 	for(i=0; i<nstk; i++) {
-		h += stk[i];
+		h += stk[i].pc;
 		h += h<<10;
 		h ^= h>>6;
 	}
@@ -80,10 +81,18 @@ 
 	h ^= h>>11;
 
 	i = h%BuckHashSize;
-	for(b = buckhash[i]; b; b=b->next)
-		if(b->typ == typ && b->hash == h && b->nstk == (uintptr)nstk &&
-		   runtime_mcmp((byte*)b->stk, (byte*)stk, nstk*sizeof stk[0]) == 0)
-			return b;
+	for(b = buckhash[i]; b; b=b->next) {
+		if(b->typ == typ && b->hash == h && b->nstk == (uintptr)nstk) {
+			for(j = 0; j < nstk; j++) {
+				if(b->stk[j].pc != stk[j].pc ||
+				   b->stk[j].lineno != stk[j].lineno ||
+				   !__go_strings_equal(b->stk[j].filename, stk[j].filename))
+					break;
+			}
+			if (j == nstk)
+				return b;
+		}
+	}
 
 	if(!alloc)
 		return nil;
@@ -241,7 +250,7 @@ 
 {
 	M *m;
 	int32 nstk;
-	uintptr stk[32];
+	Location stk[32];
 	Bucket *b;
 
 	m = runtime_m();
@@ -298,7 +307,7 @@ 
 {
 	int32 nstk;
 	int64 rate;
-	uintptr stk[32];
+	Location stk[32];
 	Bucket *b;
 
 	if(cycles <= 0)
@@ -336,7 +345,7 @@ 
 	r->alloc_objects = b->allocs;
 	r->free_objects = b->frees;
 	for(i=0; i<b->nstk && i<nelem(r->stk); i++)
-		r->stk[i] = b->stk[i];
+		r->stk[i] = b->stk[i].pc;
 	for(; i<nelem(r->stk); i++)
 		r->stk[i] = 0;
 }
@@ -396,7 +405,7 @@ 
 			r->count = b->count;
 			r->cycles = b->cycles;
 			for(i=0; (uintptr)i<b->nstk && (uintptr)i<nelem(r->stk); i++)
-				r->stk[i] = b->stk[i];
+				r->stk[i] = b->stk[i].pc;
 			for(; (uintptr)i<nelem(r->stk); i++)
 				r->stk[i] = 0;			
 		}
@@ -413,6 +422,7 @@ 
 func ThreadCreateProfile(p Slice) (n int, ok bool) {
 	TRecord *r;
 	M *first, *mp;
+	int32 i;
 	
 	first = runtime_atomicloadp(&runtime_allm);
 	n = 0;
@@ -423,7 +433,9 @@ 
 		ok = true;
 		r = (TRecord*)p.__values;
 		for(mp=first; mp; mp=mp->alllink) {
-			runtime_memmove(r->stk, mp->createstack, sizeof r->stk);
+			for(i = 0; (uintptr)i < nelem(r->stk); i++) {
+				r->stk[i] = mp->createstack[i].pc;
+			}
 			r++;
 		}
 	}
@@ -473,10 +485,14 @@ 
 static void
 saveg(G *gp, TRecord *r)
 {
-	int32 n;
+	int32 n, i;
+	Location locstk[nelem(r->stk)];
 
-	if(gp == runtime_g())
-		n = runtime_callers(0, r->stk, nelem(r->stk));
+	if(gp == runtime_g()) {
+		n = runtime_callers(0, locstk, nelem(r->stk));
+		for(i = 0; i < n; i++)
+			r->stk[i] = locstk[i].pc;
+	}
 	else {
 		// FIXME: Not implemented.
 		n = 0;
diff -r 6edf35ba05b2 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/proc.c	Wed Jan 30 13:53:47 2013 -0800
@@ -631,7 +631,7 @@ 
 struct Traceback
 {
 	G* gp;
-	uintptr pcbuf[100];
+	Location locbuf[100];
 	int32 c;
 };
 
@@ -677,7 +677,7 @@ 
 			runtime_gogo(gp);
 		}
 
-		runtime_printtrace(tb.pcbuf, tb.c, false);
+		runtime_printtrace(tb.locbuf, tb.c, false);
 		runtime_goroutinetrailer(gp);
 	}
 }
@@ -692,8 +692,8 @@ 
 
 	traceback = gp->traceback;
 	gp->traceback = nil;
-	traceback->c = runtime_callers(1, traceback->pcbuf,
-		sizeof traceback->pcbuf / sizeof traceback->pcbuf[0]);
+	traceback->c = runtime_callers(1, traceback->locbuf,
+		sizeof traceback->locbuf / sizeof traceback->locbuf[0]);
 	runtime_gogo(traceback->gp);
 }
 
@@ -1742,13 +1742,14 @@ 
 	void (*fn)(uintptr*, int32);
 	int32 hz;
 	uintptr pcbuf[100];
+	Location locbuf[100];
 } prof;
 
 // Called if we receive a SIGPROF signal.
 void
 runtime_sigprof()
 {
-	int32 n;
+	int32 n, i;
 
 	if(prof.fn == nil || prof.hz == 0)
 		return;
@@ -1758,7 +1759,9 @@ 
 		runtime_unlock(&prof);
 		return;
 	}
-	n = runtime_callers(0, prof.pcbuf, nelem(prof.pcbuf));
+	n = runtime_callers(0, prof.locbuf, nelem(prof.locbuf));
+	for(i = 0; i < n; i++)
+		prof.pcbuf[i] = prof.locbuf[i].pc;
 	if(n > 0)
 		prof.fn(prof.pcbuf, n);
 	runtime_unlock(&prof);
diff -r 6edf35ba05b2 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/runtime.h	Wed Jan 30 13:53:47 2013 -0800
@@ -83,6 +83,8 @@ 
 
 typedef struct  Traceback	Traceback;
 
+typedef struct	Location	Location;
+
 /*
  * Per-CPU declaration.
  */
@@ -155,6 +157,16 @@ 
 	uint64	nosyield;
 	uint64	nsleep;
 };
+
+// A location in the program, used for backtraces.
+struct	Location
+{
+	uintptr	pc;
+	String	filename;
+	String	function;
+	intgo	lineno;
+};
+
 struct	G
 {
 	Defer*	defer;
@@ -226,7 +238,7 @@ 
 	MCache	*mcache;
 	G*	lockedg;
 	G*	idleg;
-	uintptr	createstack[32];	// Stack that created this thread.
+	Location createstack[32];	// Stack that created this thread.
 	M*	nextwaitm;	// next M waiting for lock
 	uintptr	waitsema;	// semaphore for parking on locks
 	uint32	waitsemacount;
@@ -391,7 +403,8 @@ 
 void	runtime_goroutinetrailer(G*);
 void	runtime_traceback();
 void	runtime_tracebackothers(G*);
-void	runtime_printtrace(uintptr*, int32, bool);
+void	runtime_printtrace(Location*, int32, bool);
+String	runtime_gostring(const byte*);
 String	runtime_gostringnocopy(const byte*);
 void*	runtime_mstart(void*);
 G*	runtime_malg(int32, byte**, size_t*);
@@ -406,7 +419,7 @@ 
 void	runtime_exitsyscall(void) __asm__ (GOSYM_PREFIX "syscall.Exitsyscall");
 void	siginit(void);
 bool	__go_sigsend(int32 sig);
-int32	runtime_callers(int32, uintptr*, int32);
+int32	runtime_callers(int32, Location*, int32);
 int64	runtime_nanotime(void);
 int64	runtime_cputicks(void);
 int64	runtime_tickspersecond(void);
diff -r 6edf35ba05b2 libgo/runtime/string.goc
--- a/libgo/runtime/string.goc	Tue Jan 29 17:30:28 2013 -0800
+++ b/libgo/runtime/string.goc	Wed Jan 30 13:53:47 2013 -0800
@@ -10,6 +10,8 @@ 
 
 #define charntorune(pv, str, len) __go_get_rune(str, len, pv)
 
+const String	runtime_emptystring;
+
 intgo
 runtime_findnull(const byte *s)
 {
@@ -18,6 +20,38 @@ 
 	return __builtin_strlen((const char*) s);
 }
 
+static String
+gostringsize(intgo l, byte** pmem)
+{
+	String s;
+	byte *mem;
+
+	if(l == 0) {
+		*pmem = nil;
+		return runtime_emptystring;
+	}
+	// leave room for NUL for C runtime (e.g., callers of getenv)
+	mem = runtime_mallocgc(l+1, FlagNoPointers, 1, 0);
+	s.str = mem;
+	s.len = l;
+	mem[l] = 0;
+	*pmem = mem;
+	return s;
+}
+
+String
+runtime_gostring(const byte *str)
+{
+	intgo l;
+	String s;
+	byte *mem;
+
+	l = runtime_findnull(str);
+	s = gostringsize(l, &mem);
+	runtime_memmove(mem, str, l);
+	return s;
+}
+
 String
 runtime_gostringnocopy(const byte *str)
 {