[PATCH] D25402: [Driver] Pass -lunwind along with compiler-rt when necessary on Linux

2017-03-08 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

@compnerd, I presume that the option would do nothing when using 
`-rtlib=libgcc`, correct? Or do we support combining libgcc with another 
unwinder?


https://reviews.llvm.org/D25402



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25402: [Driver] Pass -lunwind along with compiler-rt when necessary on Linux

2017-01-26 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.

This really needs a new driver flag (`-unwinder`?) similar to `-rtlib`, as 
there are multiple unwinders, and it is unclear which unwinder is the proper 
one on a given target.  This has been something on my TODO list for a while.  
Mixing and matching undwinders is not really possible, and it is perfectly 
valid to use compiler-rt with the gcc unwinder on Linux.


https://reviews.llvm.org/D25402



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25402: [Driver] Pass -lunwind along with compiler-rt when necessary on Linux

2017-01-26 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 85875.
mgorny retitled this revision from "[Driver] Pass -lunwind when using libc++ + 
compiler-rt on Linux" to "[Driver] Pass -lunwind along with compiler-rt when 
necessary on Linux".
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added subscribers: srhines, danalbert.

Here's a v2. It turns out that you also need `-lunwind` when using to link C 
programs with `-static -rtlib=compiler-rt`. I've also disabled the changes for 
Android targets.


https://reviews.llvm.org/D25402

Files:
  lib/Driver/Tools.cpp
  test/Driver/linux-ld.c


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -443,6 +443,34 @@
 // CHECK-BASIC-LIBCXX-C-LINK: "--sysroot=[[SYSROOT]]"
 // CHECK-BASIC-LIBCXX-C-LINK: "-L[[SYSROOT]]/usr/bin/../lib"
 //
+// Test that libc++ combined with compiler-rt includes -lunwind
+// (and no gcc libraries).
+// RUN: %clangxx -no-canonical-prefixes -x c++ %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu \
+// RUN: -stdlib=libc++ -rtlib=compiler-rt \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-BASIC-LIBCXX-CLANGRT %s
+// CHECK-BASIC-LIBCXX-CLANGRT: "-lc++"
+// CHECK-BASIC-LIBCXX-CLANGRT: "-lunwind"
+// CHECK-BASIC-LIBCXX-CLANGRT-NOT: "-lgcc_s"
+// CHECK-BASIC-LIBCXX-CLANGRT-NOT: "-lgcc"
+// CHECK-BASIC-LIBCXX-CLANGRT-NOT: "-lgcc_eh"
+//
+// Test that compiler-rt with -static includes -lunwind.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: --rtlib=compiler-rt \
+// RUN: -static \
+// RUN:   | FileCheck --check-prefix=CHECK-BASIC-STATIC-CLANGRT %s
+// CHECK-BASIC-STATIC-CLANGRT: "-lunwind"
+// CHECK-BASIC-STATIC-CLANGRT-NOT: "-lgcc_s"
+// CHECK-BASIC-STATIC-CLANGRT-NOT: "-lgcc"
+// CHECK-BASIC-STATIC-CLANGRT-NOT: "-lgcc_eh"
+//
 // Test a very broken version of multiarch that shipped in Ubuntu 11.04.
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=i386-unknown-linux \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10220,6 +10220,7 @@
   // The profile runtime also needs access to system libraries.
   getToolChain().addProfileRTLibs(Args, CmdArgs);
 
+  bool NeedsUnwinder = false;
   if (D.CCCIsCXX() &&
   !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
@@ -10230,14 +10231,20 @@
 if (OnlyLibstdcxxStatic)
   CmdArgs.push_back("-Bdynamic");
 CmdArgs.push_back("-lm");
+// C++ always needs some unwinder
+NeedsUnwinder = true;
   }
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static)) {
 CmdArgs.push_back("--start-group");
+// when linking compiler-rt statically, we also need unwinder
+// (note: this is implicit no-op for libgcc builds)
+NeedsUnwinder = true;
+  }
 
   if (NeedsSanitizerDeps)
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
@@ -10278,6 +10285,11 @@
   }
 
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
+  // if unwinder is needed and we're using compiler-rt we need
+  // to explicitly link -lunwind in (libgcc has its own unwinder)
+  if (NeedsUnwinder && !isAndroid &&
+  ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT)
+CmdArgs.push_back("-lunwind");
 
   if (WantPthread && !isAndroid)
 CmdArgs.push_back("-lpthread");


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -443,6 +443,34 @@
 // CHECK-BASIC-LIBCXX-C-LINK: "--sysroot=[[SYSROOT]]"
 // CHECK-BASIC-LIBCXX-C-LINK: "-L[[SYSROOT]]/usr/bin/../lib"
 //
+// Test that libc++ combined with compiler-rt includes -lunwind
+// (and no gcc libraries).
+// RUN: %clangxx -no-canonical-prefixes -x c++ %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu \
+// RUN: -stdlib=libc++ -rtlib=compiler-rt \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-BASIC-LIBCXX-CLANGRT %s
+// CHECK-BASIC-LIBCXX-CLANGRT: "-lc++"
+// CHECK-BASIC-LIBCXX-CLANGRT: "-lunwind"