diff mbox

libgo patch committed: Use libbacktrace

Message ID mcr3922fbjw.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Sept. 28, 2012, 2:48 p.m. UTC
This patch to libgo changes it to use libbacktrace.  Previously
backtraces required the Go package debug/elf to register itself with the
runtime during the package initialization, which only worked if the
program actually imported debug/elf one way or another.  Bootstrapped
and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
mainline.

Ian


2012-09-28  Ian Lance Taylor  <iant@google.com>

	* Makefile.def: Make all-target-libgo depend on
	all-target-libbacktrace.
	* Makefile.in: Rebuild.

Comments

Rainer Orth Oct. 4, 2012, 12:11 p.m. UTC | #1
Ian Lance Taylor <iant@google.com> writes:

> This patch to libgo changes it to use libbacktrace.  Previously
> backtraces required the Go package debug/elf to register itself with the
> runtime during the package initialization, which only worked if the
> program actually imported debug/elf one way or another.  Bootstrapped
> and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
> mainline.

Unfortunately, this breaks all use of libgo on versions of Solaris < 11
which lack strnlen:

Undefined                       first referenced
 symbol                             in file
strnlen                             /var/gcc/regression/trunk/10-gcc/build/i386-
pc-solaris2.10/libgo/.libs/libgo.so
ld: fatal: symbol referencing errors. No output written to a.out
collect2: error: ld returned 1 exit status
FAIL: bufio

One could either try to also link libiberty into libgo.la, but that has
the complication of needing to decide whether to use libiberty.a or
pic/libiberty.a since libiberty is no libtool library.

Alternatively, one could add another implementation of strnlen to libgo,
which duplicates code.

	Rainer
Ian Lance Taylor Oct. 4, 2012, 3:16 p.m. UTC | #2
On Thu, Oct 4, 2012 at 5:11 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> Ian Lance Taylor <iant@google.com> writes:
>
>> This patch to libgo changes it to use libbacktrace.  Previously
>> backtraces required the Go package debug/elf to register itself with the
>> runtime during the package initialization, which only worked if the
>> program actually imported debug/elf one way or another.  Bootstrapped
>> and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
>> mainline.
>
> Unfortunately, this breaks all use of libgo on versions of Solaris < 11
> which lack strnlen:
>
> Undefined                       first referenced
>  symbol                             in file
> strnlen                             /var/gcc/regression/trunk/10-gcc/build/i386-
> pc-solaris2.10/libgo/.libs/libgo.so
> ld: fatal: symbol referencing errors. No output written to a.out
> collect2: error: ld returned 1 exit status
> FAIL: bufio
>
> One could either try to also link libiberty into libgo.la, but that has
> the complication of needing to decide whether to use libiberty.a or
> pic/libiberty.a since libiberty is no libtool library.

I guess I won't try to link libgo against libiberty.  I just changed
libbacktrace to provide its own strnlen function, like so.
Bootstrapped and ran libbacktrace testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2012-10-04  Ian Lance Taylor  <iant@google.com>

	* dwarf.c: If the system header files do not declare strnlen,
	provide our own version.
diff mbox

Patch

diff -r 0126903cb089 libgo/Makefile.am
--- a/libgo/Makefile.am	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/Makefile.am	Fri Sep 28 07:22:51 2012 -0700
@@ -39,7 +39,8 @@ 
 
 AM_CFLAGS = -fexceptions -fplan9-extensions $(SPLIT_STACK) $(WARN_CFLAGS) \
 	$(STRINGOPS_FLAG) $(OSCFLAGS) \
-	-I $(srcdir)/../libgcc -I $(MULTIBUILDTOP)../../gcc/include
+	-I $(srcdir)/../libgcc -I $(srcdir)/../libbacktrace \
+	-I $(MULTIBUILDTOP)../../gcc/include
 
 if USING_SPLIT_STACK
 AM_LDFLAGS = -XCClinker $(SPLIT_STACK)
@@ -1062,8 +1063,7 @@ 
 	go/debug/dwarf/unit.go
 go_debug_elf_files = \
 	go/debug/elf/elf.go \
-	go/debug/elf/file.go \
-	go/debug/elf/runtime.go
+	go/debug/elf/file.go
 go_debug_gosym_files = \
 	go/debug/gosym/pclntab.go \
 	go/debug/gosym/symtab.go
@@ -1782,7 +1782,8 @@ 
 libgo_la_LDFLAGS = $(PTHREAD_CFLAGS) $(AM_LDFLAGS)
 
 libgo_la_LIBADD = \
-	$(libgo_go_objs) $(LIBFFI) $(PTHREAD_LIBS) $(MATH_LIBS) $(NET_LIBS)
+	$(libgo_go_objs) ../libbacktrace/libbacktrace.la \
+	$(LIBFFI) $(PTHREAD_LIBS) $(MATH_LIBS) $(NET_LIBS)
 
 libgobegin_a_SOURCES = \
 	runtime/go-main.c
diff -r 0126903cb089 libgo/go/debug/elf/elf_test.go
--- a/libgo/go/debug/elf/elf_test.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/debug/elf/elf_test.go	Fri Sep 28 07:22:51 2012 -0700
@@ -2,10 +2,9 @@ 
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package elf_test
+package elf
 
 import (
-	. "debug/elf"
 	"fmt"
 	"testing"
 )
diff -r 0126903cb089 libgo/go/debug/elf/file_test.go
--- a/libgo/go/debug/elf/file_test.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/debug/elf/file_test.go	Fri Sep 28 07:22:51 2012 -0700
@@ -2,11 +2,10 @@ 
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package elf_test
+package elf
 
 import (
 	"debug/dwarf"
-	. "debug/elf"
 	"encoding/binary"
 	"net"
 	"os"
diff -r 0126903cb089 libgo/go/debug/elf/runtime.go
--- a/libgo/go/debug/elf/runtime.go	Wed Sep 26 22:40:01 2012 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,161 +0,0 @@ 
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// This is gccgo-specific code that uses DWARF information to fetch
-// file/line information for PC values.  This package registers itself
-// with the runtime package.
-
-package elf
-
-import (
-	"debug/dwarf"
-	"debug/macho"
-	"os"
-	"runtime"
-	"sort"
-	"sync"
-)
-
-func init() {
-	// Register our lookup functions with the runtime package.
-	runtime.RegisterDebugLookup(funcFileLine, symbolValue)
-}
-
-// The file struct holds information for a specific file that is part
-// of the execution.
-type file struct {
-	elf   *File       // If ELF
-	macho *macho.File // If Mach-O
-	dwarf *dwarf.Data // DWARF information
-
-	symsByName []sym // Sorted by name
-	symsByAddr []sym // Sorted by address
-}
-
-// Sort symbols by name.
-type symsByName []sym
-
-func (s symsByName) Len() int           { return len(s) }
-func (s symsByName) Less(i, j int) bool { return s[i].name < s[j].name }
-func (s symsByName) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-
-// Sort symbols by address.
-type symsByAddr []sym
-
-func (s symsByAddr) Len() int           { return len(s) }
-func (s symsByAddr) Less(i, j int) bool { return s[i].addr < s[j].addr }
-func (s symsByAddr) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-
-// The sym structure holds the information we care about for a symbol,
-// namely name and address.
-type sym struct {
-	name string
-	addr uintptr
-}
-
-// Open an input file.
-func open(name string) (*file, error) {
-	efile, err := Open(name)
-	var mfile *macho.File
-	if err != nil {
-		var merr error
-		mfile, merr = macho.Open(name)
-		if merr != nil {
-			return nil, err
-		}
-	}
-
-	r := &file{elf: efile, macho: mfile}
-
-	if efile != nil {
-		r.dwarf, err = efile.DWARF()
-	} else {
-		r.dwarf, err = mfile.DWARF()
-	}
-	if err != nil {
-		return nil, err
-	}
-
-	var syms []sym
-	if efile != nil {
-		esyms, err := efile.Symbols()
-		if err != nil {
-			return nil, err
-		}
-		syms = make([]sym, 0, len(esyms))
-		for _, s := range esyms {
-			if ST_TYPE(s.Info) == STT_FUNC {
-				syms = append(syms, sym{s.Name, uintptr(s.Value)})
-			}
-		}
-	} else {
-		syms = make([]sym, 0, len(mfile.Symtab.Syms))
-		for _, s := range mfile.Symtab.Syms {
-			syms = append(syms, sym{s.Name, uintptr(s.Value)})
-		}
-	}
-
-	r.symsByName = make([]sym, len(syms))
-	copy(r.symsByName, syms)
-	sort.Sort(symsByName(r.symsByName))
-
-	r.symsByAddr = syms
-	sort.Sort(symsByAddr(r.symsByAddr))
-
-	return r, nil
-}
-
-// The main executable
-var executable *file
-
-// Only open the executable once.
-var executableOnce sync.Once
-
-func openExecutable() {
-	executableOnce.Do(func() {
-		f, err := open("/proc/self/exe")
-		if err != nil {
-			f, err = open(os.Args[0])
-			if err != nil {
-				return
-			}
-		}
-		executable = f
-	})
-}
-
-// The funcFileLine function looks up the function name, file name,
-// and line number for a PC value.
-func funcFileLine(pc uintptr, function *string, file *string, line *int) bool {
-	openExecutable()
-	if executable == nil || executable.dwarf == nil {
-		return false
-	}
-	f, ln, err := executable.dwarf.FileLine(uint64(pc))
-	if err != nil {
-		return false
-	}
-	*file = f
-	*line = ln
-
-	*function = ""
-	if len(executable.symsByAddr) > 0 && pc >= executable.symsByAddr[0].addr {
-		i := sort.Search(len(executable.symsByAddr),
-			func(i int) bool { return executable.symsByAddr[i].addr > pc })
-		*function = executable.symsByAddr[i-1].name
-	}
-
-	return true
-}
-
-// The symbolValue function fetches the value of a symbol.
-func symbolValue(name string, val *uintptr) bool {
-	i := sort.Search(len(executable.symsByName),
-		func(i int) bool { return executable.symsByName[i].name >= name })
-	if i >= len(executable.symsByName) || executable.symsByName[i].name != name {
-		return false
-	}
-	*val = executable.symsByName[i].addr
-	return true
-}
diff -r 0126903cb089 libgo/go/net/http/pprof/pprof.go
--- a/libgo/go/net/http/pprof/pprof.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/net/http/pprof/pprof.go	Fri Sep 28 07:22:51 2012 -0700
@@ -35,7 +35,6 @@ 
 import (
 	"bufio"
 	"bytes"
-	_ "debug/elf"
 	"fmt"
 	"html/template"
 	"io"
diff -r 0126903cb089 libgo/go/net/ip_test.go
--- a/libgo/go/net/ip_test.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/net/ip_test.go	Fri Sep 28 07:22:51 2012 -0700
@@ -6,7 +6,6 @@ 
 
 import (
 	"bytes"
-	_ "debug/elf"
 	"reflect"
 	"runtime"
 	"testing"
diff -r 0126903cb089 libgo/go/runtime/debug/stack.go
--- a/libgo/go/runtime/debug/stack.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/runtime/debug/stack.go	Fri Sep 28 07:22:51 2012 -0700
@@ -8,7 +8,6 @@ 
 
 import (
 	"bytes"
-	_ "debug/elf"
 	"fmt"
 	"io/ioutil"
 	"os"
diff -r 0126903cb089 libgo/go/runtime/pprof/pprof.go
--- a/libgo/go/runtime/pprof/pprof.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/runtime/pprof/pprof.go	Fri Sep 28 07:22:51 2012 -0700
@@ -11,7 +11,6 @@ 
 import (
 	"bufio"
 	"bytes"
-	_ "debug/elf"
 	"fmt"
 	"io"
 	"runtime"
diff -r 0126903cb089 libgo/go/testing/testing.go
--- a/libgo/go/testing/testing.go	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/go/testing/testing.go	Fri Sep 28 07:22:51 2012 -0700
@@ -79,7 +79,6 @@ 
 package testing
 
 import (
-	_ "debug/elf"
 	"flag"
 	"fmt"
 	"os"
diff -r 0126903cb089 libgo/runtime/go-caller.c
--- a/libgo/runtime/go-caller.c	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/runtime/go-caller.c	Fri Sep 28 07:22:51 2012 -0700
@@ -8,41 +8,99 @@ 
 
 #include <stdint.h>
 
+#include "backtrace.h"
+
 #include "runtime.h"
 #include "go-string.h"
 
 /* Get the function name, file name, and line number for a PC value.
-   We use the DWARF debug information to get this.  Rather than write
-   a whole new library in C, we use the existing Go library.
-   Unfortunately, the Go library is only available if the debug/elf
-   package is imported (we use debug/elf for both ELF and Mach-O, in
-   this case).  We arrange for the debug/elf package to register
-   itself, and tweak the various packages that need this information
-   to import debug/elf where possible.  */
+   We use the backtrace library to get this.  */
 
-/* The function that returns function/file/line information.  */
+/* Data structure to gather file/line information.  */
 
-typedef _Bool (*infofn_type) (uintptr_t, struct __go_string *,
-			      struct __go_string *, int *);
-static infofn_type infofn;
+struct caller
+{
+  struct __go_string fn;
+  struct __go_string file;
+  int line;
+};
 
-/* The function that returns the value of a symbol, used to get the
-   entry address of a function.  */
+/* Collect file/line information for a PC value.  If this is called
+   more than once, due to inlined functions, we use the last call, as
+   that is usually the most useful one.  */
 
-typedef _Bool (*symvalfn_type) (struct __go_string, uintptr_t *);
-static symvalfn_type symvalfn;
+static int
+callback (void *data, uintptr_t pc __attribute__ ((unused)),
+	  const char *filename, int lineno, const char *function)
+{
+  struct caller *c = (struct caller *) data;
 
-/* This is called by debug/elf to register the function that returns
-   function/file/line information.  */
+  if (function == NULL)
+    {
+      c->fn.__data = NULL;
+      c->fn.__length = 0;
+    }
+  else
+    {
+      char *s;
 
-void RegisterDebugLookup (infofn_type, symvalfn_type)
-  __asm__ ("runtime.RegisterDebugLookup");
+      c->fn.__length = __builtin_strlen (function);
+      s = runtime_malloc (c->fn.__length);
+      __builtin_memcpy (s, function, c->fn.__length);
+      c->fn.__data = (unsigned char *) s;
+    }
 
-void
-RegisterDebugLookup (infofn_type pi, symvalfn_type ps)
+  if (filename == NULL)
+    {
+      c->file.__data = NULL;
+      c->file.__length = 0;
+    }
+  else
+    {
+      char *s;
+
+      c->file.__length = __builtin_strlen (filename);
+      s = runtime_malloc (c->file.__length);
+      __builtin_memcpy (s, filename, c->file.__length);
+      c->file.__data = (unsigned char *) s;
+    }
+
+  c->line = lineno;
+
+  return 0;
+}
+
+/* The error callback for backtrace_pcinfo and backtrace_syminfo.  */
+
+static void
+error_callback (void *data __attribute__ ((unused)),
+		const char *msg, int errnum)
 {
-  infofn = pi;
-  symvalfn = ps;
+  if (errnum == -1)
+    return;
+  if (errnum > 0)
+    runtime_printf ("%s errno %d\n", msg, errnum);
+  runtime_throw (msg);
+}
+
+/* The backtrace library state.  */
+
+static void *back_state;
+
+/* A lock to control creating back_state.  */
+
+static Lock back_state_lock;
+
+/* Fetch back_state, creating it if necessary.  */
+
+struct backtrace_state *
+__go_get_backtrace_state ()
+{
+  runtime_lock (&back_state_lock);
+  if (back_state == NULL)
+    back_state = backtrace_create_state (NULL, 1, error_callback, NULL);
+  runtime_unlock (&back_state_lock);
+  return back_state;
 }
 
 /* Return function/file/line information for PC.  */
@@ -51,19 +109,38 @@ 
 __go_file_line (uintptr pc, struct __go_string *fn, struct __go_string *file,
 		int *line)
 {
-  if (infofn == NULL)
-    return 0;
-  return infofn (pc, fn, file, line);
+  struct caller c;
+
+  runtime_memclr (&c, sizeof c);
+  backtrace_pcinfo (__go_get_backtrace_state (), pc, callback,
+		    error_callback, &c);
+  *fn = c.fn;
+  *file = c.file;
+  *line = c.line;
+  return c.file.__length > 0;
 }
 
-/* Return the value of a symbol.  */
+/* Collect symbol information.  */
 
-_Bool
-__go_symbol_value (struct __go_string sym, uintptr_t *val)
+static void
+syminfo_callback (void *data, uintptr_t pc __attribute__ ((unused)),
+		  const char *symname __attribute__ ((unused)),
+		  uintptr_t address)
 {
-  if (symvalfn == NULL)
-    return 0;
-  return symvalfn (sym, val);
+  uintptr_t *pval = (uintptr_t *) data;
+
+  *pval = address;
+}
+
+/* Set *VAL to the value of the symbol for PC.  */
+
+static _Bool
+__go_symbol_value (uintptr_t pc, uintptr_t *val)
+{
+  *val = 0;
+  backtrace_syminfo (__go_get_backtrace_state (), pc, syminfo_callback,
+		     error_callback, &val);
+  return *val != 0;
 }
 
 /* The values returned by runtime.Caller.  */
@@ -112,12 +189,15 @@ 
 
   if (!__go_file_line (pc, &fn, &file, &line))
     return NULL;
-  if (!__go_symbol_value (fn, &val))
-    return NULL;
 
   ret = (Func *) runtime_malloc (sizeof (*ret));
   ret->name = fn;
-  ret->entry = val;
+
+  if (__go_symbol_value (pc, &val))
+    ret->entry = val;
+  else
+    ret->entry = 0;
+
   return ret;
 }
 
diff -r 0126903cb089 libgo/runtime/go-callers.c
--- a/libgo/runtime/go-callers.c	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/runtime/go-callers.c	Fri Sep 28 07:22:51 2012 -0700
@@ -6,64 +6,56 @@ 
 
 #include "config.h"
 
-#include "unwind.h"
+#include "backtrace.h"
 
 #include "runtime.h"
 
-/* Argument passed to backtrace function.  */
+/* Argument passed to callback function.  */
 
 struct callers_data
 {
-  int skip;
   uintptr *pcbuf;
   int index;
   int max;
 };
 
-static _Unwind_Reason_Code
-backtrace (struct _Unwind_Context *context, void *varg)
+/* Callback function for backtrace_simple.  Just collect the PC
+   values.  Return zero to continue, non-zero to stop.  */
+
+static int
+callback (void *data, uintptr_t pc)
 {
-  struct callers_data *arg = (struct callers_data *) varg;
-  uintptr pc;
-  int ip_before_insn = 0;
+  struct callers_data *arg = (struct callers_data *) data;
 
-#ifdef HAVE_GETIPINFO
-  pc = _Unwind_GetIPInfo (context, &ip_before_insn);
-#else
-  pc = _Unwind_GetIP (context);
-#endif
+  arg->pcbuf[arg->index] = pc;
+  ++arg->index;
+  return arg->index >= arg->max;
+}
 
-  /* FIXME: If PC is in the __morestack routine, we should ignore
-     it.  */
+/* Error callback.  */
 
-  if (arg->skip > 0)
-    --arg->skip;
-  else if (arg->index >= arg->max)
-    return _URC_END_OF_STACK;
-  else
-    {
-      /* Here PC will be the return address.  We actually want the
-	 address of the call instruction, so back up one byte and
-	 count on the lookup routines handling that correctly.  */
-      if (!ip_before_insn)
-	--pc;
-      arg->pcbuf[arg->index] = pc;
-      ++arg->index;
-    }
-  return _URC_NO_REASON;
+static void
+error_callback (void *data __attribute__ ((unused)),
+		const char *msg, int errnum)
+{
+  if (errnum != 0)
+    runtime_printf ("%s errno %d\n", msg, errnum);
+  runtime_throw (msg);
 }
 
+/* Gather caller PC's.  */
+
 int32
 runtime_callers (int32 skip, uintptr *pcbuf, int32 m)
 {
-  struct callers_data arg;
+  struct callers_data data;
 
-  arg.skip = skip + 1;
-  arg.pcbuf = pcbuf;
-  arg.index = 0;
-  arg.max = m;
-  _Unwind_Backtrace (backtrace, &arg);
-  return arg.index;
+  data.pcbuf = pcbuf;
+  data.index = 0;
+  data.max = m;
+  backtrace_simple (__go_get_backtrace_state (), skip + 1, callback,
+		    error_callback, &data);
+  return data.index;
 }
 
 int Callers (int, struct __go_open_array)
diff -r 0126903cb089 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h	Wed Sep 26 22:40:01 2012 -0700
+++ b/libgo/runtime/runtime.h	Fri Sep 28 07:22:51 2012 -0700
@@ -517,6 +517,8 @@ 
 // the stacks are allocated by the splitstack library.
 extern uintptr runtime_stacks_sys;
 
-extern _Bool __go_file_line (uintptr, String*, String*, int *);
+struct backtrace_state;
+extern struct backtrace_state *__go_get_backtrace_state(void);
+extern _Bool __go_file_line(uintptr, String*, String*, int *);
 
 int32 getproccount(void);
Index: Makefile.def
===================================================================
--- Makefile.def	(revision 191830)
+++ Makefile.def	(working copy)
@@ -491,6 +491,7 @@  dependencies = { module=configure-target
 dependencies = { module=all-target-fastjar; on=all-target-zlib; };
 dependencies = { module=configure-target-libgo; on=configure-target-libffi; };
 dependencies = { module=configure-target-libgo; on=all-target-libstdc++-v3; };
+dependencies = { module=all-target-libgo; on=all-target-libbacktrace; };
 dependencies = { module=all-target-libgo; on=all-target-libffi; };
 dependencies = { module=configure-target-libjava; on=configure-target-zlib; };
 dependencies = { module=configure-target-libjava; on=configure-target-boehm-gc; };