Yes, please. It's not perfect, but I think it would be much better than having the -fsanitize=... flag be silently ignored if -nodefaultlibs is present.
Alexey? Thanks, Filipe On Monday, October 20, 2014, Eric Fiselier <[email protected]> wrote: > 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] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: > >> >> On Mon, Oct 20, 2014 at 5:01 PM, Eric Fiselier <[email protected] >> <javascript:_e(%7B%7D,'cvml','[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] >>> <javascript:_e(%7B%7D,'cvml','[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] >>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: >>>>> >>>>>> >>>>>> On Mon, Oct 20, 2014 at 2:43 PM, Eric Fiselier <[email protected] >>>>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Oct 17, 2014 at 2:53 PM, Eric Fiselier <[email protected] >>>>>>>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>>>>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, Oct 17, 2014 at 1:11 PM, Filipe Cabecinhas < >>>>>>>>>>> [email protected] >>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','[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] >>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Alexey Samsonov >>>>>>>>> [email protected] >>>>>>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Alexey Samsonov >>>>>> [email protected] >>>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');> >>>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> cfe-commits mailing list >>>>> [email protected] >>>>> <javascript:_e(%7B%7D,'cvml','[email protected]');> >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>> >>>>> >>>> >>> >> >> >> -- >> Alexey Samsonov >> [email protected] <javascript:_e(%7B%7D,'cvml','[email protected]');> >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
