Re: [PATCH] Use libbacktrace as libsanitizer's symbolizer

[Alexey Samsonov <samsonov at google dot com>]

On Mon, Nov 25, 2013 at 7:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Nov 25, 2013 at 06:53:59PM +0400, Alexey Samsonov wrote:
>> > In GCC, libbacktrace is built as a libtool convenience library only and
>> > then linked into whatever libraries want to use it.  So indeed, the plan
>> > is to link libbacktrace.la into libasan.so.1.0.0 and libasan.a
>> > (and the earlier GCC patch I've posted included corresponding toplevel
>> > configure/Makefile bits and libsanitizer Makefile/configure changes to make
>> > it happen).  E.g. libgo.so.* links libbacktrace.la into itself too.
>> > So, for GCC user experience, things will work automatically.
>>
>> Good. Note, however, that you'd need at least two versions of
>> libbacktrace - 32-bit and 64-bit.
>
> In GCC you get that automatically (unless --disable-multilib, but then you
> don't get 32-bit libsanitizer either say on x86_64-linux).
>
>> > it means finding some way in the build system/configure to detect
>> > or request libbacktrace availability (for libraries not included in
>> > GCC tree GCC's configure typically uses --with-<package>=<path>
>> > or --with-<package>-lib=<path>/--with-<package>-include=<path>) and just
>> > link against it and you'd just unpack/configure/build libbacktrace
>> > on one of your test bots.
>>
>> We indeed can add support for detecting presence of libbacktrace
>> binaries and headers
>> when we configure a build tree. But to "just link against it" we have
>> to either link against it in Clang
>> driver (I don't think we should add support for it), or link against
>> it in all the places where we build something
>> with "-fsanitize=foo", and there are quite a few of them.
>
> Or change the rules for creation of *sanitizer_common.*a, so that you
> link the libbacktrace.la convenience library into it (if configure found
> it).  If you build it using libtool/automake, you get it automatically,
> otherwise you can e.g. using ar unpack libbacktrace.a and note the filenames
> of the object files from it, then add those to the ar command line for
> creation of *sanitizer_common.*a.

This would be messy, especially assuming that we have two (both pretty ugly)
build systems in compiler-rt repository. And the argument about two
versions (32-bit and 64-bit) of
libbacktrace is more serious in this setting. Also, see note below.

>> As for redirecting libc calls from libbacktrace to our internal
>> versions (or interceptors) -
>> if it works fine for our toy tests (which is the case), I suggest to
>> wait for the actual problems gcc
>> users may discover. If there will be need in compiling a separate
>> sanitizer-specific version of libbacktrace,
>> or providing an additional layer between libbacktrace and sanitizer
>> runtimes, or both, this could probably
>> be done in gcc version of the runtime only (w/o modifying any existing
>> source files, only adding standalone gcc-specific stuff).
>
> Sure, worst case you keep it untested in your LLVM copy of libsanitizer
> and we'll just need to fix it up during merges if something breaks.
> If it will be used for GCC (and we have a P1 bug so it is a release blocker
> if llvm-symbolizer is still used), then it will be tested there.

Yes, I think that's what we should do in the short term.
I've submitted (slightly edited) version of your patch to compiler-rt
as r195837, I think
it should work after the next merge to gcc. I've done a minimal
testing of this change
(built the runtime with -DSANITIZER_LIBBACKTRACE, headers provided,
libbacktrace.a linked),
and it "kind of" worked - libbacktrace functions were properly picked
up, error callbacks was not called,
but the symbolization didn't work either:

Turns out libbacktrace doesn't work with executables produced by LLVM:
emitted compile unit DIEs don't have neither
"DW_AT_low_pc/DW_AT_high_pc" pair, nor DW_AT_ranges reference.
It means that to build a set of address ranges corresponding to each
compile unit, one actually
has to extract all the subprogram DIEs. I treat this as a bug in
libbacktrace (unless it's goal is to work only
for GCC-produced binaries). That's what I was afraid when Ian
suggested us to fork the sources.

-- 
Alexey Samsonov, MSK