[PATCH] Handle PIEs in libbacktrace

[PATCH] Handle PIEs in libbacktrace

[Jakub Jelinek]

Hi!

With the new tsan tests, I've noticed that libbacktrace symbolization
doesn't work when the binary is a PIE.
The problem is that in that case we obviously can't use base_address
of 0, the PIE typically will not have 0 bias, that is actually the sole
point of PIEs that their base address is randomized.  So, in that case
we need to wait until dl_iterate_phdr to determine the proper bias.
The executable is always the first in the search scope and/or list of
loaded modules, so we just check if the first callback gives us a dlpi_name
== NULL entry and assume it is the PIE.

Regtested on x86_64-linux, ok for trunk?

The rest of this mail except the libbacktrace patch is for the sanitizer
folks.

With this patch check-gcc tsan.exp passes fully again, but check-g++ does
not, there seems to be important differences between tsan symbolization
and say asan symbolization.  First of all, if symbolizer fails (as without
this patch for tsan because the executables are PIEs), we get something
like:
WARNING: ThreadSanitizer: data race (pid=18998)
  Write of size 4 at 0x7f4a88f0a08c by thread T1:
    #0 <null> <null>:0 (tiny_race+0x000000000b19)

  Previous write of size 4 at 0x7f4a88f0a08c by main thread:
    #0 <null> <null>:0 (tiny_race+0x000000000b7a)
    #1 __libc_start_main <null>:0 (libc.so.6+0x003c08c21b74)

  As if synchronized via sleep:
    #0 sleep ../../../../libsanitizer/tsan/tsan_interceptors.cc:239 (libtsan.so.0+0x000000048c65)
    #1 <null> <null>:0 (tiny_race+0x000000000b0a)

  Location is global '<null>' of size 0 at 0x000000000000 (tiny_race+0x00000020208c)

  Thread T1 (tid=19000, running) created by main thread at:
    #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
    #1 <null> <null>:0 (tiny_race+0x000000000b6b)
    #2 __libc_start_main <null>:0 (libc.so.6+0x003c08c21b74)

The <null> <null>:0 or <null>:0 look very ugly, e.g. asan
I think just prints the exe/dso name + offset in that case.

And the reason why check-g++ tsan.exp fails even with this patch is
that apparently tsan doesn't try to demangle the symbol names, so we get
e.g.:
FAIL: c-c++-common/tsan/atomic_stack.c  -O0  output pattern test, is ==================
WARNING: ThreadSanitizer: data race (pid=22075)
  Atomic write of size 4 at 0x7fe8c9b630ac by thread T1:
    #0 __tsan_atomic32_fetch_add ../../../../libsanitizer/tsan/tsan_interface_atomic.cc:468 (libtsan.so.0+0x00000001ec7e)
    #1 _Z7Thread1Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:11 (atomic_stack.exe+0x000000000d90)

  Previous write of size 4 at 0x7fe8c9b630ac by thread T2:
    #0 _Z7Thread2Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:16 (atomic_stack.exe+0x000000000dde)

  As if synchronized via sleep:
    #0 sleep ../../../../libsanitizer/tsan/tsan_interceptors.cc:239 (libtsan.so.0+0x000000048c65)
    #1 _Z7Thread1Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:10 (atomic_stack.exe+0x000000000d7a)

  Location is global 'Global' of size 4 at 0x7fe8c9b630ac (atomic_stack.exe+0x0000002020ac)

  Thread T1 (tid=22080, running) created by main thread at:
    #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
    #1 main /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:22 (atomic_stack.exe+0x000000000e2a)

  Thread T2 (tid=22081, finished) created by main thread at:
    #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
    #1 main /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:23 (atomic_stack.exe+0x000000000e4b)

Is the non-demangling intentional?  If yes, I guess we'd need to adjust
the testcases to cope with both mangled and non-mangled names, otherwise
please change tsan to demangle, the symbolizer has functions for it.

2013-12-06  Jakub Jelinek  <jakub@redhat.com>

	* elf.c (ET_DYN): Undefine and define again.
	(elf_add): Add exe argument, if true and ehdr.e_type is ET_DYN,
	return early -1 without closing the descriptor.
	(struct phdr_data): Add exe_descriptor.
	(phdr_callback): If pd->exe_descriptor is not -1, for very first
	call if dlpi_name is NULL just call elf_add with the exe_descriptor,
	otherwise backtrace_close the exe_descriptor if not -1.  Adjust
	call to elf_add.
	(backtrace_initialize): Adjust call to elf_add.  If it returns
	-1, set pd.exe_descriptor to descriptor, otherwise set it to -1.

--- libbacktrace/elf.c.jj	2013-11-19 08:47:37.000000000 +0100
+++ libbacktrace/elf.c	2013-12-06 08:26:53.273602062 +0100
@@ -96,6 +96,7 @@ dl_iterate_phdr (int (*callback) (struct
 #undef ELFDATA2LSB
 #undef ELFDATA2MSB
 #undef EV_CURRENT
+#undef ET_DYN
 #undef SHN_LORESERVE
 #undef SHN_XINDEX
 #undef SHN_UNDEF
@@ -171,6 +172,8 @@ typedef struct {
 
 #define EV_CURRENT 1
 
+#define ET_DYN 3
+
 typedef struct {
   b_elf_word	sh_name;		/* Section name, index in string tbl */
   b_elf_word	sh_type;		/* Type of section */
@@ -512,7 +515,7 @@ elf_syminfo (struct backtrace_state *sta
 static int
 elf_add (struct backtrace_state *state, int descriptor, uintptr_t base_address,
 	 backtrace_error_callback error_callback, void *data,
-	 fileline *fileline_fn, int *found_sym, int *found_dwarf)
+	 fileline *fileline_fn, int *found_sym, int *found_dwarf, int exe)
 {
   struct backtrace_view ehdr_view;
   b_elf_ehdr ehdr;
@@ -591,6 +594,12 @@ elf_add (struct backtrace_state *state,
       goto fail;
     }
 
+  /* If the executable is ET_DYN, it is either a PIE, or we are running
+     directly a shared library with .interp.  We need to wait for
+     dl_iterate_phdr in that case to determine the actual base_address.  */
+  if (exe && ehdr.e_type == ET_DYN)
+    return -1;
+
   shoff = ehdr.e_shoff;
   shnum = ehdr.e_shnum;
   shstrndx = ehdr.e_shstrndx;
@@ -847,6 +856,7 @@ struct phdr_data
   fileline *fileline_fn;
   int *found_sym;
   int *found_dwarf;
+  int exe_descriptor;
 };
 
 /* Callback passed to dl_iterate_phdr.  Load debug info from shared
@@ -862,17 +872,32 @@ phdr_callback (struct dl_phdr_info *info
   fileline elf_fileline_fn;
   int found_dwarf;
 
-  /* There is not much we can do if we don't have the module name.  */
+  /* There is not much we can do if we don't have the module name,
+     unless executable is ET_DYN, where we expect the very first
+     phdr_callback to be for the PIE.  */
   if (info->dlpi_name == NULL || info->dlpi_name[0] == '\0')
-    return 0;
+    {
+      if (pd->exe_descriptor == -1)
+	return 0;
+      descriptor = pd->exe_descriptor;
+      pd->exe_descriptor = -1;
+    }
+  else
+    {
+      if (pd->exe_descriptor != -1)
+	{
+	  backtrace_close (pd->exe_descriptor, pd->error_callback, pd->data);
+	  pd->exe_descriptor = -1;
+	}
 
-  descriptor = backtrace_open (info->dlpi_name, pd->error_callback, pd->data,
-			       &does_not_exist);
-  if (descriptor < 0)
-    return 0;
+      descriptor = backtrace_open (info->dlpi_name, pd->error_callback,
+				   pd->data, &does_not_exist);
+      if (descriptor < 0)
+	return 0;
+    }
 
   if (elf_add (pd->state, descriptor, info->dlpi_addr, pd->error_callback,
-	       pd->data, &elf_fileline_fn, pd->found_sym, &found_dwarf))
+	       pd->data, &elf_fileline_fn, pd->found_sym, &found_dwarf, 0))
     {
       if (found_dwarf)
 	{
@@ -893,13 +918,15 @@ backtrace_initialize (struct backtrace_s
 		      backtrace_error_callback error_callback,
 		      void *data, fileline *fileline_fn)
 {
+  int ret;
   int found_sym;
   int found_dwarf;
   fileline elf_fileline_fn;
   struct phdr_data pd;
 
-  if (!elf_add (state, descriptor, 0, error_callback, data, &elf_fileline_fn,
-		&found_sym, &found_dwarf))
+  ret = elf_add (state, descriptor, 0, error_callback, data, &elf_fileline_fn,
+		 &found_sym, &found_dwarf, 1);
+  if (!ret)
     return 0;
 
   pd.state = state;
@@ -908,6 +935,7 @@ backtrace_initialize (struct backtrace_s
   pd.fileline_fn = &elf_fileline_fn;
   pd.found_sym = &found_sym;
   pd.found_dwarf = &found_dwarf;
+  pd.exe_descriptor = ret < 0 ? descriptor : -1;
 
   dl_iterate_phdr (phdr_callback, (void *) &pd);
 

	Jakub

Re: [PATCH] Handle PIEs in libbacktrace

[Dmitry Vyukov]

On Fri, Dec 6, 2013 at 11:50 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> With the new tsan tests, I've noticed that libbacktrace symbolization
> doesn't work when the binary is a PIE.
> The problem is that in that case we obviously can't use base_address
> of 0, the PIE typically will not have 0 bias, that is actually the sole
> point of PIEs that their base address is randomized.  So, in that case
> we need to wait until dl_iterate_phdr to determine the proper bias.
> The executable is always the first in the search scope and/or list of
> loaded modules, so we just check if the first callback gives us a dlpi_name
> == NULL entry and assume it is the PIE.
>
> Regtested on x86_64-linux, ok for trunk?
>
> The rest of this mail except the libbacktrace patch is for the sanitizer
> folks.
>
> With this patch check-gcc tsan.exp passes fully again, but check-g++ does
> not, there seems to be important differences between tsan symbolization
> and say asan symbolization.  First of all, if symbolizer fails (as without
> this patch for tsan because the executables are PIEs), we get something
> like:
> WARNING: ThreadSanitizer: data race (pid=18998)
>   Write of size 4 at 0x7f4a88f0a08c by thread T1:
>     #0 <null> <null>:0 (tiny_race+0x000000000b19)
>
>   Previous write of size 4 at 0x7f4a88f0a08c by main thread:
>     #0 <null> <null>:0 (tiny_race+0x000000000b7a)
>     #1 __libc_start_main <null>:0 (libc.so.6+0x003c08c21b74)
>
>   As if synchronized via sleep:
>     #0 sleep ../../../../libsanitizer/tsan/tsan_interceptors.cc:239 (libtsan.so.0+0x000000048c65)
>     #1 <null> <null>:0 (tiny_race+0x000000000b0a)
>
>   Location is global '<null>' of size 0 at 0x000000000000 (tiny_race+0x00000020208c)
>
>   Thread T1 (tid=19000, running) created by main thread at:
>     #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
>     #1 <null> <null>:0 (tiny_race+0x000000000b6b)
>     #2 __libc_start_main <null>:0 (libc.so.6+0x003c08c21b74)
>
> The <null> <null>:0 or <null>:0 look very ugly, e.g. asan
> I think just prints the exe/dso name + offset in that case.


Tsan can not work w/o symbolization (because it relies on online
suppressions, etc). So there is little sense in making it prettier.


> And the reason why check-g++ tsan.exp fails even with this patch is
> that apparently tsan doesn't try to demangle the symbol names, so we get
> e.g.:

Demangling must be done by the symbolizer.
+samsonov for this


> FAIL: c-c++-common/tsan/atomic_stack.c  -O0  output pattern test, is ==================
> WARNING: ThreadSanitizer: data race (pid=22075)
>   Atomic write of size 4 at 0x7fe8c9b630ac by thread T1:
>     #0 __tsan_atomic32_fetch_add ../../../../libsanitizer/tsan/tsan_interface_atomic.cc:468 (libtsan.so.0+0x00000001ec7e)
>     #1 _Z7Thread1Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:11 (atomic_stack.exe+0x000000000d90)
>
>   Previous write of size 4 at 0x7fe8c9b630ac by thread T2:
>     #0 _Z7Thread2Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:16 (atomic_stack.exe+0x000000000dde)
>
>   As if synchronized via sleep:
>     #0 sleep ../../../../libsanitizer/tsan/tsan_interceptors.cc:239 (libtsan.so.0+0x000000048c65)
>     #1 _Z7Thread1Pv /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:10 (atomic_stack.exe+0x000000000d7a)
>
>   Location is global 'Global' of size 4 at 0x7fe8c9b630ac (atomic_stack.exe+0x0000002020ac)
>
>   Thread T1 (tid=22080, running) created by main thread at:
>     #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
>     #1 main /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:22 (atomic_stack.exe+0x000000000e2a)
>
>   Thread T2 (tid=22081, finished) created by main thread at:
>     #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:877 (libtsan.so.0+0x0000000461b3)
>     #1 main /usr/src/gcc/gcc/testsuite/c-c++-common/tsan/atomic_stack.c:23 (atomic_stack.exe+0x000000000e4b)
>
> Is the non-demangling intentional?  If yes, I guess we'd need to adjust
> the testcases to cope with both mangled and non-mangled names, otherwise
> please change tsan to demangle, the symbolizer has functions for it.
>
> 2013-12-06  Jakub Jelinek  <jakub@redhat.com>
>
>         * elf.c (ET_DYN): Undefine and define again.
>         (elf_add): Add exe argument, if true and ehdr.e_type is ET_DYN,
>         return early -1 without closing the descriptor.
>         (struct phdr_data): Add exe_descriptor.
>         (phdr_callback): If pd->exe_descriptor is not -1, for very first
>         call if dlpi_name is NULL just call elf_add with the exe_descriptor,
>         otherwise backtrace_close the exe_descriptor if not -1.  Adjust
>         call to elf_add.
>         (backtrace_initialize): Adjust call to elf_add.  If it returns
>         -1, set pd.exe_descriptor to descriptor, otherwise set it to -1.
>
> --- libbacktrace/elf.c.jj       2013-11-19 08:47:37.000000000 +0100
> +++ libbacktrace/elf.c  2013-12-06 08:26:53.273602062 +0100
> @@ -96,6 +96,7 @@ dl_iterate_phdr (int (*callback) (struct
>  #undef ELFDATA2LSB
>  #undef ELFDATA2MSB
>  #undef EV_CURRENT
> +#undef ET_DYN
>  #undef SHN_LORESERVE
>  #undef SHN_XINDEX
>  #undef SHN_UNDEF
> @@ -171,6 +172,8 @@ typedef struct {
>
>  #define EV_CURRENT 1
>
> +#define ET_DYN 3
> +
>  typedef struct {
>    b_elf_word   sh_name;                /* Section name, index in string tbl */
>    b_elf_word   sh_type;                /* Type of section */
> @@ -512,7 +515,7 @@ elf_syminfo (struct backtrace_state *sta
>  static int
>  elf_add (struct backtrace_state *state, int descriptor, uintptr_t base_address,
>          backtrace_error_callback error_callback, void *data,
> -        fileline *fileline_fn, int *found_sym, int *found_dwarf)
> +        fileline *fileline_fn, int *found_sym, int *found_dwarf, int exe)
>  {
>    struct backtrace_view ehdr_view;
>    b_elf_ehdr ehdr;
> @@ -591,6 +594,12 @@ elf_add (struct backtrace_state *state,
>        goto fail;
>      }
>
> +  /* If the executable is ET_DYN, it is either a PIE, or we are running
> +     directly a shared library with .interp.  We need to wait for
> +     dl_iterate_phdr in that case to determine the actual base_address.  */
> +  if (exe && ehdr.e_type == ET_DYN)
> +    return -1;
> +
>    shoff = ehdr.e_shoff;
>    shnum = ehdr.e_shnum;
>    shstrndx = ehdr.e_shstrndx;
> @@ -847,6 +856,7 @@ struct phdr_data
>    fileline *fileline_fn;
>    int *found_sym;
>    int *found_dwarf;
> +  int exe_descriptor;
>  };
>
>  /* Callback passed to dl_iterate_phdr.  Load debug info from shared
> @@ -862,17 +872,32 @@ phdr_callback (struct dl_phdr_info *info
>    fileline elf_fileline_fn;
>    int found_dwarf;
>
> -  /* There is not much we can do if we don't have the module name.  */
> +  /* There is not much we can do if we don't have the module name,
> +     unless executable is ET_DYN, where we expect the very first
> +     phdr_callback to be for the PIE.  */
>    if (info->dlpi_name == NULL || info->dlpi_name[0] == '\0')
> -    return 0;
> +    {
> +      if (pd->exe_descriptor == -1)
> +       return 0;
> +      descriptor = pd->exe_descriptor;
> +      pd->exe_descriptor = -1;
> +    }
> +  else
> +    {
> +      if (pd->exe_descriptor != -1)
> +       {
> +         backtrace_close (pd->exe_descriptor, pd->error_callback, pd->data);
> +         pd->exe_descriptor = -1;
> +       }
>
> -  descriptor = backtrace_open (info->dlpi_name, pd->error_callback, pd->data,
> -                              &does_not_exist);
> -  if (descriptor < 0)
> -    return 0;
> +      descriptor = backtrace_open (info->dlpi_name, pd->error_callback,
> +                                  pd->data, &does_not_exist);
> +      if (descriptor < 0)
> +       return 0;
> +    }
>
>    if (elf_add (pd->state, descriptor, info->dlpi_addr, pd->error_callback,
> -              pd->data, &elf_fileline_fn, pd->found_sym, &found_dwarf))
> +              pd->data, &elf_fileline_fn, pd->found_sym, &found_dwarf, 0))
>      {
>        if (found_dwarf)
>         {
> @@ -893,13 +918,15 @@ backtrace_initialize (struct backtrace_s
>                       backtrace_error_callback error_callback,
>                       void *data, fileline *fileline_fn)
>  {
> +  int ret;
>    int found_sym;
>    int found_dwarf;
>    fileline elf_fileline_fn;
>    struct phdr_data pd;
>
> -  if (!elf_add (state, descriptor, 0, error_callback, data, &elf_fileline_fn,
> -               &found_sym, &found_dwarf))
> +  ret = elf_add (state, descriptor, 0, error_callback, data, &elf_fileline_fn,
> +                &found_sym, &found_dwarf, 1);
> +  if (!ret)
>      return 0;
>
>    pd.state = state;
> @@ -908,6 +935,7 @@ backtrace_initialize (struct backtrace_s
>    pd.fileline_fn = &elf_fileline_fn;
>    pd.found_sym = &found_sym;
>    pd.found_dwarf = &found_dwarf;
> +  pd.exe_descriptor = ret < 0 ? descriptor : -1;
>
>    dl_iterate_phdr (phdr_callback, (void *) &pd);
>
>
>         Jakub

Re: [PATCH] Handle PIEs in libbacktrace

[Jakub Jelinek]

On Fri, Dec 06, 2013 at 12:19:45PM +0400, Dmitry Vyukov wrote:
> > And the reason why check-g++ tsan.exp fails even with this patch is
> > that apparently tsan doesn't try to demangle the symbol names, so we get
> > e.g.:
> 
> Demangling must be done by the symbolizer.
> +samsonov for this

So why does asan_report.cc have then:
static const char *MaybeDemangleGlobalName(const char *name) {
  // We can spoil names of globals with C linkage, so use an heuristic
  // approach to check if the name should be demangled.
  return (name[0] == '_' && name[1] == 'Z')
             ? Symbolizer::Get()->Demangle(name)
             : name;
}
and uses it where it wants to demangle?  From what I can see, even when
you are using llvm-symbolizer, sanitizer_common doesn't pass -demangle
option to it.

	Jakub

Re: [PATCH] Handle PIEs in libbacktrace

[Dmitry Vyukov]

On Fri, Dec 6, 2013 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Dec 06, 2013 at 12:19:45PM +0400, Dmitry Vyukov wrote:
>> > And the reason why check-g++ tsan.exp fails even with this patch is
>> > that apparently tsan doesn't try to demangle the symbol names, so we get
>> > e.g.:
>>
>> Demangling must be done by the symbolizer.
>> +samsonov for this
>
> So why does asan_report.cc have then:
> static const char *MaybeDemangleGlobalName(const char *name) {
>   // We can spoil names of globals with C linkage, so use an heuristic
>   // approach to check if the name should be demangled.
>   return (name[0] == '_' && name[1] == 'Z')
>              ? Symbolizer::Get()->Demangle(name)
>              : name;
> }
> and uses it where it wants to demangle?

I would say this is a defect. It's symbolizer responsibility.

> From what I can see, even when
> you are using llvm-symbolizer, sanitizer_common doesn't pass -demangle
> option to it.


It defaults to true:
static cl::opt<bool>
ClDemangle("demangle", cl::init(true), cl::desc("Demangle function names"));

Re: [PATCH] Handle PIEs in libbacktrace

[Alexey Samsonov]

(now in plain-text mode).

ASan calls this function only for global variables (not functions). It
is needed because
we use a different machinery to pass global names to ASan runtime - we
register globals
by a call from instrumented code, which also tells runtime the
(mangled) global name.

OTOH, in TSan we rely on symbolization to demangle names of global variables.

>
>  From what I can see, even when
> you are using llvm-symbolizer, sanitizer_common doesn't pass -demangle
> option to it.


-demangle defaults to "on" in llvm-symbolizer.


On Fri, Dec 6, 2013 at 12:51 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Dec 6, 2013 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Fri, Dec 06, 2013 at 12:19:45PM +0400, Dmitry Vyukov wrote:
>>> > And the reason why check-g++ tsan.exp fails even with this patch is
>>> > that apparently tsan doesn't try to demangle the symbol names, so we get
>>> > e.g.:
>>>
>>> Demangling must be done by the symbolizer.
>>> +samsonov for this
>>
>> So why does asan_report.cc have then:
>> static const char *MaybeDemangleGlobalName(const char *name) {
>>   // We can spoil names of globals with C linkage, so use an heuristic
>>   // approach to check if the name should be demangled.
>>   return (name[0] == '_' && name[1] == 'Z')
>>              ? Symbolizer::Get()->Demangle(name)
>>              : name;
>> }
>> and uses it where it wants to demangle?
>
> I would say this is a defect. It's symbolizer responsibility.
>
>> From what I can see, even when
>> you are using llvm-symbolizer, sanitizer_common doesn't pass -demangle
>> option to it.
>
>
> It defaults to true:
> static cl::opt<bool>
> ClDemangle("demangle", cl::init(true), cl::desc("Demangle function names"));



-- 
Alexey Samsonov, MSK

Re: [PATCH] Handle PIEs in libbacktrace

[Jakub Jelinek]

On Fri, Dec 06, 2013 at 12:53:02PM +0400, Alexey Samsonov wrote:
> ASan calls this function only for global variables (not functions). It
> is needed because
> we use a different machinery to pass global names to ASan runtime - we
> register globals
> by a call from instrumented code, which also tells runtime the
> (mangled) global name.

Well, given that it demangled just fine even function names, I guess it
calls it always and just it would do nothing for function names demangled
already by llvm-symbolizer if using the external symbolizer.

> OTOH, in TSan we rely on symbolization to demangle names of global variables.

Then we need something like following, with which all tests pass.  I'm not
using Demangle method from the symbolizer, because getting it requires a
mutex that is already held.

The alternative would be to just (perhaps under #ifdef SANITIZER_CP_DEMANGLE)
compile in libiberty/cp-demangle.c (similarly how libstdc++ compiles it in)
as part of libsanitizer/libiberty/ or even libsanitizer/libbacktrace/,
and tweak it, so that like libsanitizer/libbacktrace it uses internal_memcpy
etc. and uses InternalAlloc/InternalFree.  The problem is that cp-demangle.c
uses only realloc and free, and doesn't provide any hint on how large the
previously allocated memory chunk is.  So, either there is some easy way
how to query the size of InternalAlloc returned allocation, or we would need
to allocate uptr extra and store there number of bytes allocated
and emulate realloc/free that way.

--- libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc.jj	2013-12-05 12:04:28.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc	2013-12-06 10:36:02.285059188 +0100
@@ -25,6 +25,15 @@
 # endif
 #endif
 
+// C++ demangling function, as required by Itanium C++ ABI. This is weak,
+// because we do not require a C++ ABI library to be linked to a program
+// using sanitizers; if it's not present, we'll just use the mangled name.
+namespace __cxxabiv1 {
+  extern "C" SANITIZER_WEAK_ATTRIBUTE
+  char *__cxa_demangle(const char *mangled, char *buffer,
+		       size_t *length, int *status);
+}
+
 namespace __sanitizer {
 
 #if SANITIZER_LIBBACKTRACE
@@ -39,6 +48,19 @@ struct SymbolizeCodeData {
   uptr module_offset;
 };
 
+char *MaybeDemangleAndDup(const char *symname) {
+  const char *demangled = symname;
+  if (symname[0] == '_' && symname[1] == 'Z' && __cxxabiv1::__cxa_demangle) {
+    demangled = __cxxabiv1::__cxa_demangle(symname, 0, 0, 0);
+    if (demangled == NULL)
+      demangled = symname;
+  }                      
+  char *ret = internal_strdup(demangled);
+  if (demangled != symname)
+    __builtin_free ((void *) demangled);
+  return ret;
+}
+
 extern "C" {
 static int SymbolizeCodePCInfoCallback(void *vdata, uintptr_t addr,
                                        const char *filename, int lineno,
@@ -49,7 +71,7 @@ static int SymbolizeCodePCInfoCallback(v
     info->Clear();
     info->FillAddressAndModuleInfo(addr, cdata->module_name,
                                    cdata->module_offset);
-    info->function = internal_strdup(function);
+    info->function = MaybeDemangleAndDup(function);
     if (filename)
       info->file = internal_strdup(filename);
     info->line = lineno;
@@ -67,7 +89,7 @@ static void SymbolizeCodeCallback(void *
     info->Clear();
     info->FillAddressAndModuleInfo(addr, cdata->module_name,
                                    cdata->module_offset);
-    info->function = internal_strdup(symname);
+    info->function = MaybeDemangleAndDup(symname);
     cdata->n_frames = 1;
   }
 }
@@ -76,7 +98,7 @@ static void SymbolizeDataCallback(void *
                                   uintptr_t symval, uintptr_t symsize) {
   DataInfo *info = (DataInfo *)vdata;
   if (symname && symval) {
-    info->name = internal_strdup(symname);
+    info->name = MaybeDemangleAndDup(symname);
     info->start = symval;
     info->size = symsize;
   }


	Jakub

Re: [PATCH] Handle PIEs in libbacktrace

[Ian Lance Taylor]

On Thu, Dec 5, 2013 at 11:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> 2013-12-06  Jakub Jelinek  <jakub@redhat.com>
>
>         * elf.c (ET_DYN): Undefine and define again.
>         (elf_add): Add exe argument, if true and ehdr.e_type is ET_DYN,
>         return early -1 without closing the descriptor.
>         (struct phdr_data): Add exe_descriptor.
>         (phdr_callback): If pd->exe_descriptor is not -1, for very first
>         call if dlpi_name is NULL just call elf_add with the exe_descriptor,
>         otherwise backtrace_close the exe_descriptor if not -1.  Adjust
>         call to elf_add.
>         (backtrace_initialize): Adjust call to elf_add.  If it returns
>         -1, set pd.exe_descriptor to descriptor, otherwise set it to -1.

Please update the comment for elf_add to explain the return value.
This patch is OK with that change.

Thanks.

Ian

Re: [PATCH] Handle PIEs in libbacktrace

[Ian Lance Taylor]

On Fri, Dec 6, 2013 at 1:53 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> The alternative would be to just (perhaps under #ifdef SANITIZER_CP_DEMANGLE)
> compile in libiberty/cp-demangle.c (similarly how libstdc++ compiles it in)
> as part of libsanitizer/libiberty/ or even libsanitizer/libbacktrace/,
> and tweak it, so that like libsanitizer/libbacktrace it uses internal_memcpy
> etc. and uses InternalAlloc/InternalFree.  The problem is that cp-demangle.c
> uses only realloc and free, and doesn't provide any hint on how large the
> previously allocated memory chunk is.  So, either there is some easy way
> how to query the size of InternalAlloc returned allocation, or we would need
> to allocate uptr extra and store there number of bytes allocated
> and emulate realloc/free that way.

There was a recent buggy patch to the demangler that added calls to
malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
That patch must be fixed or reverted before the 4.9 release.  The main
code in the demangler must not call malloc/realloc.

When that patch is fixed, you can use the cplus_demangle_v3_callback
function to get a demangler that never calls malloc.

Ian

[PATCH] libsanitizer demangling using cp-demangle.c

[Jakub Jelinek]

On Fri, Dec 06, 2013 at 06:40:52AM -0800, Ian Lance Taylor wrote:
> There was a recent buggy patch to the demangler that added calls to
> malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
> That patch must be fixed or reverted before the 4.9 release.  The main
> code in the demangler must not call malloc/realloc.
> 
> When that patch is fixed, you can use the cplus_demangle_v3_callback
> function to get a demangler that never calls malloc.

AFAIK Gary is working on a fix, when that is fixed, with the following
patch libsanitizer (when using libbacktrace for symbolization) will not
use system malloc/realloc/free for the demangling at all.

Tested on x86_64-linux (-m64/-m32).  Note that the changes for the 3 files
unfortunately will need to be applied upstream to compiler-rt, is that
possible?

2013-12-10  Jakub Jelinek  <jakub@redhat.com>

	* sanitizer_common/sanitizer_symbolizer_libbacktrace.h
	(LibbacktraceSymbolizer::Demangle): New declaration.
	* sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
	(POSIXSymbolizer::Demangle): Use libbacktrace_symbolizer_'s Demangle
	method if possible.
	* sanitizer_common/sanitizer_symbolizer_libbacktrace.cc: Include
	"demangle.h" if SANITIZE_CP_DEMANGLE is defined.
	(struct CplusV3DemangleData): New type.
	(CplusV3DemangleCallback, CplusV3Demangle): New functions.
	(SymbolizeCodePCInfoCallback, SymbolizeCodeCallback,
	SymbolizeDataCallback): Use CplusV3Demangle.
	* sanitizer_common/Makefile.am (AM_CXXFLAGS): Add
	-DSANITIZE_CP_DEMANGLE and -I $(top_srcdir)/../include.
	* libbacktrace/backtrace-rename.h (cplus_demangle_builtin_types,
	cplus_demangle_fill_ctor, cplus_demangle_fill_dtor,
	cplus_demangle_fill_extended_operator, cplus_demangle_fill_name,
	cplus_demangle_init_info, cplus_demangle_mangled_name,
	cplus_demangle_operators, cplus_demangle_print,
	cplus_demangle_print_callback, cplus_demangle_type, cplus_demangle_v3,
	cplus_demangle_v3_callback, is_gnu_v3_mangled_ctor,
	is_gnu_v3_mangled_dtor, java_demangle_v3, java_demangle_v3_callback):
	Define.
	(__asan_internal_memcmp, __asan_internal_strncmp): New prototypes.
	(memcmp, strncmp): Redefine.
	* libbacktrace/Makefile.am (libsanitizer_libbacktrace_la_SOURCES): Add
	../../libiberty/cp-demangle.c.
	* libbacktrace/bridge.cc (__asan_internal_memcmp,
	__asan_internal_strncmp): New functions.
	* sanitizer_common/Makefile.in: Regenerated.
	* libbacktrace/Makefile.in: Regenerated.
	* configure: Regenerated.
	* configure.ac: Regenerated.
	* config.h.in: Regenerated.

--- libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.h.jj	2013-12-05 12:04:28.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.h	2013-12-10 11:01:26.777371566 +0100
@@ -29,6 +29,8 @@ class LibbacktraceSymbolizer {
 
   bool SymbolizeData(DataInfo *info);
 
+  const char *Demangle(const char *name);
+
  private:
   explicit LibbacktraceSymbolizer(void *state) : state_(state) {}
 
--- libsanitizer/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc.jj	2013-12-05 12:04:28.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc	2013-12-10 11:03:02.971876505 +0100
@@ -513,6 +513,11 @@ class POSIXSymbolizer : public Symbolize
     SymbolizerScope sym_scope(this);
     if (internal_symbolizer_ != 0)
       return internal_symbolizer_->Demangle(name);
+    if (libbacktrace_symbolizer_ != 0) {
+      const char *demangled = libbacktrace_symbolizer_->Demangle(name);
+      if (demangled)
+	return demangled;
+    }
     return DemangleCXXABI(name);
   }
 
--- libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc.jj	2013-12-09 14:32:06.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc	2013-12-10 11:48:19.803830291 +0100
@@ -20,6 +20,10 @@
 # include "backtrace-supported.h"
 # if SANITIZER_POSIX && BACKTRACE_SUPPORTED && !BACKTRACE_USES_MALLOC
 #  include "backtrace.h"
+#  if SANITIZER_CP_DEMANGLE
+#   undef ARRAY_SIZE
+#   include "demangle.h"
+#  endif
 # else
 #  define SANITIZER_LIBBACKTRACE 0
 # endif
@@ -31,6 +35,60 @@ namespace __sanitizer {
 
 namespace {
 
+#if SANITIZER_CP_DEMANGLE
+struct CplusV3DemangleData {
+  char *buf;
+  uptr size, allocated;
+};
+
+extern "C" {
+static void CplusV3DemangleCallback(const char *s, size_t l, void *vdata) {
+  CplusV3DemangleData *data = (CplusV3DemangleData *)vdata;
+  uptr needed = data->size + l + 1;
+  if (needed > data->allocated) {
+    data->allocated *= 2;
+    if (needed > data->allocated)
+      data->allocated = needed;
+    char *buf = (char *)InternalAlloc(data->allocated);
+    if (data->buf) {
+      internal_memcpy(buf, data->buf, data->size);
+      InternalFree(data->buf);
+    }
+    data->buf = buf;
+  }
+  internal_memcpy(data->buf + data->size, s, l);
+  data->buf[data->size + l] = '\0';
+  data->size += l;
+}
+}  // extern "C"
+
+char *CplusV3Demangle(const char *name, bool always_alloc) {
+  CplusV3DemangleData data;
+  data.buf = 0;
+  data.size = 0;
+  data.allocated = 0;
+  if (cplus_demangle_v3_callback(name, DMGL_PARAMS | DMGL_ANSI,
+				 CplusV3DemangleCallback, &data)) {
+    if (data.size + 64 > data.allocated)
+      return data.buf;
+    char *buf = internal_strdup(data.buf);
+    InternalFree(data.buf);
+    return buf;
+  }
+  if (data.buf)
+    InternalFree(data.buf);
+  if (always_alloc)
+    return internal_strdup(name);
+  return 0;
+}
+#else
+const char *CplusV3Demangle(const char *name, bool always_alloc) {
+  if (always_alloc)
+    return internal_strdup(name);
+  return 0;
+}
+#endif
+
 struct SymbolizeCodeData {
   AddressInfo *frames;
   uptr n_frames;
@@ -49,7 +107,7 @@ static int SymbolizeCodePCInfoCallback(v
     info->Clear();
     info->FillAddressAndModuleInfo(addr, cdata->module_name,
                                    cdata->module_offset);
-    info->function = internal_strdup(function);
+    info->function = CplusV3Demangle(function, true);
     if (filename)
       info->file = internal_strdup(filename);
     info->line = lineno;
@@ -67,7 +125,7 @@ static void SymbolizeCodeCallback(void *
     info->Clear();
     info->FillAddressAndModuleInfo(addr, cdata->module_name,
                                    cdata->module_offset);
-    info->function = internal_strdup(symname);
+    info->function = CplusV3Demangle(symname, true);
     cdata->n_frames = 1;
   }
 }
@@ -76,7 +134,7 @@ static void SymbolizeDataCallback(void *
                                   uintptr_t symval, uintptr_t symsize) {
   DataInfo *info = (DataInfo *)vdata;
   if (symname && symval) {
-    info->name = internal_strdup(symname);
+    info->name = CplusV3Demangle(symname, true);
     info->start = symval;
     info->size = symsize;
   }
@@ -121,6 +179,17 @@ bool LibbacktraceSymbolizer::SymbolizeDa
   return true;
 }
 
+const char *LibbacktraceSymbolizer::Demangle(const char *name) {
+#if SANITIZER_CP_DEMANGLE
+  const char *demangled = CplusV3Demangle(name, false);
+  if (demangled)
+    return demangled;
+  return name;
+#else
+  return 0;
+#endif
+}
+
 #else  // SANITIZER_LIBBACKTRACE
 
 LibbacktraceSymbolizer *LibbacktraceSymbolizer::get(LowLevelAllocator *alloc) {
@@ -139,6 +208,10 @@ bool LibbacktraceSymbolizer::SymbolizeDa
   return false;
 }
 
+const char *LibbacktraceSymbolizer::Demangle(const char *name) {
+  return 0;
+}
+
 #endif  // SANITIZER_LIBBACKTRACE
 
 }  // namespace __sanitizer
--- libsanitizer/sanitizer_common/Makefile.am.jj	2013-12-10 09:56:36.000000000 +0100
+++ libsanitizer/sanitizer_common/Makefile.am	2013-12-10 10:41:03.745693274 +0100
@@ -7,8 +7,10 @@ DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_C
 AM_CXXFLAGS = -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long  -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros
 AM_CXXFLAGS += $(LIBSTDCXX_RAW_CXX_CXXFLAGS)
 if LIBBACKTRACE_SUPPORTED
-AM_CXXFLAGS += -DSANITIZER_LIBBACKTRACE -I $(top_srcdir)/../libbacktrace \
+AM_CXXFLAGS += -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
+	       -I $(top_srcdir)/../libbacktrace \
 	       -I $(top_builddir)/libbacktrace \
+	       -I $(top_srcdir)/../include \
 	       -include $(top_srcdir)/libbacktrace/backtrace-rename.h
 endif
 ACLOCAL_AMFLAGS = -I m4
--- libsanitizer/libbacktrace/backtrace-rename.h.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/libbacktrace/backtrace-rename.h	2013-12-10 11:50:24.186187737 +0100
@@ -14,25 +14,49 @@
 #define backtrace_vector_grow __asan_backtrace_vector_grow
 #define backtrace_vector_release __asan_backtrace_vector_release
 
+#define cplus_demangle_builtin_types __asan_cplus_demangle_builtin_types
+#define cplus_demangle_fill_ctor __asan_cplus_demangle_fill_ctor
+#define cplus_demangle_fill_dtor __asan_cplus_demangle_fill_dtor
+#define cplus_demangle_fill_extended_operator __asan_cplus_demangle_fill_extended_operator
+#define cplus_demangle_fill_name __asan_cplus_demangle_fill_name
+#define cplus_demangle_init_info __asan_cplus_demangle_init_info
+#define cplus_demangle_mangled_name __asan_cplus_demangle_mangled_name
+#define cplus_demangle_operators __asan_cplus_demangle_operators
+#define cplus_demangle_print __asan_cplus_demangle_print
+#define cplus_demangle_print_callback __asan_cplus_demangle_print_callback
+#define cplus_demangle_type __asan_cplus_demangle_type
+#define cplus_demangle_v3 __asan_cplus_demangle_v3
+#define cplus_demangle_v3_callback __asan_cplus_demangle_v3_callback
+#define is_gnu_v3_mangled_ctor __asan_is_gnu_v3_mangled_ctor
+#define is_gnu_v3_mangled_dtor __asan_is_gnu_v3_mangled_dtor
+#define java_demangle_v3 __asan_java_demangle_v3
+#define java_demangle_v3_callback __asan_java_demangle_v3_callback
+
 #ifndef __cplusplus
 
 #include <string.h>
 
 extern void *__asan_internal_memcpy (void *, const void *, size_t);
 extern void *__asan_internal_memset (void *, int, size_t);
+extern int __asan_internal_memcmp (const void *, const void *, size_t);
 extern int __asan_internal_strcmp (const char *, const char *);
+extern int __asan_internal_strncmp (const char *, const char *, size_t);
 extern size_t __asan_internal_strlen (const char *);
 extern size_t __asan_internal_strnlen (const char *, size_t);
 
 #undef memcpy
 #undef memset
+#undef memcmp
 #undef strcmp
+#undef strncmp
 #undef strlen
 #undef strnlen
 
 #define memcpy(x,y,z) __asan_internal_memcpy (x, y, z)
 #define memset(x,y,z) __asan_internal_memset (x, y, z)
+#define memcmp(x,y,z) __asan_internal_memcmp (x, y, z)
 #define strcmp(x,y) __asan_internal_strcmp (x, y)
+#define strncmp(x,y,z) __asan_internal_strncmp (x, y, z)
 #define strlen(x) __asan_internal_strlen (x)
 #ifdef HAVE_DECL_STRNLEN
 #define strnlen(x,y) __asan_internal_strnlen (x, y)
--- libsanitizer/libbacktrace/Makefile.am.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/libbacktrace/Makefile.am	2013-12-10 10:07:33.115084811 +0100
@@ -51,6 +51,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
 	../../libbacktrace/internal.h \
 	../../libbacktrace/posix.c \
 	../../libbacktrace/state.c \
+	../../libiberty/cp-demangle.c \
 	bridge.cc
 
 FORMAT_FILES = \
--- libsanitizer/libbacktrace/bridge.cc.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/libbacktrace/bridge.cc	2013-12-10 11:48:49.038678487 +0100
@@ -52,11 +52,23 @@ __asan_internal_memset (void *dest, int
 }
 
 int
+__asan_internal_memcmp (const void *s1, const void *s2, size_t n)
+{
+  return __sanitizer::internal_memcmp (s1, s2, n);
+}
+
+int
 __asan_internal_strcmp (const char *s1, const char *s2)
 {
   return __sanitizer::internal_strcmp (s1, s2);
 }
 
+int
+__asan_internal_strncmp (const char *s1, const char *s2, size_t n)
+{
+  return __sanitizer::internal_strncmp (s1, s2, n);
+}
+
 size_t
 __asan_internal_strlen (const char *str)
 {
--- libsanitizer/sanitizer_common/Makefile.in.jj	2013-12-10 10:28:02.000000000 +0100
+++ libsanitizer/sanitizer_common/Makefile.in	2013-12-10 11:38:37.921836716 +0100
@@ -35,8 +35,10 @@ POST_UNINSTALL = :
 build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
-@LIBBACKTRACE_SUPPORTED_TRUE@am__append_1 = -DSANITIZER_LIBBACKTRACE -I $(top_srcdir)/../libbacktrace \
+@LIBBACKTRACE_SUPPORTED_TRUE@am__append_1 = -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
+@LIBBACKTRACE_SUPPORTED_TRUE@	       -I $(top_srcdir)/../libbacktrace \
 @LIBBACKTRACE_SUPPORTED_TRUE@	       -I $(top_builddir)/libbacktrace \
+@LIBBACKTRACE_SUPPORTED_TRUE@	       -I $(top_srcdir)/../include \
 @LIBBACKTRACE_SUPPORTED_TRUE@	       -include $(top_srcdir)/libbacktrace/backtrace-rename.h
 
 subdir = sanitizer_common
--- libsanitizer/libbacktrace/Makefile.in.jj	2013-12-10 09:57:05.000000000 +0100
+++ libsanitizer/libbacktrace/Makefile.in	2013-12-10 11:38:37.858837042 +0100
@@ -90,7 +90,7 @@ CONFIG_CLEAN_VPATH_FILES =
 LTLIBRARIES = $(noinst_LTLIBRARIES)
 am__DEPENDENCIES_1 =
 am_libsanitizer_libbacktrace_la_OBJECTS = atomic.lo dwarf.lo \
-	fileline.lo posix.lo state.lo bridge.lo
+	fileline.lo posix.lo state.lo cp-demangle.lo bridge.lo
 libsanitizer_libbacktrace_la_OBJECTS =  \
 	$(am_libsanitizer_libbacktrace_la_OBJECTS)
 DEFAULT_INCLUDES = -I.@am__isrc@ -I$(top_builddir)
@@ -280,6 +280,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
 	../../libbacktrace/internal.h \
 	../../libbacktrace/posix.c \
 	../../libbacktrace/state.c \
+	../../libiberty/cp-demangle.c \
 	bridge.cc
 
 FORMAT_FILES = \
@@ -362,6 +363,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/alloc.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/atomic.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/bridge.Plo@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/cp-demangle.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/dwarf.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/elf.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/fileline.Plo@am__quote@
@@ -428,6 +430,13 @@ state.lo: ../../libbacktrace/state.c
 @AMDEP_TRUE@@am__fastdepCC_FALSE@	DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
 @am__fastdepCC_FALSE@	$(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o state.lo `test -f '../../libbacktrace/state.c' || echo '$(srcdir)/'`../../libbacktrace/state.c
 
+cp-demangle.lo: ../../libiberty/cp-demangle.c
+@am__fastdepCC_TRUE@	$(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT cp-demangle.lo -MD -MP -MF $(DEPDIR)/cp-demangle.Tpo -c -o cp-demangle.lo `test -f '../../libiberty/cp-demangle.c' || echo '$(srcdir)/'`../../libiberty/cp-demangle.c
+@am__fastdepCC_TRUE@	$(am__mv) $(DEPDIR)/cp-demangle.Tpo $(DEPDIR)/cp-demangle.Plo
+@AMDEP_TRUE@@am__fastdepCC_FALSE@	source='../../libiberty/cp-demangle.c' object='cp-demangle.lo' libtool=yes @AMDEPBACKSLASH@
+@AMDEP_TRUE@@am__fastdepCC_FALSE@	DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
+@am__fastdepCC_FALSE@	$(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o cp-demangle.lo `test -f '../../libiberty/cp-demangle.c' || echo '$(srcdir)/'`../../libiberty/cp-demangle.c
+
 elf.lo: ../../libbacktrace/elf.c
 @am__fastdepCC_TRUE@	$(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT elf.lo -MD -MP -MF $(DEPDIR)/elf.Tpo -c -o elf.lo `test -f '../../libbacktrace/elf.c' || echo '$(srcdir)/'`../../libbacktrace/elf.c
 @am__fastdepCC_TRUE@	$(am__mv) $(DEPDIR)/elf.Tpo $(DEPDIR)/elf.Plo
--- libsanitizer/configure.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/configure	2013-12-10 10:27:35.569870491 +0100
@@ -16134,12 +16134,14 @@ ac_config_commands="$ac_config_commands
 
 
 
-for ac_header in sys/mman.h
+for ac_header in sys/mman.h alloca.h
 do :
-  ac_fn_c_check_header_mongrel "$LINENO" "sys/mman.h" "ac_cv_header_sys_mman_h" "$ac_includes_default"
-if test "x$ac_cv_header_sys_mman_h" = x""yes; then :
+  as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
+ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
+eval as_val=\$$as_ac_Header
+   if test "x$as_val" = x""yes; then :
   cat >>confdefs.h <<_ACEOF
-#define HAVE_SYS_MMAN_H 1
+#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1
 _ACEOF
 
 fi
--- libsanitizer/configure.ac.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/configure.ac	2013-12-10 10:27:25.878930779 +0100
@@ -208,7 +208,7 @@ AC_SUBST(BACKTRACE_SUPPORTED)
 
 GCC_HEADER_STDINT(gstdint.h)
 
-AC_CHECK_HEADERS(sys/mman.h)
+AC_CHECK_HEADERS(sys/mman.h alloca.h)
 if test "$ac_cv_header_sys_mman_h" = "no"; then
   have_mmap=no
 else
--- libsanitizer/config.h.in.jj	2013-12-10 09:55:20.000000000 +0100
+++ libsanitizer/config.h.in	2013-12-10 10:26:53.000000000 +0100
@@ -3,6 +3,9 @@
 /* ELF size: 32 or 64 */
 #undef BACKTRACE_ELF_SIZE
 
+/* Define to 1 if you have the <alloca.h> header file. */
+#undef HAVE_ALLOCA_H
+
 /* Define to 1 if you have the __atomic functions */
 #undef HAVE_ATOMIC_FUNCTIONS
 


	Jakub

Re: [PATCH] libsanitizer demangling using cp-demangle.c

[Dodji Seketeli]

Jakub Jelinek <jakub@redhat.com> a écrit:


>
> 2013-12-10  Jakub Jelinek  <jakub@redhat.com>
>
> 	* sanitizer_common/sanitizer_symbolizer_libbacktrace.h
> 	(LibbacktraceSymbolizer::Demangle): New declaration.
> 	* sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
> 	(POSIXSymbolizer::Demangle): Use libbacktrace_symbolizer_'s Demangle
> 	method if possible.
> 	* sanitizer_common/sanitizer_symbolizer_libbacktrace.cc: Include
> 	"demangle.h" if SANITIZE_CP_DEMANGLE is defined.
> 	(struct CplusV3DemangleData): New type.
> 	(CplusV3DemangleCallback, CplusV3Demangle): New functions.
> 	(SymbolizeCodePCInfoCallback, SymbolizeCodeCallback,
> 	SymbolizeDataCallback): Use CplusV3Demangle.
> 	* sanitizer_common/Makefile.am (AM_CXXFLAGS): Add
> 	-DSANITIZE_CP_DEMANGLE and -I $(top_srcdir)/../include.
> 	* libbacktrace/backtrace-rename.h (cplus_demangle_builtin_types,
> 	cplus_demangle_fill_ctor, cplus_demangle_fill_dtor,
> 	cplus_demangle_fill_extended_operator, cplus_demangle_fill_name,
> 	cplus_demangle_init_info, cplus_demangle_mangled_name,
> 	cplus_demangle_operators, cplus_demangle_print,
> 	cplus_demangle_print_callback, cplus_demangle_type, cplus_demangle_v3,
> 	cplus_demangle_v3_callback, is_gnu_v3_mangled_ctor,
> 	is_gnu_v3_mangled_dtor, java_demangle_v3, java_demangle_v3_callback):
> 	Define.
> 	(__asan_internal_memcmp, __asan_internal_strncmp): New prototypes.
> 	(memcmp, strncmp): Redefine.
> 	* libbacktrace/Makefile.am (libsanitizer_libbacktrace_la_SOURCES): Add
> 	../../libiberty/cp-demangle.c.
> 	* libbacktrace/bridge.cc (__asan_internal_memcmp,
> 	__asan_internal_strncmp): New functions.
> 	* sanitizer_common/Makefile.in: Regenerated.
> 	* libbacktrace/Makefile.in: Regenerated.
> 	* configure: Regenerated.
> 	* configure.ac: Regenerated.
> 	* config.h.in: Regenerated.

This looks good to me.

Thanks.

-- 
		Dodji

Re: [PATCH] libsanitizer demangling using cp-demangle.c

[Konstantin Serebryany]

On Tue, Dec 10, 2013 at 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Dec 06, 2013 at 06:40:52AM -0800, Ian Lance Taylor wrote:
>> There was a recent buggy patch to the demangler that added calls to
>> malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
>> That patch must be fixed or reverted before the 4.9 release.  The main
>> code in the demangler must not call malloc/realloc.
>>
>> When that patch is fixed, you can use the cplus_demangle_v3_callback
>> function to get a demangler that never calls malloc.
>
> AFAIK Gary is working on a fix, when that is fixed, with the following
> patch libsanitizer (when using libbacktrace for symbolization) will not
> use system malloc/realloc/free for the demangling at all.
>
> Tested on x86_64-linux (-m64/-m32).  Note that the changes for the 3 files
> unfortunately will need to be applied upstream to compiler-rt, is that
> possible?
>
> 2013-12-10  Jakub Jelinek  <jakub@redhat.com>
>
>         * sanitizer_common/sanitizer_symbolizer_libbacktrace.h
>         (LibbacktraceSymbolizer::Demangle): New declaration.
>         * sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

sanitizer_symbolizer_posix_libcdep.cc is the file from upstream.
If it gets any change in the GCC variant, I will not be able to do
merges from upstream until the same code is applied upstream.


>         (POSIXSymbolizer::Demangle): Use libbacktrace_symbolizer_'s Demangle
>         method if possible.
>         * sanitizer_common/sanitizer_symbolizer_libbacktrace.cc: Include
>         "demangle.h" if SANITIZE_CP_DEMANGLE is defined.
>         (struct CplusV3DemangleData): New type.
>         (CplusV3DemangleCallback, CplusV3Demangle): New functions.
>         (SymbolizeCodePCInfoCallback, SymbolizeCodeCallback,
>         SymbolizeDataCallback): Use CplusV3Demangle.
>         * sanitizer_common/Makefile.am (AM_CXXFLAGS): Add
>         -DSANITIZE_CP_DEMANGLE and -I $(top_srcdir)/../include.
>         * libbacktrace/backtrace-rename.h (cplus_demangle_builtin_types,
>         cplus_demangle_fill_ctor, cplus_demangle_fill_dtor,
>         cplus_demangle_fill_extended_operator, cplus_demangle_fill_name,
>         cplus_demangle_init_info, cplus_demangle_mangled_name,
>         cplus_demangle_operators, cplus_demangle_print,
>         cplus_demangle_print_callback, cplus_demangle_type, cplus_demangle_v3,
>         cplus_demangle_v3_callback, is_gnu_v3_mangled_ctor,
>         is_gnu_v3_mangled_dtor, java_demangle_v3, java_demangle_v3_callback):
>         Define.
>         (__asan_internal_memcmp, __asan_internal_strncmp): New prototypes.
>         (memcmp, strncmp): Redefine.
>         * libbacktrace/Makefile.am (libsanitizer_libbacktrace_la_SOURCES): Add
>         ../../libiberty/cp-demangle.c.
>         * libbacktrace/bridge.cc (__asan_internal_memcmp,
>         __asan_internal_strncmp): New functions.
>         * sanitizer_common/Makefile.in: Regenerated.
>         * libbacktrace/Makefile.in: Regenerated.
>         * configure: Regenerated.
>         * configure.ac: Regenerated.
>         * config.h.in: Regenerated.
>
> --- libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.h.jj        2013-12-05 12:04:28.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.h   2013-12-10 11:01:26.777371566 +0100
> @@ -29,6 +29,8 @@ class LibbacktraceSymbolizer {
>
>    bool SymbolizeData(DataInfo *info);
>
> +  const char *Demangle(const char *name);
> +
>   private:
>    explicit LibbacktraceSymbolizer(void *state) : state_(state) {}
>
> --- libsanitizer/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc.jj      2013-12-05 12:04:28.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc 2013-12-10 11:03:02.971876505 +0100
> @@ -513,6 +513,11 @@ class POSIXSymbolizer : public Symbolize
>      SymbolizerScope sym_scope(this);
>      if (internal_symbolizer_ != 0)
>        return internal_symbolizer_->Demangle(name);
> +    if (libbacktrace_symbolizer_ != 0) {
> +      const char *demangled = libbacktrace_symbolizer_->Demangle(name);
> +      if (demangled)
> +       return demangled;
> +    }
>      return DemangleCXXABI(name);
>    }
>
> --- libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc.jj       2013-12-09 14:32:06.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_symbolizer_libbacktrace.cc  2013-12-10 11:48:19.803830291 +0100
> @@ -20,6 +20,10 @@
>  # include "backtrace-supported.h"
>  # if SANITIZER_POSIX && BACKTRACE_SUPPORTED && !BACKTRACE_USES_MALLOC
>  #  include "backtrace.h"
> +#  if SANITIZER_CP_DEMANGLE
> +#   undef ARRAY_SIZE
> +#   include "demangle.h"
> +#  endif
>  # else
>  #  define SANITIZER_LIBBACKTRACE 0
>  # endif
> @@ -31,6 +35,60 @@ namespace __sanitizer {
>
>  namespace {
>
> +#if SANITIZER_CP_DEMANGLE
> +struct CplusV3DemangleData {
> +  char *buf;
> +  uptr size, allocated;
> +};
> +
> +extern "C" {
> +static void CplusV3DemangleCallback(const char *s, size_t l, void *vdata) {
> +  CplusV3DemangleData *data = (CplusV3DemangleData *)vdata;
> +  uptr needed = data->size + l + 1;
> +  if (needed > data->allocated) {
> +    data->allocated *= 2;
> +    if (needed > data->allocated)
> +      data->allocated = needed;
> +    char *buf = (char *)InternalAlloc(data->allocated);
> +    if (data->buf) {
> +      internal_memcpy(buf, data->buf, data->size);
> +      InternalFree(data->buf);
> +    }
> +    data->buf = buf;
> +  }
> +  internal_memcpy(data->buf + data->size, s, l);
> +  data->buf[data->size + l] = '\0';
> +  data->size += l;
> +}
> +}  // extern "C"
> +
> +char *CplusV3Demangle(const char *name, bool always_alloc) {
> +  CplusV3DemangleData data;
> +  data.buf = 0;
> +  data.size = 0;
> +  data.allocated = 0;
> +  if (cplus_demangle_v3_callback(name, DMGL_PARAMS | DMGL_ANSI,
> +                                CplusV3DemangleCallback, &data)) {
> +    if (data.size + 64 > data.allocated)
> +      return data.buf;
> +    char *buf = internal_strdup(data.buf);
> +    InternalFree(data.buf);
> +    return buf;
> +  }
> +  if (data.buf)
> +    InternalFree(data.buf);
> +  if (always_alloc)
> +    return internal_strdup(name);
> +  return 0;
> +}
> +#else
> +const char *CplusV3Demangle(const char *name, bool always_alloc) {
> +  if (always_alloc)
> +    return internal_strdup(name);
> +  return 0;
> +}
> +#endif
> +
>  struct SymbolizeCodeData {
>    AddressInfo *frames;
>    uptr n_frames;
> @@ -49,7 +107,7 @@ static int SymbolizeCodePCInfoCallback(v
>      info->Clear();
>      info->FillAddressAndModuleInfo(addr, cdata->module_name,
>                                     cdata->module_offset);
> -    info->function = internal_strdup(function);
> +    info->function = CplusV3Demangle(function, true);
>      if (filename)
>        info->file = internal_strdup(filename);
>      info->line = lineno;
> @@ -67,7 +125,7 @@ static void SymbolizeCodeCallback(void *
>      info->Clear();
>      info->FillAddressAndModuleInfo(addr, cdata->module_name,
>                                     cdata->module_offset);
> -    info->function = internal_strdup(symname);
> +    info->function = CplusV3Demangle(symname, true);
>      cdata->n_frames = 1;
>    }
>  }
> @@ -76,7 +134,7 @@ static void SymbolizeDataCallback(void *
>                                    uintptr_t symval, uintptr_t symsize) {
>    DataInfo *info = (DataInfo *)vdata;
>    if (symname && symval) {
> -    info->name = internal_strdup(symname);
> +    info->name = CplusV3Demangle(symname, true);
>      info->start = symval;
>      info->size = symsize;
>    }
> @@ -121,6 +179,17 @@ bool LibbacktraceSymbolizer::SymbolizeDa
>    return true;
>  }
>
> +const char *LibbacktraceSymbolizer::Demangle(const char *name) {
> +#if SANITIZER_CP_DEMANGLE
> +  const char *demangled = CplusV3Demangle(name, false);
> +  if (demangled)
> +    return demangled;
> +  return name;
> +#else
> +  return 0;
> +#endif
> +}
> +
>  #else  // SANITIZER_LIBBACKTRACE
>
>  LibbacktraceSymbolizer *LibbacktraceSymbolizer::get(LowLevelAllocator *alloc) {
> @@ -139,6 +208,10 @@ bool LibbacktraceSymbolizer::SymbolizeDa
>    return false;
>  }
>
> +const char *LibbacktraceSymbolizer::Demangle(const char *name) {
> +  return 0;
> +}
> +
>  #endif  // SANITIZER_LIBBACKTRACE
>
>  }  // namespace __sanitizer
> --- libsanitizer/sanitizer_common/Makefile.am.jj        2013-12-10 09:56:36.000000000 +0100
> +++ libsanitizer/sanitizer_common/Makefile.am   2013-12-10 10:41:03.745693274 +0100
> @@ -7,8 +7,10 @@ DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_C
>  AM_CXXFLAGS = -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long  -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros
>  AM_CXXFLAGS += $(LIBSTDCXX_RAW_CXX_CXXFLAGS)
>  if LIBBACKTRACE_SUPPORTED
> -AM_CXXFLAGS += -DSANITIZER_LIBBACKTRACE -I $(top_srcdir)/../libbacktrace \
> +AM_CXXFLAGS += -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
> +              -I $(top_srcdir)/../libbacktrace \
>                -I $(top_builddir)/libbacktrace \
> +              -I $(top_srcdir)/../include \
>                -include $(top_srcdir)/libbacktrace/backtrace-rename.h
>  endif
>  ACLOCAL_AMFLAGS = -I m4
> --- libsanitizer/libbacktrace/backtrace-rename.h.jj     2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/libbacktrace/backtrace-rename.h        2013-12-10 11:50:24.186187737 +0100
> @@ -14,25 +14,49 @@
>  #define backtrace_vector_grow __asan_backtrace_vector_grow
>  #define backtrace_vector_release __asan_backtrace_vector_release
>
> +#define cplus_demangle_builtin_types __asan_cplus_demangle_builtin_types
> +#define cplus_demangle_fill_ctor __asan_cplus_demangle_fill_ctor
> +#define cplus_demangle_fill_dtor __asan_cplus_demangle_fill_dtor
> +#define cplus_demangle_fill_extended_operator __asan_cplus_demangle_fill_extended_operator
> +#define cplus_demangle_fill_name __asan_cplus_demangle_fill_name
> +#define cplus_demangle_init_info __asan_cplus_demangle_init_info
> +#define cplus_demangle_mangled_name __asan_cplus_demangle_mangled_name
> +#define cplus_demangle_operators __asan_cplus_demangle_operators
> +#define cplus_demangle_print __asan_cplus_demangle_print
> +#define cplus_demangle_print_callback __asan_cplus_demangle_print_callback
> +#define cplus_demangle_type __asan_cplus_demangle_type
> +#define cplus_demangle_v3 __asan_cplus_demangle_v3
> +#define cplus_demangle_v3_callback __asan_cplus_demangle_v3_callback
> +#define is_gnu_v3_mangled_ctor __asan_is_gnu_v3_mangled_ctor
> +#define is_gnu_v3_mangled_dtor __asan_is_gnu_v3_mangled_dtor
> +#define java_demangle_v3 __asan_java_demangle_v3
> +#define java_demangle_v3_callback __asan_java_demangle_v3_callback
> +
>  #ifndef __cplusplus
>
>  #include <string.h>
>
>  extern void *__asan_internal_memcpy (void *, const void *, size_t);
>  extern void *__asan_internal_memset (void *, int, size_t);
> +extern int __asan_internal_memcmp (const void *, const void *, size_t);
>  extern int __asan_internal_strcmp (const char *, const char *);
> +extern int __asan_internal_strncmp (const char *, const char *, size_t);
>  extern size_t __asan_internal_strlen (const char *);
>  extern size_t __asan_internal_strnlen (const char *, size_t);
>
>  #undef memcpy
>  #undef memset
> +#undef memcmp
>  #undef strcmp
> +#undef strncmp
>  #undef strlen
>  #undef strnlen
>
>  #define memcpy(x,y,z) __asan_internal_memcpy (x, y, z)
>  #define memset(x,y,z) __asan_internal_memset (x, y, z)
> +#define memcmp(x,y,z) __asan_internal_memcmp (x, y, z)
>  #define strcmp(x,y) __asan_internal_strcmp (x, y)
> +#define strncmp(x,y,z) __asan_internal_strncmp (x, y, z)
>  #define strlen(x) __asan_internal_strlen (x)
>  #ifdef HAVE_DECL_STRNLEN
>  #define strnlen(x,y) __asan_internal_strnlen (x, y)
> --- libsanitizer/libbacktrace/Makefile.am.jj    2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/libbacktrace/Makefile.am       2013-12-10 10:07:33.115084811 +0100
> @@ -51,6 +51,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
>         ../../libbacktrace/internal.h \
>         ../../libbacktrace/posix.c \
>         ../../libbacktrace/state.c \
> +       ../../libiberty/cp-demangle.c \
>         bridge.cc
>
>  FORMAT_FILES = \
> --- libsanitizer/libbacktrace/bridge.cc.jj      2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/libbacktrace/bridge.cc 2013-12-10 11:48:49.038678487 +0100
> @@ -52,11 +52,23 @@ __asan_internal_memset (void *dest, int
>  }
>
>  int
> +__asan_internal_memcmp (const void *s1, const void *s2, size_t n)
> +{
> +  return __sanitizer::internal_memcmp (s1, s2, n);
> +}
> +
> +int
>  __asan_internal_strcmp (const char *s1, const char *s2)
>  {
>    return __sanitizer::internal_strcmp (s1, s2);
>  }
>
> +int
> +__asan_internal_strncmp (const char *s1, const char *s2, size_t n)
> +{
> +  return __sanitizer::internal_strncmp (s1, s2, n);
> +}
> +
>  size_t
>  __asan_internal_strlen (const char *str)
>  {
> --- libsanitizer/sanitizer_common/Makefile.in.jj        2013-12-10 10:28:02.000000000 +0100
> +++ libsanitizer/sanitizer_common/Makefile.in   2013-12-10 11:38:37.921836716 +0100
> @@ -35,8 +35,10 @@ POST_UNINSTALL = :
>  build_triplet = @build@
>  host_triplet = @host@
>  target_triplet = @target@
> -@LIBBACKTRACE_SUPPORTED_TRUE@am__append_1 = -DSANITIZER_LIBBACKTRACE -I $(top_srcdir)/../libbacktrace \
> +@LIBBACKTRACE_SUPPORTED_TRUE@am__append_1 = -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
> +@LIBBACKTRACE_SUPPORTED_TRUE@         -I $(top_srcdir)/../libbacktrace \
>  @LIBBACKTRACE_SUPPORTED_TRUE@         -I $(top_builddir)/libbacktrace \
> +@LIBBACKTRACE_SUPPORTED_TRUE@         -I $(top_srcdir)/../include \
>  @LIBBACKTRACE_SUPPORTED_TRUE@         -include $(top_srcdir)/libbacktrace/backtrace-rename.h
>
>  subdir = sanitizer_common
> --- libsanitizer/libbacktrace/Makefile.in.jj    2013-12-10 09:57:05.000000000 +0100
> +++ libsanitizer/libbacktrace/Makefile.in       2013-12-10 11:38:37.858837042 +0100
> @@ -90,7 +90,7 @@ CONFIG_CLEAN_VPATH_FILES =
>  LTLIBRARIES = $(noinst_LTLIBRARIES)
>  am__DEPENDENCIES_1 =
>  am_libsanitizer_libbacktrace_la_OBJECTS = atomic.lo dwarf.lo \
> -       fileline.lo posix.lo state.lo bridge.lo
> +       fileline.lo posix.lo state.lo cp-demangle.lo bridge.lo
>  libsanitizer_libbacktrace_la_OBJECTS =  \
>         $(am_libsanitizer_libbacktrace_la_OBJECTS)
>  DEFAULT_INCLUDES = -I.@am__isrc@ -I$(top_builddir)
> @@ -280,6 +280,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
>         ../../libbacktrace/internal.h \
>         ../../libbacktrace/posix.c \
>         ../../libbacktrace/state.c \
> +       ../../libiberty/cp-demangle.c \
>         bridge.cc
>
>  FORMAT_FILES = \
> @@ -362,6 +363,7 @@ distclean-compile:
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/alloc.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/atomic.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/bridge.Plo@am__quote@
> +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/cp-demangle.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/dwarf.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/elf.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/fileline.Plo@am__quote@
> @@ -428,6 +430,13 @@ state.lo: ../../libbacktrace/state.c
>  @AMDEP_TRUE@@am__fastdepCC_FALSE@      DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
>  @am__fastdepCC_FALSE@  $(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o state.lo `test -f '../../libbacktrace/state.c' || echo '$(srcdir)/'`../../libbacktrace/state.c
>
> +cp-demangle.lo: ../../libiberty/cp-demangle.c
> +@am__fastdepCC_TRUE@   $(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT cp-demangle.lo -MD -MP -MF $(DEPDIR)/cp-demangle.Tpo -c -o cp-demangle.lo `test -f '../../libiberty/cp-demangle.c' || echo '$(srcdir)/'`../../libiberty/cp-demangle.c
> +@am__fastdepCC_TRUE@   $(am__mv) $(DEPDIR)/cp-demangle.Tpo $(DEPDIR)/cp-demangle.Plo
> +@AMDEP_TRUE@@am__fastdepCC_FALSE@      source='../../libiberty/cp-demangle.c' object='cp-demangle.lo' libtool=yes @AMDEPBACKSLASH@
> +@AMDEP_TRUE@@am__fastdepCC_FALSE@      DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
> +@am__fastdepCC_FALSE@  $(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o cp-demangle.lo `test -f '../../libiberty/cp-demangle.c' || echo '$(srcdir)/'`../../libiberty/cp-demangle.c
> +
>  elf.lo: ../../libbacktrace/elf.c
>  @am__fastdepCC_TRUE@   $(LIBTOOL)  --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT elf.lo -MD -MP -MF $(DEPDIR)/elf.Tpo -c -o elf.lo `test -f '../../libbacktrace/elf.c' || echo '$(srcdir)/'`../../libbacktrace/elf.c
>  @am__fastdepCC_TRUE@   $(am__mv) $(DEPDIR)/elf.Tpo $(DEPDIR)/elf.Plo
> --- libsanitizer/configure.jj   2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/configure      2013-12-10 10:27:35.569870491 +0100
> @@ -16134,12 +16134,14 @@ ac_config_commands="$ac_config_commands
>
>
>
> -for ac_header in sys/mman.h
> +for ac_header in sys/mman.h alloca.h
>  do :
> -  ac_fn_c_check_header_mongrel "$LINENO" "sys/mman.h" "ac_cv_header_sys_mman_h" "$ac_includes_default"
> -if test "x$ac_cv_header_sys_mman_h" = x""yes; then :
> +  as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
> +ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
> +eval as_val=\$$as_ac_Header
> +   if test "x$as_val" = x""yes; then :
>    cat >>confdefs.h <<_ACEOF
> -#define HAVE_SYS_MMAN_H 1
> +#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1
>  _ACEOF
>
>  fi
> --- libsanitizer/configure.ac.jj        2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/configure.ac   2013-12-10 10:27:25.878930779 +0100
> @@ -208,7 +208,7 @@ AC_SUBST(BACKTRACE_SUPPORTED)
>
>  GCC_HEADER_STDINT(gstdint.h)
>
> -AC_CHECK_HEADERS(sys/mman.h)
> +AC_CHECK_HEADERS(sys/mman.h alloca.h)
>  if test "$ac_cv_header_sys_mman_h" = "no"; then
>    have_mmap=no
>  else
> --- libsanitizer/config.h.in.jj 2013-12-10 09:55:20.000000000 +0100
> +++ libsanitizer/config.h.in    2013-12-10 10:26:53.000000000 +0100
> @@ -3,6 +3,9 @@
>  /* ELF size: 32 or 64 */
>  #undef BACKTRACE_ELF_SIZE
>
> +/* Define to 1 if you have the <alloca.h> header file. */
> +#undef HAVE_ALLOCA_H
> +
>  /* Define to 1 if you have the __atomic functions */
>  #undef HAVE_ATOMIC_FUNCTIONS
>
>
>
>         Jakub

Re: [PATCH] libsanitizer demangling using cp-demangle.c

[Jakub Jelinek]

On Thu, Jan 09, 2014 at 05:51:05PM +0400, Konstantin Serebryany wrote:
> On Tue, Dec 10, 2013 at 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Fri, Dec 06, 2013 at 06:40:52AM -0800, Ian Lance Taylor wrote:
> >> There was a recent buggy patch to the demangler that added calls to
> >> malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
> >> That patch must be fixed or reverted before the 4.9 release.  The main
> >> code in the demangler must not call malloc/realloc.
> >>
> >> When that patch is fixed, you can use the cplus_demangle_v3_callback
> >> function to get a demangler that never calls malloc.
> >
> > AFAIK Gary is working on a fix, when that is fixed, with the following
> > patch libsanitizer (when using libbacktrace for symbolization) will not
> > use system malloc/realloc/free for the demangling at all.
> >
> > Tested on x86_64-linux (-m64/-m32).  Note that the changes for the 3 files
> > unfortunately will need to be applied upstream to compiler-rt, is that
> > possible?
> >
> > 2013-12-10  Jakub Jelinek  <jakub@redhat.com>
> >
> >         * sanitizer_common/sanitizer_symbolizer_libbacktrace.h
> >         (LibbacktraceSymbolizer::Demangle): New declaration.
> >         * sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
> 
> sanitizer_symbolizer_posix_libcdep.cc is the file from upstream.
> If it gets any change in the GCC variant, I will not be able to do
> merges from upstream until the same code is applied upstream.

Sure, but we are nearing GCC 4.9 stage3 finish and really need to demangle
the libbacktrace provided output.  Has the compiler-rt situation been
cleared up?  Haven't seen any follow-ups after Chandler's reversion.
So, this change is meant to be temporary, with hope that in upstream this
will be resolved, either with the same patch or something similar.

	Jakub

Re: [PATCH] libsanitizer demangling using cp-demangle.c

[Konstantin Serebryany]

On Thu, Jan 9, 2014 at 5:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jan 09, 2014 at 05:51:05PM +0400, Konstantin Serebryany wrote:
>> On Tue, Dec 10, 2013 at 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Fri, Dec 06, 2013 at 06:40:52AM -0800, Ian Lance Taylor wrote:
>> >> There was a recent buggy patch to the demangler that added calls to
>> >> malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
>> >> That patch must be fixed or reverted before the 4.9 release.  The main
>> >> code in the demangler must not call malloc/realloc.
>> >>
>> >> When that patch is fixed, you can use the cplus_demangle_v3_callback
>> >> function to get a demangler that never calls malloc.
>> >
>> > AFAIK Gary is working on a fix, when that is fixed, with the following
>> > patch libsanitizer (when using libbacktrace for symbolization) will not
>> > use system malloc/realloc/free for the demangling at all.
>> >
>> > Tested on x86_64-linux (-m64/-m32).  Note that the changes for the 3 files
>> > unfortunately will need to be applied upstream to compiler-rt, is that
>> > possible?
>> >
>> > 2013-12-10  Jakub Jelinek  <jakub@redhat.com>
>> >
>> >         * sanitizer_common/sanitizer_symbolizer_libbacktrace.h
>> >         (LibbacktraceSymbolizer::Demangle): New declaration.
>> >         * sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
>>
>> sanitizer_symbolizer_posix_libcdep.cc is the file from upstream.
>> If it gets any change in the GCC variant, I will not be able to do
>> merges from upstream until the same code is applied upstream.
>
> Sure, but we are nearing GCC 4.9 stage3 finish and really need to demangle
> the libbacktrace provided output.  Has the compiler-rt situation been
> cleared up?

I hope it just did (see the fresh Chandler's reply).

--kcc

> Haven't seen any follow-ups after Chandler's reversion.
> So, this change is meant to be temporary, with hope that in upstream this
> will be resolved, either with the same patch or something similar.
>
>         Jakub

Re: [PATCH] libsanitizer demangling using cp-demangle.c

[Alexey Samsonov]

I've landed patches for libbacktrace and cp-demangle support in LLVM.
However, they required some changes (e.g. some files LLVM trunk were
modified after the last merge). This means that the next merge to GCC
(IIUC it won't happen anytime soon before GCC 4.9 release) will not be
clean. Sorry for delay.

On Fri, Jan 10, 2014 at 7:56 AM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> On Thu, Jan 9, 2014 at 5:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, Jan 09, 2014 at 05:51:05PM +0400, Konstantin Serebryany wrote:
>>> On Tue, Dec 10, 2013 at 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> > On Fri, Dec 06, 2013 at 06:40:52AM -0800, Ian Lance Taylor wrote:
>>> >> There was a recent buggy patch to the demangler that added calls to
>>> >> malloc and realloc (2013-10-25 Gary Benson <gbenson@redhat.com>).
>>> >> That patch must be fixed or reverted before the 4.9 release.  The main
>>> >> code in the demangler must not call malloc/realloc.
>>> >>
>>> >> When that patch is fixed, you can use the cplus_demangle_v3_callback
>>> >> function to get a demangler that never calls malloc.
>>> >
>>> > AFAIK Gary is working on a fix, when that is fixed, with the following
>>> > patch libsanitizer (when using libbacktrace for symbolization) will not
>>> > use system malloc/realloc/free for the demangling at all.
>>> >
>>> > Tested on x86_64-linux (-m64/-m32).  Note that the changes for the 3 files
>>> > unfortunately will need to be applied upstream to compiler-rt, is that
>>> > possible?
>>> >
>>> > 2013-12-10  Jakub Jelinek  <jakub@redhat.com>
>>> >
>>> >         * sanitizer_common/sanitizer_symbolizer_libbacktrace.h
>>> >         (LibbacktraceSymbolizer::Demangle): New declaration.
>>> >         * sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
>>>
>>> sanitizer_symbolizer_posix_libcdep.cc is the file from upstream.
>>> If it gets any change in the GCC variant, I will not be able to do
>>> merges from upstream until the same code is applied upstream.
>>
>> Sure, but we are nearing GCC 4.9 stage3 finish and really need to demangle
>> the libbacktrace provided output.  Has the compiler-rt situation been
>> cleared up?
>
> I hope it just did (see the fresh Chandler's reply).
>
> --kcc
>
>> Haven't seen any follow-ups after Chandler's reversion.
>> So, this change is meant to be temporary, with hope that in upstream this
>> will be resolved, either with the same patch or something similar.
>>
>>         Jakub



-- 
Alexey Samsonov, MSK

-Og bug? (was: [PATCH] libsanitizer demangling using cp-demangle.c)

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]

Hi!

This got committed to trunk as r206477; one small nit:

On Tue, 10 Dec 2013 12:38:34 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> 	* libbacktrace/Makefile.am (libsanitizer_libbacktrace_la_SOURCES): Add
> 	../../libiberty/cp-demangle.c.

Trying to build trunk r207180 with C*FLAGS='-Og -ggdb', a compiler
warning in cp-demangle.c (as seen for other instances of cp-demangle.c
being built) is promoted to a hard error in the libsanitizer context, due
to -Werror usage:

    libtool: compile:  [...]/build/./gcc/xgcc -B[...]/build/./gcc/ -B[...]/install/x86_64-unknown-linux-gnu/bin/ -B[...]/install/x86_64-unknown-linux-gnu/lib/ -isystem [...]/install/x86_64-unknown-linux-gnu/include -isystem [...]/install/x86_64-unknown-linux-gnu/sys-include -DHAVE_CONFIG_H -I. -I../../../../source/libsanitizer/libbacktrace -I.. -I ../../../../source/libsanitizer/../include -I ../../../../source/libsanitizer/../libgcc -I ../../libgcc -I .. -I ../../../../source/libsanitizer -I ../../../../source/libsanitizer/../libbacktrace -W -Wall -Wwrite-strings -Wmissing-format-attribute -Wcast-qual -Werror -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -g -O2 -Og -ggdb -MT cp-demangle.lo -MD -MP -MF .deps/cp-demangle.Tpo -c ../../../../source/libsanitizer/libbacktrace/../../libiberty/cp-demangle.c  -fPIC -DPIC -o .libs/cp-demangle.o
    ../../../../source/libsanitizer/libbacktrace/../../libiberty/cp-demangle.c: In function 'd_demangle_callback':
    ../../../../source/libsanitizer/libbacktrace/../../libiberty/cp-demangle.c:5842:14: error: 'dc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
                  : 0;
                  ^
    cc1: all warnings being treated as errors
    make[4]: *** [cp-demangle.lo] Error 1
    make[4]: Leaving directory `[...]/build/x86_64-unknown-linux-gnu/libsanitizer/libbacktrace'

GCC fails to track that all the possible values for enum type indeed have
been covered, and so dc must have been initialized.  As the warning/error
does not appear with -O0, is this in fact a -Og bug?  If not, solve this
by initializing dc to NULL (lame...) ;-), or maybe as follows?

Avoid "'dc' may be uninitialized" warning.

	libiberty/
	* cp-demangle.c (d_demangle_callback): Put __builtin_unreachable
	in place, to help the compiler.

--- libiberty/cp-demangle.c
+++ libiberty/cp-demangle.c
@@ -5824,6 +5824,8 @@ d_demangle_callback (const char *mangled, int options,
 			  NULL);
 	d_advance (&di, strlen (d_str (&di)));
 	break;
+      default:
+	__builtin_unreachable ();
       }
 
     /* If DMGL_PARAMS is set, then if we didn't consume the entire


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: -Og bug? (was: [PATCH] libsanitizer demangling using cp-demangle.c)

[Ian Lance Taylor]

On Tue, Jan 28, 2014 at 6:36 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Avoid "'dc' may be uninitialized" warning.
>
>         libiberty/
>         * cp-demangle.c (d_demangle_callback): Put __builtin_unreachable
>         in place, to help the compiler.
>
> --- libiberty/cp-demangle.c
> +++ libiberty/cp-demangle.c
> @@ -5824,6 +5824,8 @@ d_demangle_callback (const char *mangled, int options,
>                           NULL);
>         d_advance (&di, strlen (d_str (&di)));
>         break;
> +      default:
> +       __builtin_unreachable ();

You can't call __builtin_unreachable in this code, because libiberty
in stage 1 will be compiled by the host compiler and
__builtin_unreachable is specific to GCC.

This patch is OK if you call abort instead of __builtin_unreachable.

Thanks.

Ian

Re: -Og bug?

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]

Hi!

On Tue, 28 Jan 2014 06:52:30 -0800, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Jan 28, 2014 at 6:36 AM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
> > Avoid "'dc' may be uninitialized" warning.
> >
> >         libiberty/
> >         * cp-demangle.c (d_demangle_callback): Put __builtin_unreachable
> >         in place, to help the compiler.

For my own education: why is this not considered a GCC trunk bug?  It is
xgcc/cc1 which is coming up with this (bogus?) warning, but only for -Og
and not for -O0, -O1, etc.?

> > --- libiberty/cp-demangle.c
> > +++ libiberty/cp-demangle.c
> > @@ -5824,6 +5824,8 @@ d_demangle_callback (const char *mangled, int options,
> >                           NULL);
> >         d_advance (&di, strlen (d_str (&di)));
> >         break;
> > +      default:
> > +       __builtin_unreachable ();
> 
> You can't call __builtin_unreachable in this code, because libiberty
> in stage 1 will be compiled by the host compiler and
> __builtin_unreachable is specific to GCC.

Right, thanks for catching that.

> This patch is OK if you call abort instead of __builtin_unreachable.

As soon as I'm clear that this is not in fact a GCC bug, I'll commit the
following.  <stdlib.h> already is being included.  Source code comment
snatched from regex.c.

Avoid "'dc' may be uninitialized" warning.

	libiberty/
	* cp-demangle.c (d_demangle_callback): Put an abort call in
	place, to help the compiler.

--- libiberty/cp-demangle.c
+++ libiberty/cp-demangle.c
@@ -5824,6 +5824,8 @@ d_demangle_callback (const char *mangled, int options,
 			  NULL);
 	d_advance (&di, strlen (d_str (&di)));
 	break;
+      default:
+	abort (); /* We have listed all the cases.  */
       }
 
     /* If DMGL_PARAMS is set, then if we didn't consume the entire


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: -Og bug?

[Ian Lance Taylor]

On Tue, Jan 28, 2014 at 8:11 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> On Tue, 28 Jan 2014 06:52:30 -0800, Ian Lance Taylor <iant@google.com> wrote:
>> On Tue, Jan 28, 2014 at 6:36 AM, Thomas Schwinge
>> <thomas@codesourcery.com> wrote:
>> > Avoid "'dc' may be uninitialized" warning.
>> >
>> >         libiberty/
>> >         * cp-demangle.c (d_demangle_callback): Put __builtin_unreachable
>> >         in place, to help the compiler.
>
> For my own education: why is this not considered a GCC trunk bug?  It is
> xgcc/cc1 which is coming up with this (bogus?) warning, but only for -Og
> and not for -O0, -O1, etc.?

I don't really have an opinion on whether this is a bug in GCC or
not.  Since libiberty is compiled by other compilers, I think your
cp-demangle.c patch is reasonable and appropriate either way.

In particular, it's not a bug for the compiler to consider the
possibility that type may take on a value not named in the enum.
C/C++ impose no restrictions on values of enum type.  It's valid to
write code that stores a value that is not an enum constant into a
variable of enum type, so it's reasonable for the compiler to consider
the possibility, even though we can clearly see that it can not
happen.

I don't know why the compiler reports a different warning for -O1 and
-Og.  I encourage you to reduce the code into a standalone test case
and file a bug report.

Ian

Re: -Og bug?

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

Hi!

On Tue, 28 Jan 2014 09:12:44 -0800, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Jan 28, 2014 at 8:11 AM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
> > On Tue, 28 Jan 2014 06:52:30 -0800, Ian Lance Taylor <iant@google.com> wrote:
> >> On Tue, Jan 28, 2014 at 6:36 AM, Thomas Schwinge
> >> <thomas@codesourcery.com> wrote:
> >> > Avoid "'dc' may be uninitialized" warning.
> >> >
> >> >         libiberty/
> >> >         * cp-demangle.c (d_demangle_callback): Put __builtin_unreachable
> >> >         in place, to help the compiler.
> >
> > For my own education: why is this not considered a GCC trunk bug?  It is
> > xgcc/cc1 which is coming up with this (bogus?) warning, but only for -Og
> > and not for -O0, -O1, etc.?
> 
> I don't really have an opinion on whether this is a bug in GCC or
> not.  Since libiberty is compiled by other compilers, I think your
> cp-demangle.c patch is reasonable and appropriate either way.
> 
> In particular, it's not a bug for the compiler to consider the
> possibility that type may take on a value not named in the enum.
> C/C++ impose no restrictions on values of enum type.  It's valid to
> write code that stores a value that is not an enum constant into a
> variable of enum type, so it's reasonable for the compiler to consider
> the possibility, even though we can clearly see that it can not
> happen.

OK, I agree to all of that, but I'd assume that if the compiler doesn't
do such value tracking to see whether all cases have been covered, it
also souldn't emit such possibly unitialized warning, to not cause false
positive warnings.

> I don't know why the compiler reports a different warning for -O1 and
> -Og.  I encourage you to reduce the code into a standalone test case
> and file a bug report.

<http://gcc.gnu.org/PR59970>.  Will try to continue with that one, but at
low priority.


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: -Og bug?

[Ian Lance Taylor]

`On Tue, Jan 28, 2014 at 1:10 PM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
>
> OK, I agree to all of that, but I'd assume that if the compiler doesn't
> do such value tracking to see whether all cases have been covered, it
> also souldn't emit such possibly unitialized warning, to not cause false
> positive warnings.

The -Wuninitialized warning is full of false positives.

It is the canonical example of why warnings that are driven by
optimizations are difficult for users in practice.

Ian

Re: -Og bug?

[Richard Biener]

On Tue, Jan 28, 2014 at 10:24 PM, Ian Lance Taylor <iant@google.com> wrote:
> `On Tue, Jan 28, 2014 at 1:10 PM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
>>
>> OK, I agree to all of that, but I'd assume that if the compiler doesn't
>> do such value tracking to see whether all cases have been covered, it
>> also souldn't emit such possibly unitialized warning, to not cause false
>> positive warnings.
>
> The -Wuninitialized warning is full of false positives.
>
> It is the canonical example of why warnings that are driven by
> optimizations are difficult for users in practice.

Indeed.  In this case it's of course the "optimistic" data-flow done by
the -Wuninit pass - if it were to assume that a value is initialized if
it cannot prove it isn't then we'd get no false positives but also a lot
of false negatives.  Currently if it cannot prove it is initialized on a path
the pass assumes it is uninitialized on it.

As always you could do both dataflow kinds and add an extra "maybe"
before cases where both analyses do not agree.

Note that the current "maybe" is supposed to mean that there exists
a path to the use where the value seems to be(!) uninitialized.  In contrast
to there exists a path to the use where the value _is_ uninitialized or
even "all paths to the use have the value uninitialized" (the cases where
no maybe is emitted).

Richard.

> Ian