[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Timm Bäder via Phabricator via cfe-commits]

tbaeder abandoned this revision.
tbaeder added a comment.

Thanks for the input everyone.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Making -stdlib=libc++ not affect linker option for `-static` and `-static-pie` 
looks good to me, for the GNU toolchain. They are expected to pass `-lc++ 
-lc++abi` if they use separate `libc++abi.a`.
Just specifying `-lc++abi` does not work with GNU ld and gold anyway.
It may make some users unhappy if we do that for dynamic linking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think the root cause of the push back you're getting is that this is normally 
handled by vendors when they decide how they're going to ship libc++ on their 
platform or in their toolchain.

For example, on Apple platforms, we ship `libc++.dylib`, and we make sure to 
re-export `libc++abi.dylib`'s symbols from `libc++.dylib`. The result is that 
you only need to specify `-lc++` (which the compiler does by default), and 
everything "works". Since we don't ship `libc++.a`, there's no out-of-the-box 
solution to make it work - it's not officially supported. Some folks do diverge 
from that, however we assume they are willing to get their hands dirty and use 
the flags required to make it work (i.e. `-lc++ -lc++abi`). My understanding is 
that the same goes for other platforms, where each has their preferred way of 
shipping libc++. If we were to officially ship `libc++.a` in the SDKs for Apple 
platforms, I could imagine that we'd want to merge `libc++abi.a` into 
`libc++.a`, and there again you'd only need to specify `-static -lc++` and 
everything would work (unless I'm missing something).

Making that work out-of-the-box would be something we'd want to achieve in how 
we're building and shipping libc++, not by modifying the behaviour of the Clang 
driver on multiple platforms.

In D96070#2544297 , @tbaeder wrote:

> [...]
>
> Or should clang just not do anything here and people should be aware they 
> need to pass additional flags when using libc++ (on Linux)?

The TL;DR of what I wrote above is that I believe this is the correct answer. 
My main area is Apple platforms and not Linux, though, so this is just my .02 
as someone who ships libc++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D96070#2546330 , @MaskRay wrote:

> we cannot control `(optional -lc++abi/-lcxxrt/others)`, which is depended by 
> the installation configuration (e.g. 
> LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY, or libc++.a as a linker script 
> (https://bugs.llvm.org/show_bug.cgi?id=46321), or .deplibs) and whether you 
> are specifying --sysroot. Perhaps I had the opportunity to redesign the 
> matter, I would let `-stdlib=libc++` not affect linker options at all. Users 
> just should specify `-lc++` and related options by themselves.

Right, but at least some of these sound like the can or should be solved in 
libc++ then. Reading https://reviews.llvm.org/D60794, I thought this was not 
going to happen and the solution should instead be in clang.

What is the ideal solution here? If `-static -stdlib=libc++` is not meant to 
work, is the working solution documented anywhere that we could link people to? 
If your blog post isn't official of course :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The problem is that we have `-lc++ (optional -lc++abi/-lcxxrt/others) (optional 
-lpthread))` dependency. While we can control `-lpthread` with `-pthread` 
(which does extra things on its own), we cannot control `(optional 
-lc++abi/-lcxxrt/others)`, which is depended by the installation configuration 
(e.g. LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY, or libc++.a as a linker 
script (https://bugs.llvm.org/show_bug.cgi?id=46321), or .deplibs) and whether 
you are specifying --sysroot. Perhaps I had the opportunity to redesign the 
matter, I would let `-stdlib=libc++` not affect linker options at all. Users 
just should specify `-lc++` and related options by themselves.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I can't speak for other systems, but forcing libpthread to be linked is in 
general not desirable as it creates a non-trivial runtime cost, especially when 
 is not used. That's still a pretty common use case of C++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D96070#2544297 , @tbaeder wrote:

> In D96070#2543172 , @mstorsjo wrote:
>
>> In particular, if libcxx is built with `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` 
>> enabled, the libcxxabi static library is merged into libc++.a, making static 
>> linking work just fine.
>
> I can't test this right now, but it is manually specifying pthread not needed 
> in that case?

That would only bundle libcxxabi, not pthreads, so that'd still be required.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D96070#2543172 , @mstorsjo wrote:

> In particular, if libcxx is built with `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` 
> enabled, the libcxxabi static library is merged into libc++.a, making static 
> linking work just fine.

I can't test this right now, but it is manually specifying pthread not needed 
in that case?

In D96070#2543334 , @MaskRay wrote:

> Persoanlly 
> (https://maskray.me/blog/2020-12-12-c++-exception-handling-abi#use-libc-and-libcabi)
>  I use:
>
> `clang++ -stdlib=libc++ -static-libstdc++ -nostdlib++ a.cc 
> -Wl,--push-state,-Bstatic -lc++ -lc++abi -Wl,--pop-state -pthread`
>
> This is still a bit inferior because there is a duplicate -lc++ passed by the 
> driver.
>
> Dynamically link against libc++.so (which depends on libc++abi.so) 
> (additionally specify -pthread if threads are used):
> `clang++ -stdlib=libc++ -nostdlib++ a.cc -lc++ -lc++abi` (clang 
> -stdlib=libc++ a.cc -lc++ -lc++abi does not pass -lm to the linker.)
> Omitting `-lc++abi` most works, but can fail if your program reference some 
> definitions in ` ` directly and you are using gold or 
> LLD.

Are you suggesting clang should do the same?

Or should clang just not do anything here and people should be aware they need 
to pass additional flags when using libc++ (on Linux)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

The goal here is for distributions to be able to build libc++ one way and then 
have it work with clang without requiring that users add additional linker 
flags besides -static or -stdlib=libc++.  Is there a combination of CMake 
arguments we can use when building libc++ to make all static and shared linking 
with libc++ work out of the box?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

It is difficult for clang driver to make the decision because libc++ has 
multiple C++ ABI implementations as @mstorsjo said.

Persoanlly I use:

`clang++ -stdlib=libc++ -static-libstdc++ -nostdlib++ a.cc 
-Wl,--push-state,-Bstatic -lc++ -lc++abi -Wl,--pop-state -pthread`

This is still a bit inferior because there is a duplicate -lc++ passed by the 
driver.

Dynamically link against libc++.so (which depends on libc++abi.so) 
(additionally specify -pthread if threads are used):
`clang++ -stdlib=libc++ -nostdlib++ a.cc -lc++ -lc++abi` (clang -stdlib=libc++ 
a.cc -lc++ -lc++abi does not pass -lm to the linker.)
Omitting `-lc++abi` most works, but can fail if your program reference some 
definitions in ` ` directly and you are using gold or LLD.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The NetBSD part is most definitely not acceptable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I'm not convinced that this is the right way of handling it. Libc++ can be used 
on top of a number of C++ ABI libraries (both libsupc++/libstdc++ and 
libcxxabi), and how they are linked varies with how a toolchain is assembled. 
In particular, if libcxx is built with `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` 
enabled, the libcxxabi static library is merged into libc++.a, making static 
linking work just fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96070/new/

https://reviews.llvm.org/D96070

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: rsmith, EricWF, ldionne, tstellar, hfinkel.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

currently the following fails:

  clang++ -stdlib=libc++ -static ./test.cpp

because libc++ needs libc++abi and pthreads.

This change has already been proposed in a different version, using 
`-static-libc++` in https://reviews.llvm.org/D63329

And https://reviews.llvm.org/D60794 tried to do it in libc++ instead of clang.

This version makes both linking with `-static` as above work, as well as 
`-static-libstdc++ -stdlib=libc++`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96070

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/libcxx-link.cpp
  clang/test/Driver/netbsd.cpp


Index: clang/test/Driver/netbsd.cpp
===
--- clang/test/Driver/netbsd.cpp
+++ clang/test/Driver/netbsd.cpp
@@ -250,7 +250,7 @@
 // S-ARM-7: clang{{.*}}" "-cc1" "-triple" "armv5e-unknown-netbsd7.0.0-eabi"
 // S-ARM-7: ld{{.*}}" "--eh-frame-hdr" "-Bstatic"
 // S-ARM-7: "-o" "a.out" "{{.*}}/usr/lib{{/|}}crt0.o" 
"{{.*}}/usr/lib{{/|}}eabi{{/|}}crti.o"
-// S-ARM-7: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc++" "-lm" "-lc"
+// S-ARM-7: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc++" "-lc++abi" 
"-lm" "-lc"
 // S-ARM-7: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o"
 
 // S-AARCH64: clang{{.*}}" "-cc1" "-triple" "aarch64-unknown-netbsd"
Index: clang/test/Driver/libcxx-link.cpp
===
--- /dev/null
+++ clang/test/Driver/libcxx-link.cpp
@@ -0,0 +1,20 @@
+
+
+// Regular shared link
+// RUN: %clang++ %s -stdlib=libc++
+
+// Static link via -static-libstdc++
+// RUN: %clang++ %s -stdlib=libc++ -static-libstdc++
+
+// Static link via -static
+// RUN: %clang++ %s -stdlib=libc++ -static
+
+// Both
+// RUN: %clang++ %s -stdlib=libc++ -static-libstdc++ -static
+
+#include 
+int main(int argc, char **argv) {
+  std::cout << "Hello Word\n";
+
+  return 0;
+}
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -613,6 +613,14 @@
 // FIXME: Does this really make sense for all GNU toolchains?
 WantPthread = true;
 
+  // libc++ links against pthreads so for static links we need
+  // to supply this manually
+  if (!WantPthread &&
+  getToolChain().GetCXXStdlibType(Args) == ToolChain::CST_Libcxx &&
+  (Args.hasArg(options::OPT_static) ||
+   Args.hasArg(options::OPT_static_libstdcxx)))
+WantPthread = true;
+
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
   if (WantPthread && !isAndroid)
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1041,6 +1041,9 @@
   switch (Type) {
   case ToolChain::CST_Libcxx:
 CmdArgs.push_back("-lc++");
+if (Args.hasArg(options::OPT_static) ||
+Args.hasArg(options::OPT_static_libstdcxx))
+  CmdArgs.push_back("-lc++abi");
 break;
 
   case ToolChain::CST_Libstdcxx:


Index: clang/test/Driver/netbsd.cpp
===
--- clang/test/Driver/netbsd.cpp
+++ clang/test/Driver/netbsd.cpp
@@ -250,7 +250,7 @@
 // S-ARM-7: clang{{.*}}" "-cc1" "-triple" "armv5e-unknown-netbsd7.0.0-eabi"
 // S-ARM-7: ld{{.*}}" "--eh-frame-hdr" "-Bstatic"
 // S-ARM-7: "-o" "a.out" "{{.*}}/usr/lib{{/|}}crt0.o" "{{.*}}/usr/lib{{/|}}eabi{{/|}}crti.o"
-// S-ARM-7: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc++" "-lm" "-lc"
+// S-ARM-7: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc++" "-lc++abi" "-lm" "-lc"
 // S-ARM-7: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o"
 
 // S-AARCH64: clang{{.*}}" "-cc1" "-triple" "aarch64-unknown-netbsd"
Index: clang/test/Driver/libcxx-link.cpp
===
--- /dev/null
+++ clang/test/Driver/libcxx-link.cpp
@@ -0,0 +1,20 @@
+
+
+// Regular shared link
+// RUN: %clang++ %s -stdlib=libc++
+
+// Static link via -static-libstdc++
+// RUN: %clang++ %s -stdlib=libc++ -static-libstdc++
+
+// Static link via -static
+// RUN: %clang++ %s -stdlib=libc++ -static
+
+// Both
+// RUN: %clang++ %s -stdlib=libc++ -static-libstdc++ -static
+
+#include 
+int main(int argc, char **argv) {
+  std::cout << "Hello Word\n";
+
+  return 0;
+}
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -613,6 +613,14 @@
 // FIXME: Does this