On Mon, Nov 25, 2013 at 7:02 PM, Jakub Jelinek <ja...@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