What are the thoughts about going back to linking the static analyzer library and adding a different flag that allows you to drop it? For example '-nosanitizerruntime'.
/Eric On Mon, Oct 20, 2014 at 7:29 PM, Alexey Samsonov <[email protected]> wrote: > > On Mon, Oct 20, 2014 at 5:01 PM, Eric Fiselier <[email protected]> wrote: > >> That works for me, but isn't that the old behavior? >> > > It almost is. Unfortunately, we've seen the cases where -nodefaultlibs is > the only reasonable discriminator which distinguishes linking actual > executable from manual linking of relocatable > file (unless we go and parse flags passed to linker in -Wl, which we > certainly don't want to do), so it's not dynamic system libraries that > cause the problem, it's sanitizer runtime, > which should only be linked once into the final executable, not > intermediate files. > > But I agree it's kind of messed up, and it's questionable we can treat > sanitizer runtimes as "system" libraries. > > >> >> /Eric >> >> On Mon, Oct 20, 2014 at 5:53 PM, Filipe Cabecinhas <[email protected]> wrote: >> >>> I still don't like that -fsanitize=undefined doesn't bring the sanitizer >>> *.a files. They're not default libs for any classification that I can come >>> up with, they're being explicitly linked in. >>> >>> What about this: >>> >>> If both are specified (-fsanitize=blah, and -nodefault libs), then >>> -fsanitize=blah will only pull in the static sanitizer library, but not its >>> dependencies (-lc, -lpthread, etc), since those _could_ be understood as >>> "default libs". It will be up to the user to figure out how to get those >>> symbols (by passing additional linker options), since they have to be >>> available at runtime. >>> >>> That way, we're following "the spirit of" both flags. >>> >>> gcc's manual says: >>> -nodefaultlibs >>> Do not use the standard system libraries when linking. Only the >>> libraries you specify are passed to the linker, and options specifying >>> linkage of the system libraries, such as -static-libgcc or -shared-libgcc, >>> are ignored. The standard startup files are used normally, unless >>> -nostartfiles is used. >>> >>> With that description, it makes total sense that -static-libgcc is >>> ignored. It's not that -nodefaultlibs "wins" over it. It's that >>> -static-libgcc says "if linking with libgcc, use the static version”. >>> >> > I agree, and "-nodefaultlibs" implies "don't link with libgcc". > > >> >>> What do you think about this option? >>> This makes us not have to figure out, outside of clang, where the >>> sanitizer libs are, and makes those -nodefaultlibs cases understandable >>> (and not pull in libraries you weren't expecting). >>> >> > > > >> >>> Thanks, >>> >>> Filipe >>> >>> >>> >>> On Mon, Oct 20, 2014 at 4:37 PM, Eric Fiselier <[email protected]> wrote: >>> >>>> Sorry I was being stupid and had the static sanitizer libraries after >>>> some dynamic ones which caused some linker errors. >>>> Do they need to be wrapped in -whole-archive/-no-whole-archive? Do I >>>> also need to pass the '--dynamic-list' flags? >>>> >>>> Although I got the tests working, getting the CMake build to add the >>>> proper libraries is going to be a lot more difficult. >>>> I would implore you to consider a solution that shifts the work onto >>>> the compiler. >>>> >>>> /Eric >>>> >>>> On Mon, Oct 20, 2014 at 5:31 PM, Alexey Samsonov <[email protected]> >>>> wrote: >>>> >>>>> >>>>> On Mon, Oct 20, 2014 at 2:43 PM, Eric Fiselier <[email protected]> wrote: >>>>> >>>>>> Hi All, >>>>>> >>>>>> I've spent some time trying to work around this. I probe clang to >>>>>> find the libclang_rt libraries but I cant seem to link them correctly. >>>>>> Would anybody be able to offer advice as to how to properly link the >>>>>> static sanitizer libraries? >>>>>> >>>>> >>>>> What do you mean? If you know the location of sanitizer runtimes, you >>>>> can just add them to linker invocation, possibly wrapped in >>>>> -whole-archive/-no-whole-archive. >>>>> Or are you talking about adding one more flag to force linking in >>>>> sanitizer runtimes? >>>>> >>>>> >>>>>> >>>>>> /Eric >>>>>> >>>>>> On Sat, Oct 18, 2014 at 12:06 PM, Eric Fiselier <[email protected]> wrote: >>>>>> >>>>>>> > Would it help if we teach the Clang driver to print the path to >>>>>>> sanitizer runtime libraries (smth. like GCC's -print-libgcc-file-name >>>>>>> flag)? >>>>>>> >>>>>>> That would be one way to solve the problem and probably a good >>>>>>> approach. >>>>>>> I would rather have flags that force the libraries to be linked even >>>>>>> in the presence of '-nodefaultlibs'. >>>>>>> It seems somewhat odd that GCC ignores -static-libgcc with >>>>>>> -nodefaultlibs. >>>>>>> >>>>>>> Either way, with this new behavior there are going to be some >>>>>>> growing pains, but that is our problem. >>>>>>> >>>>>>> On Fri, Oct 17, 2014 at 5:38 PM, Alexey Samsonov <[email protected] >>>>>>> > wrote: >>>>>>> >>>>>>>> >>>>>>>> On Fri, Oct 17, 2014 at 2:53 PM, Eric Fiselier <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> > If I understand correctly, when you pass -fsanitize=undefined >>>>>>>>> (or whatever) to the linker job, all it does is add the library. If >>>>>>>>> this is >>>>>>>>> correct, then it makes no sense to have -nodefaultlibs remove it: >>>>>>>>> it's not >>>>>>>>> a default lib and it was explicitly added by passing -fsanitize to >>>>>>>>> the link >>>>>>>>> job. >>>>>>>>> >>>>>>>>> I agree. It seems to me your asking for the library to be linked >>>>>>>>> in explicitly. However I think the current behavior is acceptable as >>>>>>>>> well. >>>>>>>>> A couple of salient points. >>>>>>>>> 1. As mentioned repeatedly, it isn't always possible to configure >>>>>>>>> c++ projects so they only use -fsanitize when compiling and not >>>>>>>>> linking. >>>>>>>>> 2. There is a need for a way to remove the default sanitizer >>>>>>>>> libraries. >>>>>>>>> 3. GCC also removes the sanitizer libraries when -fsanitize and >>>>>>>>> -nodefaultlibs are given. >>>>>>>>> >>>>>>>>> > Why can't we link with libc++ using -stdlib=libc++ flag? >>>>>>>>> Linking against libc++abi instead of (not "in addition to") libsupc++ >>>>>>>>> (or >>>>>>>>> whatever) might be challenging, though. >>>>>>>>> However, I think it would really make sense to add support for >>>>>>>>> this configuration to Clang driver. It would make your LIT configs >>>>>>>>> simpler >>>>>>>>> (you'll just invoke the Clang with >>>>>>>>> some arguments, instead of linking with libc++ and libc++abi >>>>>>>>> manually), and make usage of libc++/libc++abi easier for end user. >>>>>>>>> >>>>>>>>> I'm currently working on patching the clang driver to better >>>>>>>>> handle linking against libc++ and its many ABI libraries. >>>>>>>>> It will take some work before this approach is viable for testing >>>>>>>>> libc++, and even when it is viable older compilers and GCC will still >>>>>>>>> have >>>>>>>>> to be supported separately. >>>>>>>>> >>>>>>>>> Currently the libc++ test suite handles linking the required >>>>>>>>> libraries works with both GCC and Clang. It is generic and flexible >>>>>>>>> across >>>>>>>>> a wide range of compilers, platforms and ABI library combinations. >>>>>>>>> Changing the way we set up the tests to deal with this will >>>>>>>>> require a fair amount of work and a ton of special cases. I'll look >>>>>>>>> into >>>>>>>>> what we can do to make the change. >>>>>>>>> >>>>>>>> >>>>>>>> Would it help if we teach the Clang driver to print the path to >>>>>>>> sanitizer runtime libraries (smth. like GCC's -print-libgcc-file-name >>>>>>>> flag)? >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks for your time >>>>>>>>> /Eric >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Oct 17, 2014 at 3:24 PM, Alexey Samsonov < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Oct 17, 2014 at 1:11 PM, Filipe Cabecinhas < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> I don't know anything about the go compiler, but it seems to me >>>>>>>>>>> this patch shouldn't be done like this. >>>>>>>>>>> >>>>>>>>>>> If I understand correctly, when you pass -fsanitize=undefined >>>>>>>>>>> (or whatever) to the linker job, all it does is add the library. If >>>>>>>>>>> this is >>>>>>>>>>> correct, then it makes no sense to have -nodefaultlibs remove it: >>>>>>>>>>> it's not >>>>>>>>>>> a default lib and it was explicitly added by passing -fsanitize to >>>>>>>>>>> the link >>>>>>>>>>> job. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -fsanitize=whatever not only adds the static sanitizer runtime >>>>>>>>>> library, but also pulls in its dependencies on system libraries >>>>>>>>>> (-lc, -ldl, >>>>>>>>>> -lpthread, -lrt). It would be weird to add these libraries if the >>>>>>>>>> user >>>>>>>>>> explicitly specified -nodefaultlibs. Generally, it's ok to make this >>>>>>>>>> flag >>>>>>>>>> win other flags, adding libraries explicitly. For example, GCC manual >>>>>>>>>> specifies that "-static-libgcc" is ignored in presence of >>>>>>>>>> "-nodefaultlibs". >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> From what's in the bug report, isn't it possible to change go's >>>>>>>>>>> behaviour by passing it -fsanitize=blah in CFLAGS but now passing >>>>>>>>>>> it in >>>>>>>>>>> LDFLAGS, since it will add it by itself? >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Not really, it's hard to fix the build system in your project if >>>>>>>>>> it builds 10 binaries with LDFLAGS, and 10 more targets with >>>>>>>>>> "LDFLAGS + >>>>>>>>>> -nodefaultlibs + ...".It's nice if the driver is able to handle it >>>>>>>>>> automatically and discard the irrelevant flags in the second case. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> >>>>>>>>>>> Filipe >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Friday, October 17, 2014, Eric Fiselier <[email protected]> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi All, >>>>>>>>>>>> >>>>>>>>>>>> The libc++ LIT test driver uses -nodefaultlibs so that it can >>>>>>>>>>>> properly link against libc++ and the ABI library. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> Why can't we link with libc++ using -stdlib=libc++ flag? Linking >>>>>>>>>> against libc++abi instead of (not "in addition to") libsupc++ (or >>>>>>>>>> whatever) >>>>>>>>>> might be challenging, though. >>>>>>>>>> However, I think it would really make sense to add support for >>>>>>>>>> this configuration to Clang driver. It would make your LIT configs >>>>>>>>>> simpler >>>>>>>>>> (you'll just invoke the Clang with >>>>>>>>>> some arguments, instead of linking with libc++ and libc++abi >>>>>>>>>> manually), and make usage of libc++/libc++abi easier for end user. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Since this patch we can no longer use sanitizers when running >>>>>>>>>>>> our test suite since it's quite difficult to mimic the driver's >>>>>>>>>>>> behavior >>>>>>>>>>>> and manually link in the sanitizer runtimes. >>>>>>>>>>>> >>>>>>>>>>>> I agree with the rational behind your change. >>>>>>>>>>>> However, since it's difficult to manually link the sanitizer >>>>>>>>>>>> runtimes, it would be nice to have a way to force the front end to >>>>>>>>>>>> do it >>>>>>>>>>>> even when -nodefaultlibs is present. >>>>>>>>>>>> I think we should consider adding '-static-lib*san' flags >>>>>>>>>>>> similar to the ones provided by GCC. >>>>>>>>>>>> >>>>>>>>>>>> I'm not very knowledgeable about the clang linker driver so any >>>>>>>>>>>> input is welcome. >>>>>>>>>>>> >>>>>>>>>>>> /Eric >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Fri, Sep 26, 2014 at 3:22 PM, Alexey Samsonov < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Author: samsonov >>>>>>>>>>>>> Date: Fri Sep 26 16:22:08 2014 >>>>>>>>>>>>> New Revision: 218541 >>>>>>>>>>>>> >>>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=218541&view=rev >>>>>>>>>>>>> Log: >>>>>>>>>>>>> Don't link in sanitizer runtimes if -nostdlib/-nodefaultlibs >>>>>>>>>>>>> is provided. >>>>>>>>>>>>> >>>>>>>>>>>>> It makes no sense to link in sanitizer runtimes in this case: >>>>>>>>>>>>> the user >>>>>>>>>>>>> probably doesn't want to see any system/toolchain libs in his >>>>>>>>>>>>> link if he >>>>>>>>>>>>> provides these flags, and the link will most likely fail >>>>>>>>>>>>> anyway - as sanitizer >>>>>>>>>>>>> runtimes depend on libpthread, libdl, libc etc. >>>>>>>>>>>>> >>>>>>>>>>>>> Also, see discussion in >>>>>>>>>>>>> https://code.google.com/p/address-sanitizer/issues/detail?id=344 >>>>>>>>>>>>> >>>>>>>>>>>>> Modified: >>>>>>>>>>>>> cfe/trunk/lib/Driver/Tools.cpp >>>>>>>>>>>>> cfe/trunk/test/Driver/sanitizer-ld.c >>>>>>>>>>>>> >>>>>>>>>>>>> Modified: cfe/trunk/lib/Driver/Tools.cpp >>>>>>>>>>>>> URL: >>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=218541&r1=218540&r2=218541&view=diff >>>>>>>>>>>>> >>>>>>>>>>>>> ============================================================================== >>>>>>>>>>>>> --- cfe/trunk/lib/Driver/Tools.cpp (original) >>>>>>>>>>>>> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Sep 26 16:22:08 2014 >>>>>>>>>>>>> @@ -2243,6 +2243,10 @@ collectSanitizerRuntimes(const ToolChain >>>>>>>>>>>>> // C runtime, etc). Returns true if sanitizer system deps >>>>>>>>>>>>> need to be linked in. >>>>>>>>>>>>> static bool addSanitizerRuntimes(const ToolChain &TC, const >>>>>>>>>>>>> ArgList &Args, >>>>>>>>>>>>> ArgStringList &CmdArgs) { >>>>>>>>>>>>> + // Don't link in any sanitizer runtimes if we have no >>>>>>>>>>>>> system libraries. >>>>>>>>>>>>> + if (Args.hasArg(options::OPT_nostdlib) || >>>>>>>>>>>>> + Args.hasArg(options::OPT_nodefaultlibs)) >>>>>>>>>>>>> + return false; >>>>>>>>>>>>> SmallVector<StringRef, 4> SharedRuntimes, StaticRuntimes, >>>>>>>>>>>>> HelperStaticRuntimes; >>>>>>>>>>>>> collectSanitizerRuntimes(TC, Args, SharedRuntimes, >>>>>>>>>>>>> StaticRuntimes, >>>>>>>>>>>>> >>>>>>>>>>>>> Modified: cfe/trunk/test/Driver/sanitizer-ld.c >>>>>>>>>>>>> URL: >>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sanitizer-ld.c?rev=218541&r1=218540&r2=218541&view=diff >>>>>>>>>>>>> >>>>>>>>>>>>> ============================================================================== >>>>>>>>>>>>> --- cfe/trunk/test/Driver/sanitizer-ld.c (original) >>>>>>>>>>>>> +++ cfe/trunk/test/Driver/sanitizer-ld.c Fri Sep 26 16:22:08 >>>>>>>>>>>>> 2014 >>>>>>>>>>>>> @@ -301,3 +301,10 @@ >>>>>>>>>>>>> // CHECK-LSAN-ASAN-LINUX-NOT: libclang_rt.lsan >>>>>>>>>>>>> // CHECK-LSAN-ASAN-LINUX: libclang_rt.asan-x86_64 >>>>>>>>>>>>> // CHECK-LSAN-ASAN-LINUX-NOT: libclang_rt.lsan >>>>>>>>>>>>> + >>>>>>>>>>>>> +// RUN: %clang -nostdlib -fsanitize=address %s -### -o %t.o >>>>>>>>>>>>> 2>&1 \ >>>>>>>>>>>>> +// RUN: -target x86_64-unknown-linux \ >>>>>>>>>>>>> +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ >>>>>>>>>>>>> +// RUN: | FileCheck --check-prefix=CHECK-NOSTDLIB %s >>>>>>>>>>>>> +// CHECK-NOSTDLIB: "{{.*}}ld{{(.exe)?}}" >>>>>>>>>>>>> +// CHECK-NOSTDLIB-NOT: libclang_rt.asan >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> _______________________________________________ >>>>>>>>>>>>> cfe-commits mailing list >>>>>>>>>>>>> [email protected] >>>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Alexey Samsonov >>>>>>>>>> [email protected] >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Alexey Samsonov >>>>>>>> [email protected] >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Alexey Samsonov >>>>> [email protected] >>>>> >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>> >>> >> > > > -- > Alexey Samsonov > [email protected] >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
