r218541 - Don't link in sanitizer runtimes if -nostdlib/-nodefaultlibs is provided.

Alexey Samsonov vonosmas at gmail.com
Wed Oct 22 17:57:00 PDT 2014 

On Tue, Oct 21, 2014 at 10:42 AM, Filipe Cabecinhas <me at filcab.net> 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 <eric at efcs.ca> 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 <vonosmas at gmail.com>
>> wrote:
>>
>>>
>>> On Mon, Oct 20, 2014 at 5:01 PM, Eric Fiselier <eric at efcs.ca> 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 <me at filcab.net>
>>>> 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 <eric at efcs.ca> 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 <vonosmas at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Mon, Oct 20, 2014 at 2:43 PM, Eric Fiselier <eric at efcs.ca> 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 <eric at efcs.ca>
>>>>>>>> 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 <
>>>>>>>>> vonosmas at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, Oct 17, 2014 at 2:53 PM, Eric Fiselier <eric at efcs.ca>
>>>>>>>>>> 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 <
>>>>>>>>>>> vonosmas at gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Oct 17, 2014 at 1:11 PM, Filipe Cabecinhas <
>>>>>>>>>>>> filcab at filcab.net> 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 <eric at efcs.ca>
>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>>> vonosmas at gmail.com> 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
>>>>>>>>>>>>>>> cfe-commits at cs.uiuc.edu
>>>>>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Alexey Samsonov
>>>>>>>>>>>> vonosmas at gmail.com
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Alexey Samsonov
>>>>>>>>>> vonosmas at gmail.com
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Alexey Samsonov
>>>>>>> vonosmas at gmail.com
>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Alexey Samsonov
>>> vonosmas at gmail.com
>>>
>>
>>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141022/cbea3964/attachment.html>

More information about the cfe-commits mailing list