diff mbox

Use libbacktrace as libsanitizer's symbolizer

Message ID 20131119080410.GN892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 19, 2013, 8:04 a.m. UTC 
On Mon, Nov 18, 2013 at 09:09:03AM -0800, Ian Lance Taylor wrote:
> > 2) for tsan querying of data symbols, apparently the classes want to see
> >    not just the symbol name and start value, but also size.  libbacktrace
> >    has all this info available, just doesn't pass it down to the callback.
> >    I wonder if we'd need to create yet another libbacktrace entrypoint, or
> >    if it would be acceptable to do source code incompatible, ABI (at least
> >    on all sane targets) compatible version of just adding another
> >    uintptr_t symsize argument to backtrace_syminfo_callback.
> 
> I think it would be fine to change the callback.  I doubt that
> libbacktrace is so widely used that we need to worry about backward
> compatibility at this stage.  In particular I imagine that any users
> of libbacktrace are simply copying the source code, since there is no
> installable package.

So how about this?  Due to the CLA etc. I have not done the obvious change
to libgo/runtime/go-caller.c (syminfo_callback) that is needed together with
that.

2013-11-19  Jakub Jelinek  <jakub@redhat.com>

	* backtrace.h (backtrace_syminfo_callback): Add symsize argument.
	* elf.c (elf_syminfo): Pass 0 or sym->size to the callback as
	last argument.
	* btest.c (struct symdata): Add size field.
	(callback_three): Add symsize argument.  Copy it to the data->size
	field.
	(f23): Set symdata.size to 0.
	(test5): Likewise.  If sizeof (int) > 1, lookup address of
	((uintptr_t) &global) + 1.  Verify symdata.val and symdata.size
	values.



	Jakub

Comments

Ian Lance Taylor Nov. 19, 2013, 2:43 p.m. UTC | #1 
On Tue, Nov 19, 2013 at 12:04 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Nov 18, 2013 at 09:09:03AM -0800, Ian Lance Taylor wrote:
>> > 2) for tsan querying of data symbols, apparently the classes want to see
>> >    not just the symbol name and start value, but also size.  libbacktrace
>> >    has all this info available, just doesn't pass it down to the callback.
>> >    I wonder if we'd need to create yet another libbacktrace entrypoint, or
>> >    if it would be acceptable to do source code incompatible, ABI (at least
>> >    on all sane targets) compatible version of just adding another
>> >    uintptr_t symsize argument to backtrace_syminfo_callback.
>>
>> I think it would be fine to change the callback.  I doubt that
>> libbacktrace is so widely used that we need to worry about backward
>> compatibility at this stage.  In particular I imagine that any users
>> of libbacktrace are simply copying the source code, since there is no
>> installable package.
>
> So how about this?  Due to the CLA etc. I have not done the obvious change
> to libgo/runtime/go-caller.c (syminfo_callback) that is needed together with
> that.
>
> 2013-11-19  Jakub Jelinek  <jakub@redhat.com>
>
>         * backtrace.h (backtrace_syminfo_callback): Add symsize argument.
>         * elf.c (elf_syminfo): Pass 0 or sym->size to the callback as
>         last argument.
>         * btest.c (struct symdata): Add size field.
>         (callback_three): Add symsize argument.  Copy it to the data->size
>         field.
>         (f23): Set symdata.size to 0.
>         (test5): Likewise.  If sizeof (int) > 1, lookup address of
>         ((uintptr_t) &global) + 1.  Verify symdata.val and symdata.size
>         values.

This is OK.

Thanks.

I will take care of libgo when this is committed.

Ian
Jakub Jelinek Nov. 19, 2013, 2:46 p.m. UTC | #2 
On Tue, Nov 19, 2013 at 06:43:07AM -0800, Ian Lance Taylor wrote:
> > 2013-11-19  Jakub Jelinek  <jakub@redhat.com>
> >
> >         * backtrace.h (backtrace_syminfo_callback): Add symsize argument.
> >         * elf.c (elf_syminfo): Pass 0 or sym->size to the callback as
> >         last argument.
> >         * btest.c (struct symdata): Add size field.
> >         (callback_three): Add symsize argument.  Copy it to the data->size
> >         field.
> >         (f23): Set symdata.size to 0.
> >         (test5): Likewise.  If sizeof (int) > 1, lookup address of
> >         ((uintptr_t) &global) + 1.  Verify symdata.val and symdata.size
> >         values.
> 
> This is OK.
> 
> I will take care of libgo when this is committed.

Ok, thanks, in now (== r205028).

	Jakub
diff mbox

Patch

--- libbacktrace/backtrace.h.jj	2013-11-18 09:59:08.000000000 +0100
+++ libbacktrace/backtrace.h	2013-11-19 08:46:32.537927858 +0100
@@ -169,12 +169,13 @@  extern int backtrace_pcinfo (struct back
 /* The type of the callback argument to backtrace_syminfo.  DATA and
    PC are the arguments passed to backtrace_syminfo.  SYMNAME is the
    name of the symbol for the corresponding code.  SYMVAL is the
-   value.  SYMNAME will be NULL if no error occurred but the symbol
-   could not be found.  */
+   value and SYMSIZE is the size of the symbol.  SYMNAME will be NULL
+   if no error occurred but the symbol could not be found.  */
 
 typedef void (*backtrace_syminfo_callback) (void *data, uintptr_t pc,
 					    const char *symname,
-					    uintptr_t symval);
+					    uintptr_t symval,
+					    uintptr_t symsize);
 
 /* Given ADDR, an address or program counter in the current program,
    call the callback information with the symbol name and value
--- libbacktrace/elf.c.jj	2013-11-19 08:35:09.000000000 +0100
+++ libbacktrace/elf.c	2013-11-19 08:47:37.646598147 +0100
@@ -502,9 +502,9 @@  elf_syminfo (struct backtrace_state *sta
     }
 
   if (sym == NULL)
-    callback (data, addr, NULL, 0);
+    callback (data, addr, NULL, 0, 0);
   else
-    callback (data, addr, sym->name, sym->address);
+    callback (data, addr, sym->name, sym->address, sym->size);
 }
 
 /* Add the backtrace data for one ELF file.  */
--- libbacktrace/btest.c.jj	2013-11-18 09:59:08.000000000 +0100
+++ libbacktrace/btest.c	2013-11-19 08:56:29.320901588 +0100
@@ -92,7 +92,7 @@  struct sdata
 struct symdata
 {
   const char *name;
-  uintptr_t val;
+  uintptr_t val, size;
   int failed;
 };
 
@@ -238,7 +238,8 @@  error_callback_two (void *vdata, const c
 
 static void
 callback_three (void *vdata, uintptr_t pc ATTRIBUTE_UNUSED,
-		const char *symname, uintptr_t symval)
+		const char *symname, uintptr_t symval,
+		uintptr_t symsize)
 {
   struct symdata *data = (struct symdata *) vdata;
 
@@ -250,6 +251,7 @@  callback_three (void *vdata, uintptr_t p
       assert (data->name != NULL);
     }
   data->val = symval;
+  data->size = symsize;
 }
 
 /* The backtrace_syminfo error callback function.  */
@@ -458,6 +460,7 @@  f23 (int f1line, int f2line)
 
 	  symdata.name = NULL;
 	  symdata.val = 0;
+	  symdata.size = 0;
 	  symdata.failed = 0;
 
 	  i = backtrace_syminfo (state, addrs[j], callback_three,
@@ -605,12 +608,17 @@  test5 (void)
 {
   struct symdata symdata;
   int i;
+  uintptr_t addr = (uintptr_t) &global;
+
+  if (sizeof (global) > 1)
+    addr += 1;
 
   symdata.name = NULL;
   symdata.val = 0;
+  symdata.size = 0;
   symdata.failed = 0;
 
-  i = backtrace_syminfo (state, (uintptr_t) &global, callback_three,
+  i = backtrace_syminfo (state, addr, callback_three,
 			 error_callback_three, &symdata);
   if (i == 0)
     {
@@ -634,6 +642,22 @@  test5 (void)
 		   symdata.name, "global");
 	  symdata.failed = 1;
 	}
+      else if (symdata.val != (uintptr_t) &global)
+	{
+	  fprintf (stderr,
+		   "test5: unexpected syminfo value got %lx expected %lx\n",
+		   (unsigned long) symdata.val,
+		   (unsigned long) (uintptr_t) &global);
+	  symdata.failed = 1;
+	}
+      else if (symdata.size != sizeof (global))
+	{
+	  fprintf (stderr,
+		   "test5: unexpected syminfo size got %lx expected %lx\n",
+		   (unsigned long) symdata.size,
+		   (unsigned long) sizeof (global));
+	  symdata.failed = 1;
+	}
     }
 
   printf ("%s: backtrace_syminfo variable\n",