Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FreeBSD] Support -stdlib=libstdc++ #126302

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arichardson
Copy link
Member

The experimental-library-flag.cpp test was failing on FreeBSD builders,
which turned to be caused by missing support for -stdlib=libcstdc++ (and
just using a hardcoded libc++ in all cases).
Simplify FreeBSD::AddCXXStdlibLibArgs() by deferring to the parent class
and dealing with the FreeSBD < 14 profiling support as a special case.

Created using spr 1.3.6-beta.1
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-clang

Author: Alexander Richardson (arichardson)

Changes

The experimental-library-flag.cpp test was failing on FreeBSD builders,
which turned to be caused by missing support for -stdlib=libcstdc++ (and
just using a hardcoded libc++ in all cases).
Simplify FreeBSD::AddCXXStdlibLibArgs() by deferring to the parent class
and dealing with the FreeSBD < 14 profiling support as a special case.


Full diff: https://github.com/llvm/llvm-project/pull/126302.diff

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/FreeBSD.cpp (+5-5)
  • (modified) clang/test/Driver/experimental-library-flag.cpp (+5)
  • (modified) clang/test/Driver/freebsd.cpp (+6-2)
diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index a6d859f0ebfec23..bd6ab0f8aba5768 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -452,12 +452,12 @@ void FreeBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
 
 void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
                                   ArgStringList &CmdArgs) const {
+  Generic_ELF::AddCXXStdlibLibArgs(Args, CmdArgs);
   unsigned Major = getTriple().getOSMajorVersion();
-  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
-
-  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
-  if (Args.hasArg(options::OPT_fexperimental_library))
-    CmdArgs.push_back("-lc++experimental");
+  bool SuffixedLib = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
+  if (SuffixedLib && GetCXXStdlibType(Args) == CST_Libcxx)
+    llvm::replace(CmdArgs, static_cast<const char *>("-lc++"),
+                  static_cast<const char *>("-lc++_p"));
 }
 
 void FreeBSD::AddCudaIncludeArgs(const ArgList &DriverArgs,
diff --git a/clang/test/Driver/experimental-library-flag.cpp b/clang/test/Driver/experimental-library-flag.cpp
index db6a90b50f2553c..62b007516897e4b 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -9,6 +9,11 @@
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s
 
+/// The FreeBSD driver did not support -stdlib=libstdc++ previously, check that it does the right thing here.
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s
+
 // -fexperimental-library must be passed to CC1.
 // CHECK: -fexperimental-library
 
diff --git a/clang/test/Driver/freebsd.cpp b/clang/test/Driver/freebsd.cpp
index dc8c98d3c3cb749..af6b873a387f13c 100644
--- a/clang/test/Driver/freebsd.cpp
+++ b/clang/test/Driver/freebsd.cpp
@@ -1,9 +1,13 @@
 // RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd -stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
 // RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd10.0 -stdlib=platform 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-TEN %s
+// RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: %clangxx %s -### -o %t.o --target=amdf64-unknown-freebsd -stdlib=libc++ 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd -stdlib=libstdc++ 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STDLIBCXX %s
 // CHECK-DEFAULT: "-lc++" "-lm"
-// CHECK-TEN: "-lc++" "-lm"
+// CHECK-STDLIBCXX: "-lstdc++" "-lm"
 
 // RUN: %clangxx %s -### -pg -o %t.o --target=amd64-unknown-freebsd -stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-DEFAULT %s

@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-clang-driver

Author: Alexander Richardson (arichardson)

Changes

The experimental-library-flag.cpp test was failing on FreeBSD builders,
which turned to be caused by missing support for -stdlib=libcstdc++ (and
just using a hardcoded libc++ in all cases).
Simplify FreeBSD::AddCXXStdlibLibArgs() by deferring to the parent class
and dealing with the FreeSBD < 14 profiling support as a special case.


Full diff: https://github.com/llvm/llvm-project/pull/126302.diff

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/FreeBSD.cpp (+5-5)
  • (modified) clang/test/Driver/experimental-library-flag.cpp (+5)
  • (modified) clang/test/Driver/freebsd.cpp (+6-2)
diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index a6d859f0ebfec23..bd6ab0f8aba5768 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -452,12 +452,12 @@ void FreeBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
 
 void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
                                   ArgStringList &CmdArgs) const {
+  Generic_ELF::AddCXXStdlibLibArgs(Args, CmdArgs);
   unsigned Major = getTriple().getOSMajorVersion();
-  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
-
-  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
-  if (Args.hasArg(options::OPT_fexperimental_library))
-    CmdArgs.push_back("-lc++experimental");
+  bool SuffixedLib = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
+  if (SuffixedLib && GetCXXStdlibType(Args) == CST_Libcxx)
+    llvm::replace(CmdArgs, static_cast<const char *>("-lc++"),
+                  static_cast<const char *>("-lc++_p"));
 }
 
 void FreeBSD::AddCudaIncludeArgs(const ArgList &DriverArgs,
diff --git a/clang/test/Driver/experimental-library-flag.cpp b/clang/test/Driver/experimental-library-flag.cpp
index db6a90b50f2553c..62b007516897e4b 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -9,6 +9,11 @@
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s
 
+/// The FreeBSD driver did not support -stdlib=libstdc++ previously, check that it does the right thing here.
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
+// RUN: %clangxx --target=x86_64-unknown-freebsd -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s
+
 // -fexperimental-library must be passed to CC1.
 // CHECK: -fexperimental-library
 
diff --git a/clang/test/Driver/freebsd.cpp b/clang/test/Driver/freebsd.cpp
index dc8c98d3c3cb749..af6b873a387f13c 100644
--- a/clang/test/Driver/freebsd.cpp
+++ b/clang/test/Driver/freebsd.cpp
@@ -1,9 +1,13 @@
 // RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd -stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
 // RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd10.0 -stdlib=platform 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-TEN %s
+// RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: %clangxx %s -### -o %t.o --target=amdf64-unknown-freebsd -stdlib=libc++ 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: %clangxx %s -### -o %t.o --target=amd64-unknown-freebsd -stdlib=libstdc++ 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STDLIBCXX %s
 // CHECK-DEFAULT: "-lc++" "-lm"
-// CHECK-TEN: "-lc++" "-lm"
+// CHECK-STDLIBCXX: "-lstdc++" "-lm"
 
 // RUN: %clangxx %s -### -pg -o %t.o --target=amd64-unknown-freebsd -stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-DEFAULT %s

Created using spr 1.3.6-beta.1
CmdArgs.push_back("-lc++experimental");
bool SuffixedLib = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
if (SuffixedLib && GetCXXStdlibType(Args) == CST_Libcxx)
llvm::replace(CmdArgs, static_cast<const char *>("-lc++"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast here is needed since it doesn't decay and complains about char[5] vs char[7]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how simple the base AddCXXStdlibLibArgs is, should we not just reimplement the other bits here rather than do this slightly gross replacement? That's what all the other implementations seem to do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair but I was thinking it would be good to reuse code as much as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly shorter than copy-pasting so I prefer this approach.

@brad0
Copy link
Contributor

brad0 commented Feb 10, 2025

It's not missing as in like someone forgot. It is intentional. Same goes for FreeBSD and OpenBSD's Drivers. The base OS uses libc++ and anything else is not supported.

@arichardson
Copy link
Member Author

It's not missing as in like someone forgot. It is intentional. Same goes for FreeBSD and OpenBSD's Drivers. The base OS uses libc++ and anything else is not supported.

You can install it from ports though and it should be possible to use that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants