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

Eric Fiselier eric at efcs.ca
Mon Oct 20 19:13:15 PDT 2014 

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

More information about the cfe-commits mailing list