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

[Alexey Samsonov]

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