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

Alexey Samsonov vonosmas at gmail.com
Mon Oct 20 18:29:36 PDT 2014 

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141020/3c73f870/attachment.html>

More information about the cfe-commits mailing list