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

Filipe Cabecinhas me at filcab.net
Tue Oct 21 10:42:05 PDT 2014 

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 <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
> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>> wrote:
>
>>
>> On Mon, Oct 20, 2014 at 5:01 PM, Eric Fiselier <eric at efcs.ca
>> <javascript:_e(%7B%7D,'cvml','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
>>> <javascript:_e(%7B%7D,'cvml','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
>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>> wrote:
>>>>>
>>>>>>
>>>>>> On Mon, Oct 20, 2014 at 2:43 PM, Eric Fiselier <eric at efcs.ca
>>>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Fri, Oct 17, 2014 at 2:53 PM, Eric Fiselier <eric at efcs.ca
>>>>>>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Oct 17, 2014 at 1:11 PM, Filipe Cabecinhas <
>>>>>>>>>>> filcab at filcab.net
>>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','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
>>>>>>>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Alexey Samsonov
>>>>>>>>> vonosmas at gmail.com
>>>>>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Alexey Samsonov
>>>>>> vonosmas at gmail.com
>>>>>> <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at cs.uiuc.edu
>>>>> <javascript:_e(%7B%7D,'cvml','cfe-commits at cs.uiuc.edu');>
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com <javascript:_e(%7B%7D,'cvml','vonosmas at gmail.com');>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141021/971d50bc/attachment.html>

More information about the cfe-commits mailing list