On Tue, Oct 21, 2014 at 10:42 AM, Filipe Cabecinhas <[email protected]> wrote:
> Yes, please. No, we already have -fno-sanitize= flag. If you add "-fsanitize=foo" to your global LD_FLAGS, you can effectively cancel it for a specific linker invocation by appending "-fno-sanitize=foo" to the argument list. > 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? > Looks like we should just revert this change. It's unfortunate, but all the alternatives are worse. Done in r220455. Sorry for all the inconvenience. > > 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]> >> 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] >>> >> >> -- Alexey Samsonov [email protected]
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
