On Fri, Dec 6, 2013 at 11:50 AM, Jakub Jelinek <ja...@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 <ja...@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
