diff mbox

libbacktrace patch RFC: Look up variables in backtrace_syminfo

Message ID mcr38mxicf5.fsf@iant-glaptop.roam.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor Nov. 15, 2013, 9:26 p.m. UTC
Jakub asked whether it would be possible to extend backtrace_syminfo to
work for variables as well as functions.  It's a straightforward
extension, implemented by this patch.  Bootstrapped and ran libbacktrace
tests on x86_64-unknown-linux-gnu.  Any comments on this patch before I
submit it?

Ian


2013-11-15  Ian Lance Taylor  <iant@google.com>

	* backtrace.h (backtrace_syminfo): Update comment and parameter
	name to take any address, not just a PC value.
	* elf.c (STT_OBJECT): Define.
	(elf_nosyms): Rename parameter pc to addr.
	(elf_symbol_search): Rename local variable pc to addr.
	(elf_initialize_syminfo): Add STT_OBJECT symbols to elf_symbols.
	(elf_syminfo): Rename parameter pc to addr.
	* btest.c (global): New global variable.
	(test5): New test.
	(main): Call test5.

Comments

Jakub Jelinek Nov. 15, 2013, 9:34 p.m. UTC | #1
On Fri, Nov 15, 2013 at 01:26:54PM -0800, Ian Lance Taylor wrote:
> Jakub asked whether it would be possible to extend backtrace_syminfo to
> work for variables as well as functions.  It's a straightforward
> extension, implemented by this patch.  Bootstrapped and ran libbacktrace
> tests on x86_64-unknown-linux-gnu.  Any comments on this patch before I
> submit it?

Looks good to me.

OT,

   permanent buffer.  If THREADED is non-zero the state may be
   accessed by multiple threads simultaneously, and the library will
   use appropriate locks (this requires that the library be configured
   with --enable-backtrace-threads).  If THREADED is zero the state

in backtrace.h in backtrace_create_state comment doesn't look to be
up to date, there is no --enable-backtrace-threads it seems, just
depending on configure either it is thread safe or not (and doesn't use
locks).

	Jakub
diff mbox

Patch

Index: libbacktrace/elf.c
===================================================================
--- libbacktrace/elf.c	(revision 204717)
+++ libbacktrace/elf.c	(working copy)
@@ -101,6 +101,7 @@  dl_iterate_phdr (int (*callback) (struct
 #undef SHT_SYMTAB
 #undef SHT_STRTAB
 #undef SHT_DYNSYM
+#undef STT_OBJECT
 #undef STT_FUNC
 
 /* Basic types.  */
@@ -215,6 +216,7 @@  typedef struct
 
 #endif /* BACKTRACE_ELF_SIZE != 32 */
 
+#define STT_OBJECT 1
 #define STT_FUNC 2
 
 /* An index of ELF sections we care about.  */
@@ -293,7 +295,7 @@  elf_nodebug (struct backtrace_state *sta
 
 static void
 elf_nosyms (struct backtrace_state *state ATTRIBUTE_UNUSED,
-	    uintptr_t pc ATTRIBUTE_UNUSED,
+	    uintptr_t addr ATTRIBUTE_UNUSED,
 	    backtrace_syminfo_callback callback ATTRIBUTE_UNUSED,
 	    backtrace_error_callback error_callback, void *data)
 {
@@ -316,7 +318,7 @@  elf_symbol_compare (const void *v1, cons
     return 0;
 }
 
-/* Compare a PC against an elf_symbol for bsearch.  We allocate one
+/* Compare an ADDR against an elf_symbol for bsearch.  We allocate one
    extra entry in the array so that this can look safely at the next
    entry.  */
 
@@ -325,12 +327,12 @@  elf_symbol_search (const void *vkey, con
 {
   const uintptr_t *key = (const uintptr_t *) vkey;
   const struct elf_symbol *entry = (const struct elf_symbol *) ventry;
-  uintptr_t pc;
+  uintptr_t addr;
 
-  pc = *key;
-  if (pc < entry->address)
+  addr = *key;
+  if (addr < entry->address)
     return -1;
-  else if (pc >= entry->address + entry->size)
+  else if (addr >= entry->address + entry->size)
     return 1;
   else
     return 0;
@@ -360,7 +362,10 @@  elf_initialize_syminfo (struct backtrace
   elf_symbol_count = 0;
   for (i = 0; i < sym_count; ++i, ++sym)
     {
-      if ((sym->st_info & 0xf) == STT_FUNC)
+      int info;
+
+      info = sym->st_info & 0xf;
+      if (info == STT_FUNC || info == STT_OBJECT)
 	++elf_symbol_count;
     }
 
@@ -375,7 +380,10 @@  elf_initialize_syminfo (struct backtrace
   j = 0;
   for (i = 0; i < sym_count; ++i, ++sym)
     {
-      if ((sym->st_info & 0xf) != STT_FUNC)
+      int info;
+
+      info = sym->st_info & 0xf;
+      if (info != STT_FUNC && info != STT_OBJECT)
 	continue;
       if (sym->st_name >= strtab_size)
 	{
@@ -445,10 +453,10 @@  elf_add_syminfo_data (struct backtrace_s
     }
 }
 
-/* Return the symbol name and value for a PC.  */
+/* Return the symbol name and value for an ADDR.  */
 
 static void
-elf_syminfo (struct backtrace_state *state, uintptr_t pc,
+elf_syminfo (struct backtrace_state *state, uintptr_t addr,
 	     backtrace_syminfo_callback callback,
 	     backtrace_error_callback error_callback ATTRIBUTE_UNUSED,
 	     void *data)
@@ -463,7 +471,7 @@  elf_syminfo (struct backtrace_state *sta
 	   edata = edata->next)
 	{
 	  sym = ((struct elf_symbol *)
-		 bsearch (&pc, edata->symbols, edata->count,
+		 bsearch (&addr, edata->symbols, edata->count,
 			  sizeof (struct elf_symbol), elf_symbol_search));
 	  if (sym != NULL)
 	    break;
@@ -485,7 +493,7 @@  elf_syminfo (struct backtrace_state *sta
 	    break;
 
 	  sym = ((struct elf_symbol *)
-		 bsearch (&pc, edata->symbols, edata->count,
+		 bsearch (&addr, edata->symbols, edata->count,
 			  sizeof (struct elf_symbol), elf_symbol_search));
 	  if (sym != NULL)
 	    break;
@@ -495,9 +503,9 @@  elf_syminfo (struct backtrace_state *sta
     }
 
   if (sym == NULL)
-    callback (data, pc, NULL, 0);
+    callback (data, addr, NULL, 0);
   else
-    callback (data, pc, sym->name, sym->address);
+    callback (data, addr, sym->name, sym->address);
 }
 
 /* Add the backtrace data for one ELF file.  */
Index: libbacktrace/btest.c
===================================================================
--- libbacktrace/btest.c	(revision 204717)
+++ libbacktrace/btest.c	(working copy)
@@ -598,6 +598,53 @@  f33 (int f1line, int f2line)
   return failures;
 }
 
+int global = 1;
+
+static int
+test5 (void)
+{
+  struct symdata symdata;
+  int i;
+
+  symdata.name = NULL;
+  symdata.val = 0;
+  symdata.failed = 0;
+
+  i = backtrace_syminfo (state, (uintptr_t) &global, callback_three,
+			 error_callback_three, &symdata);
+  if (i == 0)
+    {
+      fprintf (stderr,
+	       "test5: unexpected return value from backtrace_syminfo %d\n",
+	       i);
+      symdata.failed = 1;
+    }
+
+  if (!symdata.failed)
+    {
+      if (symdata.name == NULL)
+	{
+	  fprintf (stderr, "test5: NULL syminfo name\n");
+	  symdata.failed = 1;
+	}
+      else if (strcmp (symdata.name, "global") != 0)
+	{
+	  fprintf (stderr,
+		   "test5: unexpected syminfo name got %s expected %s\n",
+		   symdata.name, "global");
+	  symdata.failed = 1;
+	}
+    }
+
+  printf ("%s: backtrace_syminfo variable\n",
+	  symdata.failed ? "FAIL" : "PASS");
+
+  if (symdata.failed)
+    ++failures;
+
+  return failures;
+}
+
 static void
 error_callback_create (void *data ATTRIBUTE_UNUSED, const char *msg,
 		       int errnum)
@@ -622,6 +669,7 @@  main (int argc ATTRIBUTE_UNUSED, char **
   test2 ();
   test3 ();
   test4 ();
+  test5 ();
 #endif
 
   exit (failures ? EXIT_FAILURE : EXIT_SUCCESS);
Index: libbacktrace/backtrace.h
===================================================================
--- libbacktrace/backtrace.h	(revision 204717)
+++ libbacktrace/backtrace.h	(working copy)
@@ -177,17 +177,17 @@  typedef void (*backtrace_syminfo_callbac
 					    const char *symname,
 					    uintptr_t symval);
 
-/* Given PC, a program counter in the current program, call the
-   callback information with the symbol name and value describing the
-   function in which PC may be found.  This will call either CALLBACK
-   or ERROR_CALLBACK exactly once.  This returns 1 on success, 0 on
-   failure.  This function requires the symbol table but does not
-   require the debug info.  Note that if the symbol table is present
-   but PC could not be found in the table, CALLBACK will be called
-   with a NULL SYMNAME argument.  Returns 1 on success, 0 on
-   error.  */
+/* Given ADDR, an address or program counter in the current program,
+   call the callback information with the symbol name and value
+   describing the function or variable in which ADDR may be found.
+   This will call either CALLBACK or ERROR_CALLBACK exactly once.
+   This returns 1 on success, 0 on failure.  This function requires
+   the symbol table but does not require the debug info.  Note that if
+   the symbol table is present but ADDR could not be found in the
+   table, CALLBACK will be called with a NULL SYMNAME argument.
+   Returns 1 on success, 0 on error.  */
 
-extern int backtrace_syminfo (struct backtrace_state *state, uintptr_t pc,
+extern int backtrace_syminfo (struct backtrace_state *state, uintptr_t addr,
 			      backtrace_syminfo_callback callback,
 			      backtrace_error_callback error_callback,
 			      void *data);