[Clang] Take libstdc++ into account during GCC detection #145056
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Generic_GCC::GCCInstallationDetector class always picks the latest available GCC installation directory. This often breaks C++ compilation on systems on which this directory does not contain a libstdc++ installation. On Ubuntu 22.04 systems, for instance, this can happen if the "gcc-12" package gets installed without the corresponding "g++-12" or "libstdc++-12" package in addition to the default "g++-11" installation. This patch changes the GCC installation selection, if compiling for C++ using libstdc++, to consider only GCC installation directories which also contain libstdc++. This is accomplished by enabling the GCCInstallationDetector to reuse the existing functionality for determinig the libstdc++ include directories which had to be decoupled from its existing uses.
frederik-h
changed the title
Contributor
Author
|
See also this related discourse thread. |
* Always prefer GCC installations with libstdc++ Previously, GCC installation directories containing libstdc++ include directories were only preferred if the compiler flags indicated that they might be required. This might lead to different GCC installations directories being used for C and C++ compilation. * Revert to old GCC version detection and inform about future behavior Instead of actually preferring GCC installations containing libstdc++, stick to the old logic. Inform about the directory that will be chosen in the future. * Add test for new warnings * Make DebianMultiarch available to GCCInstallationHasLibStdcxxIncludePaths The correct DebianMultiarch to use in the invocation of GCCInstallation.addGCCLibStdCxxIncludePaths in GCCInstallationHasLibStdcxxIncludePaths depends on the subclass of Generic_GCC from which the latter function has been invoked. This information is only available in a different place. Add a function for converting the triple component of the GCC installation directory to the GCCInstallationDetector. This function is then adjusted from the different Generic_GCC subclasses. * Revert some unnecessary changes and fix minor bugs.
frederik-h
changed the title
llvmbot
added
clang
frederik-h
requested review from
AaronBallman,
MaskRay,
benshi001 and
ian-twilightcoder
Member
|
@llvm/pr-subscribers-clang Author: Frederik Harwath (frederik-h) ChangesThe Generic_GCC::GCCInstallationDetector class picks the GCC installation directory with the largest version number. Since the location of the libstdc++ include directories is tied to the GCC version, this can break C++ compilation if the libstdc++ headers for this particular GCC version are not available. Linux distributions tend to package the libstdc++ headers separately from GCC. This frequently leads to situations in which a newer version of GCC gets installed as a dependency of another package without installing the corresponding libstdc++ package. Clang then fails to compile C++ code because it cannot find the libstdc++ headers. Since libstdc++ headers are in fact installed on the system, the GCC installation continues to work, the user may not be aware of the details of the GCC detection, and the compiler does not recognize the situation and emit a warning, this behavior can be hard to understand - as witnessed by many related bug reports over the years. The goal of this work is to change the GCC detection to prefer GCC installations that contain libstdc++ include directories over those which do not. This should happen regardless of the input language since picking different GCC installations for a build that mixes C and C++ might lead to incompatibilities. This patch does not yet change the automatic GCC installation directory choice. Instead, it does introduce a warning that informs the user about the future change if the chosen GCC installation directory differs from the one that would be chosen if the libstdc++ headers are taken into account. See also this related Discourse discussion: https://discourse.llvm.org/t/rfc-take-libstdc-into-account-during-gcc-detection/86992. Patch is 40.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145056.diff 37 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 34b6c0d7a8acd..c91517f9796ff 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -874,4 +874,9 @@ def warn_drv_openacc_without_cir
: Warning<"OpenACC directives will result in no runtime behavior; use "
"-fclangir to enable runtime effect">,
InGroup<SourceUsesOpenACC>;
+
+def warn_drv_gcc_install_dir_libstdcxx : Warning<
+ "future releases of the clang compiler will prefer GCC installations "
+ "containing libstdc++ include directories; '%0' would be chosen over '%1'">,
+ InGroup<DiagGroup<"gcc-install-dir-libstdcxx">>;
}
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index b8899e78176b4..61c70bd4c0288 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -228,9 +228,6 @@ class ToolChain {
static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
- static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- const Twine &Path);
static void addExternCSystemInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
@@ -244,12 +241,13 @@ class ToolChain {
static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
ArrayRef<StringRef> Paths);
-
static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
const Twine &C = "", const Twine &D = "");
///@}
-
public:
+ static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ const Twine &Path);
virtual ~ToolChain();
// Accessors
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..c8b6283e4dad0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1378,13 +1378,6 @@ void ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
CC1Args.push_back(DriverArgs.MakeArgString(Path));
}
-/// Utility function to add a system include directory to CC1 arguments.
-void ToolChain::addSystemInclude(const ArgList &DriverArgs,
- ArgStringList &CC1Args, const Twine &Path) {
- CC1Args.push_back("-internal-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
-}
-
/// Utility function to add a system include directory with extern "C"
/// semantics to CC1 arguments.
///
@@ -1407,6 +1400,14 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, Path);
}
+/// Utility function to add a system include directory to CC1 arguments.
+/*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
+ ArgStringList &CC1Args,
+ const Twine &Path) {
+ CC1Args.push_back("-internal-isystem");
+ CC1Args.push_back(DriverArgs.MakeArgString(Path));
+}
+
/// Utility function to add a list of system framework directories to CC1.
void ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index 731076d9754a9..5b275c1f8be54 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -433,6 +433,7 @@ const StringRef PossibleAVRLibcLocations[] = {
AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
: Generic_ELF(D, Triple, Args) {
+
GCCInstallation.init(Triple, Args);
if (getCPUName(D, Args, Triple).empty())
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f5e2655857432..21686cc01d180 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2135,10 +2135,11 @@ void Generic_GCC::GCCInstallationDetector::init(
StringRef TripleText =
llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
- Version = GCCVersion::Parse(VersionText);
- GCCTriple.setTriple(TripleText);
- GCCInstallPath = std::string(InstallDir);
- GCCParentLibPath = GCCInstallPath + "/../../..";
+ SelectedInstallation.Version = GCCVersion::Parse(VersionText);
+ SelectedInstallation.GCCTriple.setTriple(TripleText);
+ SelectedInstallation.GCCInstallPath = std::string(InstallDir);
+ SelectedInstallation.GCCParentLibPath =
+ SelectedInstallation.GCCInstallPath + "/../../..";
IsValid = true;
}
return;
@@ -2198,7 +2199,7 @@ void Generic_GCC::GCCInstallationDetector::init(
// Loop over the various components which exist and select the best GCC
// installation available. GCC installs are ranked by version number.
const GCCVersion VersionZero = GCCVersion::Parse("0.0.0");
- Version = VersionZero;
+ SelectedInstallation.Version = VersionZero;
for (const std::string &Prefix : Prefixes) {
auto &VFS = D.getVFS();
if (!VFS.exists(Prefix))
@@ -2226,7 +2227,7 @@ void Generic_GCC::GCCInstallationDetector::init(
}
// Skip other prefixes once a GCC installation is found.
- if (Version > VersionZero)
+ if (SelectedInstallation.Version > VersionZero)
break;
}
}
@@ -2235,14 +2236,17 @@ void Generic_GCC::GCCInstallationDetector::print(raw_ostream &OS) const {
for (const auto &InstallPath : CandidateGCCInstallPaths)
OS << "Found candidate GCC installation: " << InstallPath << "\n";
- if (!GCCInstallPath.empty())
- OS << "Selected GCC installation: " << GCCInstallPath << "\n";
+ if (!SelectedInstallation.GCCInstallPath.empty())
+ OS << "Selected GCC installation: " << SelectedInstallation.GCCInstallPath
+ << "\n";
for (const auto &Multilib : Multilibs)
OS << "Candidate multilib: " << Multilib << "\n";
- if (Multilibs.size() != 0 || !SelectedMultilib.isDefault())
- OS << "Selected multilib: " << SelectedMultilib << "\n";
+ if (Multilibs.size() != 0 ||
+ !SelectedInstallation.SelectedMultilib.isDefault())
+ OS << "Selected multilib: " << SelectedInstallation.SelectedMultilib
+ << "\n";
}
bool Generic_GCC::GCCInstallationDetector::getBiarchSibling(Multilib &M) const {
@@ -2780,14 +2784,50 @@ bool Generic_GCC::GCCInstallationDetector::ScanGCCForMultilibs(
}
Multilibs = Detected.Multilibs;
- SelectedMultilib = Detected.SelectedMultilibs.empty()
- ? Multilib()
- : Detected.SelectedMultilibs.back();
+ SelectedInstallation.SelectedMultilib =
+ Detected.SelectedMultilibs.empty() ? Multilib()
+ : Detected.SelectedMultilibs.back();
BiarchSibling = Detected.BiarchSibling;
return true;
}
+bool Generic_GCC::GCCInstallationDetector::SelectGCCInstallationDirectory(
+ const SmallVector<Generic_GCC::GCCInstallCandidate, 3> &Installations,
+ const ArgList &Args,
+ Generic_GCC::GCCInstallCandidate &SelectedInstallation) const {
+ if (Installations.empty())
+ return false;
+
+ SelectedInstallation =
+ *max_element(Installations, [](const auto &Max, const auto &I) {
+ return I.Version > Max.Version;
+ });
+
+ // FIXME Start selecting installation with libstdc++ in clang 22,
+ // using the current way of selecting the installation as a fallback
+ // only. For now, warn if the installation with libstdc++ differs
+ // from SelectedInstallation.
+ const GCCInstallCandidate *InstallWithIncludes = nullptr;
+ for (const auto &I : Installations) {
+ if ((!InstallWithIncludes || I.Version > InstallWithIncludes->Version) &&
+ GCCInstallationHasLibStdcxxIncludePaths(I, Args))
+ InstallWithIncludes = &I;
+ }
+
+ if (InstallWithIncludes && SelectedInstallation.GCCInstallPath !=
+ InstallWithIncludes->GCCInstallPath)
+ D.Diag(diag::warn_drv_gcc_install_dir_libstdcxx)
+ << InstallWithIncludes->GCCInstallPath
+ << SelectedInstallation.GCCInstallPath;
+
+ // TODO Warn if SelectedInstallation does not contain libstdc++ includes
+ // although compiler flags indicate that it is required (C++ compilation,
+ // libstdc++ not explicitly disabled).
+
+ return true;
+}
+
void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
const llvm::Triple &TargetTriple, const ArgList &Args,
const std::string &LibDir, StringRef CandidateTriple,
@@ -2817,6 +2857,7 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
TargetTriple.getVendor() == llvm::Triple::Freescale ||
TargetTriple.getVendor() == llvm::Triple::OpenEmbedded}};
+ SmallVector<GCCInstallCandidate, 3> Installations;
for (auto &Suffix : Suffixes) {
if (!Suffix.Active)
continue;
@@ -2834,23 +2875,31 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
continue; // Saw this path before; no need to look at it again.
if (CandidateVersion.isOlderThan(4, 1, 1))
continue;
- if (CandidateVersion <= Version)
+ if (CandidateVersion <= SelectedInstallation.Version && IsValid)
continue;
if (!ScanGCCForMultilibs(TargetTriple, Args, LI->path(),
NeedsBiarchSuffix))
continue;
- Version = CandidateVersion;
- GCCTriple.setTriple(CandidateTriple);
+ GCCInstallCandidate Installation;
+ Installation.Version = CandidateVersion;
+ Installation.GCCTriple.setTriple(CandidateTriple);
// FIXME: We hack together the directory name here instead of
// using LI to ensure stable path separators across Windows and
// Linux.
- GCCInstallPath = (LibDir + "/" + LibSuffix + "/" + VersionText).str();
- GCCParentLibPath = (GCCInstallPath + "/../" + Suffix.ReversePath).str();
- IsValid = true;
+ Installation.GCCInstallPath =
+ (LibDir + "/" + LibSuffix + "/" + VersionText).str();
+ Installation.GCCParentLibPath =
+ (Installation.GCCInstallPath + "/../" + Suffix.ReversePath).str();
+ Installation.SelectedMultilib = getMultilib();
+
+ Installations.push_back(Installation);
}
}
+
+ IsValid |=
+ SelectGCCInstallationDirectory(Installations, Args, SelectedInstallation);
}
bool Generic_GCC::GCCInstallationDetector::ScanGentooConfigs(
@@ -2928,10 +2977,12 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig(
NeedsBiarchSuffix))
continue;
- Version = GCCVersion::Parse(ActiveVersion.second);
- GCCInstallPath = GentooPath;
- GCCParentLibPath = GentooPath + std::string("/../../..");
- GCCTriple.setTriple(ActiveVersion.first);
+ SelectedInstallation.Version =
+ GCCVersion::Parse(ActiveVersion.second);
+ SelectedInstallation.GCCInstallPath = GentooPath;
+ SelectedInstallation.GCCParentLibPath =
+ GentooPath + std::string("/../../..");
+ SelectedInstallation.GCCTriple.setTriple(ActiveVersion.first);
IsValid = true;
return true;
}
@@ -3134,8 +3185,9 @@ void Generic_GCC::AddMultilibIncludeArgs(const ArgList &DriverArgs,
// gcc TOOL_INCLUDE_DIR.
const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
std::string LibPath(GCCInstallation.getParentLibPath());
- addSystemInclude(DriverArgs, CC1Args,
- Twine(LibPath) + "/../" + GCCTriple.str() + "/include");
+ ToolChain::addSystemInclude(DriverArgs, CC1Args,
+ Twine(LibPath) + "/../" + GCCTriple.str() +
+ "/include");
const auto &Callback = Multilibs.includeDirsCallback();
if (Callback) {
@@ -3222,12 +3274,14 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
return;
}
-bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
- Twine IncludeSuffix,
- const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- bool DetectDebian) const {
- if (!getVFS().exists(IncludeDir))
+static bool addLibStdCXXIncludePaths(llvm::vfs::FileSystem &vfs,
+ Twine IncludeDir, StringRef Triple,
+ Twine IncludeSuffix,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ bool DetectDebian = false) {
+
+ if (!vfs.exists(IncludeDir))
return false;
// Debian native gcc uses g++-multiarch-incdir.diff which uses
@@ -3239,39 +3293,48 @@ bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
std::string Path =
(Include + "/" + Triple + Dir.substr(Include.size()) + IncludeSuffix)
.str();
- if (DetectDebian && !getVFS().exists(Path))
+ if (DetectDebian && !vfs.exists(Path))
return false;
// GPLUSPLUS_INCLUDE_DIR
- addSystemInclude(DriverArgs, CC1Args, IncludeDir);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir);
// GPLUSPLUS_TOOL_INCLUDE_DIR. If Triple is not empty, add a target-dependent
// include directory.
if (DetectDebian)
- addSystemInclude(DriverArgs, CC1Args, Path);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, Path);
else if (!Triple.empty())
- addSystemInclude(DriverArgs, CC1Args,
- IncludeDir + "/" + Triple + IncludeSuffix);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args,
+ IncludeDir + "/" + Triple + IncludeSuffix);
// GPLUSPLUS_BACKWARD_INCLUDE_DIR
- addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
return true;
}
-bool Generic_GCC::addGCCLibStdCxxIncludePaths(
- const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
- StringRef DebianMultiarch) const {
- assert(GCCInstallation.isValid());
+bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
+ Twine IncludeSuffix,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ bool DetectDebian) const {
+ return ::addLibStdCXXIncludePaths(getVFS(), IncludeDir, Triple, IncludeSuffix,
+ DriverArgs, CC1Args, DetectDebian);
+}
+
+bool Generic_GCC::GCCInstallCandidate::addGCCLibStdCxxIncludePaths(
+ llvm::vfs::FileSystem &vfs, const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args, StringRef DebianMultiarch) const {
// By default, look for the C++ headers in an include directory adjacent to
// the lib directory of the GCC installation. Note that this is expect to be
// equivalent to '/usr/include/c++/X.Y' in almost all cases.
- StringRef LibDir = GCCInstallation.getParentLibPath();
- StringRef InstallDir = GCCInstallation.getInstallPath();
- StringRef TripleStr = GCCInstallation.getTriple().str();
- const Multilib &Multilib = GCCInstallation.getMultilib();
- const GCCVersion &Version = GCCInstallation.getVersion();
+ StringRef LibDir = getParentLibPath();
+ StringRef InstallDir = getInstallPath();
+ StringRef TripleStr = getTriple().str();
+ const Multilib &Multilib = getMultilib();
+ const GCCVersion &Version = getVersion();
// Try /../$triple/include/c++/$version (gcc --print-multiarch is not empty).
- if (addLibStdCXXIncludePaths(
+ if (::addLibStdCXXIncludePaths(
+ vfs,
LibDir.str() + "/../" + TripleStr + "/include/c++/" + Version.Text,
TripleStr, Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
@@ -3279,22 +3342,24 @@ bool Generic_GCC::addGCCLibStdCxxIncludePaths(
// Try /gcc/$triple/$version/include/c++/ (gcc --print-multiarch is not
// empty). Like above but for GCC built with
// --enable-version-specific-runtime-libs.
- if (addLibStdCXXIncludePaths(LibDir.str() + "/gcc/" + TripleStr + "/" +
- Version.Text + "/include/c++/",
- TripleStr, Multilib.includeSuffix(), DriverArgs,
- CC1Args))
+ if (::addLibStdCXXIncludePaths(vfs,
+ LibDir.str() + "/gcc/" + TripleStr + "/" +
+ Version.Text + "/include/c++/",
+ TripleStr, Multilib.includeSuffix(),
+ DriverArgs, CC1Args))
return true;
// Detect Debian g++-multiarch-incdir.diff.
- if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
- DebianMultiarch, Multilib.includeSuffix(),
- DriverArgs, CC1Args, /*Debian=*/true))
+ if (::addLibStdCXXIncludePaths(
+ vfs, LibDir.str() + "/../include/c++/" + Version.Text,
+ DebianMultiarch, Multilib.includeSuffix(), DriverArgs, CC1Args,
+ /*Debian=*/true))
return true;
// Try /../include/c++/$version (gcc --print-multiarch is empty).
- if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
- TripleStr, Multilib.includeSuffix(), DriverArgs,
- CC1Args))
+ if (::addLibStdCXXIncludePaths(
+ vfs, LibDir.str() + "/../include/c++/" + Version.Text, TripleStr,
+ Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
// Otherwise, fall back on a bunch of options which don't use multiarch
@@ -3309,20 +3374,50 @@ bool Generic_GCC::addGCCLibStdCxxIncludePaths(
};
for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {
- if (addLibStdCXXIncludePaths(IncludePath, TripleStr,
- Multilib.includeSuffix(), DriverArgs, CC1Args))
+ if (::addLibStdCXXIncludePaths(vfs, IncludePath, TripleStr,
+ Multilib.includeSuffix(), DriverArgs,
+ CC1Args))
return true;
}
return false;
}
+bool Generic_GCC::GCCInstallationDetector::
+ GCCInstallationHasLibStdcxxIncludePaths(
+ const GCCInstallCandidate &GCCInstallation,
+ const llvm::opt::ArgList &DriverArgs) const {
+ StringRef DebianMultiarch =
+ TripleToDebianMultiarch(GCCInstallation.getTriple());
+
+ // The following function checks for libstdc++ include paths and
+ // adds them to the provided argument list. Here we just need the
+ // check.
+ llvm::opt::ArgStringList dummyCC1Args;
+ return GCCInstallation.addGCCLibStdCxxIncludePaths(
+ D.getVFS(), DriverArgs, dummyCC1Args, DebianMultiarch);
+}
+
+bool Generic_GCC::addGCCLibStdCxxIncludePaths(
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const {
+ assert(GCCInstallation.isValid());
+
+ // Detect Debian g++-multiarch-incdir.diff.
+ StringRef DebianMultiarch =
+ GCCInstallation.TripleToDebianMultiarch(GCCInstallation.getTriple());
+
+ return GCCInstallation.getSelectedInstallation().addGCCLibStdCxxIncludePaths(
+ getVFS(), DriverArgs, CC1Args, DebianMultiarch);
+}
+
void
Generic_GCC::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
- if (GCCInstallati...
[truncated]
|
Member
|
@llvm/pr-subscribers-clang-driver Author: Frederik Harwath (frederik-h) ChangesThe Generic_GCC::GCCInstallationDetector class picks the GCC installation directory with the largest version number. Since the location of the libstdc++ include directories is tied to the GCC version, this can break C++ compilation if the libstdc++ headers for this particular GCC version are not available. Linux distributions tend to package the libstdc++ headers separately from GCC. This frequently leads to situations in which a newer version of GCC gets installed as a dependency of another package without installing the corresponding libstdc++ package. Clang then fails to compile C++ code because it cannot find the libstdc++ headers. Since libstdc++ headers are in fact installed on the system, the GCC installation continues to work, the user may not be aware of the details of the GCC detection, and the compiler does not recognize the situation and emit a warning, this behavior can be hard to understand - as witnessed by many related bug reports over the years. The goal of this work is to change the GCC detection to prefer GCC installations that contain libstdc++ include directories over those which do not. This should happen regardless of the input language since picking different GCC installations for a build that mixes C and C++ might lead to incompatibilities. This patch does not yet change the automatic GCC installation directory choice. Instead, it does introduce a warning that informs the user about the future change if the chosen GCC installation directory differs from the one that would be chosen if the libstdc++ headers are taken into account. See also this related Discourse discussion: https://discourse.llvm.org/t/rfc-take-libstdc-into-account-during-gcc-detection/86992. Patch is 40.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145056.diff 37 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 34b6c0d7a8acd..c91517f9796ff 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -874,4 +874,9 @@ def warn_drv_openacc_without_cir
: Warning<"OpenACC directives will result in no runtime behavior; use "
"-fclangir to enable runtime effect">,
InGroup<SourceUsesOpenACC>;
+
+def warn_drv_gcc_install_dir_libstdcxx : Warning<
+ "future releases of the clang compiler will prefer GCC installations "
+ "containing libstdc++ include directories; '%0' would be chosen over '%1'">,
+ InGroup<DiagGroup<"gcc-install-dir-libstdcxx">>;
}
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index b8899e78176b4..61c70bd4c0288 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -228,9 +228,6 @@ class ToolChain {
static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
- static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- const Twine &Path);
static void addExternCSystemInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
@@ -244,12 +241,13 @@ class ToolChain {
static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
ArrayRef<StringRef> Paths);
-
static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
const Twine &C = "", const Twine &D = "");
///@}
-
public:
+ static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ const Twine &Path);
virtual ~ToolChain();
// Accessors
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..c8b6283e4dad0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1378,13 +1378,6 @@ void ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
CC1Args.push_back(DriverArgs.MakeArgString(Path));
}
-/// Utility function to add a system include directory to CC1 arguments.
-void ToolChain::addSystemInclude(const ArgList &DriverArgs,
- ArgStringList &CC1Args, const Twine &Path) {
- CC1Args.push_back("-internal-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
-}
-
/// Utility function to add a system include directory with extern "C"
/// semantics to CC1 arguments.
///
@@ -1407,6 +1400,14 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, Path);
}
+/// Utility function to add a system include directory to CC1 arguments.
+/*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
+ ArgStringList &CC1Args,
+ const Twine &Path) {
+ CC1Args.push_back("-internal-isystem");
+ CC1Args.push_back(DriverArgs.MakeArgString(Path));
+}
+
/// Utility function to add a list of system framework directories to CC1.
void ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index 731076d9754a9..5b275c1f8be54 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -433,6 +433,7 @@ const StringRef PossibleAVRLibcLocations[] = {
AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
: Generic_ELF(D, Triple, Args) {
+
GCCInstallation.init(Triple, Args);
if (getCPUName(D, Args, Triple).empty())
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f5e2655857432..21686cc01d180 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2135,10 +2135,11 @@ void Generic_GCC::GCCInstallationDetector::init(
StringRef TripleText =
llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
- Version = GCCVersion::Parse(VersionText);
- GCCTriple.setTriple(TripleText);
- GCCInstallPath = std::string(InstallDir);
- GCCParentLibPath = GCCInstallPath + "/../../..";
+ SelectedInstallation.Version = GCCVersion::Parse(VersionText);
+ SelectedInstallation.GCCTriple.setTriple(TripleText);
+ SelectedInstallation.GCCInstallPath = std::string(InstallDir);
+ SelectedInstallation.GCCParentLibPath =
+ SelectedInstallation.GCCInstallPath + "/../../..";
IsValid = true;
}
return;
@@ -2198,7 +2199,7 @@ void Generic_GCC::GCCInstallationDetector::init(
// Loop over the various components which exist and select the best GCC
// installation available. GCC installs are ranked by version number.
const GCCVersion VersionZero = GCCVersion::Parse("0.0.0");
- Version = VersionZero;
+ SelectedInstallation.Version = VersionZero;
for (const std::string &Prefix : Prefixes) {
auto &VFS = D.getVFS();
if (!VFS.exists(Prefix))
@@ -2226,7 +2227,7 @@ void Generic_GCC::GCCInstallationDetector::init(
}
// Skip other prefixes once a GCC installation is found.
- if (Version > VersionZero)
+ if (SelectedInstallation.Version > VersionZero)
break;
}
}
@@ -2235,14 +2236,17 @@ void Generic_GCC::GCCInstallationDetector::print(raw_ostream &OS) const {
for (const auto &InstallPath : CandidateGCCInstallPaths)
OS << "Found candidate GCC installation: " << InstallPath << "\n";
- if (!GCCInstallPath.empty())
- OS << "Selected GCC installation: " << GCCInstallPath << "\n";
+ if (!SelectedInstallation.GCCInstallPath.empty())
+ OS << "Selected GCC installation: " << SelectedInstallation.GCCInstallPath
+ << "\n";
for (const auto &Multilib : Multilibs)
OS << "Candidate multilib: " << Multilib << "\n";
- if (Multilibs.size() != 0 || !SelectedMultilib.isDefault())
- OS << "Selected multilib: " << SelectedMultilib << "\n";
+ if (Multilibs.size() != 0 ||
+ !SelectedInstallation.SelectedMultilib.isDefault())
+ OS << "Selected multilib: " << SelectedInstallation.SelectedMultilib
+ << "\n";
}
bool Generic_GCC::GCCInstallationDetector::getBiarchSibling(Multilib &M) const {
@@ -2780,14 +2784,50 @@ bool Generic_GCC::GCCInstallationDetector::ScanGCCForMultilibs(
}
Multilibs = Detected.Multilibs;
- SelectedMultilib = Detected.SelectedMultilibs.empty()
- ? Multilib()
- : Detected.SelectedMultilibs.back();
+ SelectedInstallation.SelectedMultilib =
+ Detected.SelectedMultilibs.empty() ? Multilib()
+ : Detected.SelectedMultilibs.back();
BiarchSibling = Detected.BiarchSibling;
return true;
}
+bool Generic_GCC::GCCInstallationDetector::SelectGCCInstallationDirectory(
+ const SmallVector<Generic_GCC::GCCInstallCandidate, 3> &Installations,
+ const ArgList &Args,
+ Generic_GCC::GCCInstallCandidate &SelectedInstallation) const {
+ if (Installations.empty())
+ return false;
+
+ SelectedInstallation =
+ *max_element(Installations, [](const auto &Max, const auto &I) {
+ return I.Version > Max.Version;
+ });
+
+ // FIXME Start selecting installation with libstdc++ in clang 22,
+ // using the current way of selecting the installation as a fallback
+ // only. For now, warn if the installation with libstdc++ differs
+ // from SelectedInstallation.
+ const GCCInstallCandidate *InstallWithIncludes = nullptr;
+ for (const auto &I : Installations) {
+ if ((!InstallWithIncludes || I.Version > InstallWithIncludes->Version) &&
+ GCCInstallationHasLibStdcxxIncludePaths(I, Args))
+ InstallWithIncludes = &I;
+ }
+
+ if (InstallWithIncludes && SelectedInstallation.GCCInstallPath !=
+ InstallWithIncludes->GCCInstallPath)
+ D.Diag(diag::warn_drv_gcc_install_dir_libstdcxx)
+ << InstallWithIncludes->GCCInstallPath
+ << SelectedInstallation.GCCInstallPath;
+
+ // TODO Warn if SelectedInstallation does not contain libstdc++ includes
+ // although compiler flags indicate that it is required (C++ compilation,
+ // libstdc++ not explicitly disabled).
+
+ return true;
+}
+
void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
const llvm::Triple &TargetTriple, const ArgList &Args,
const std::string &LibDir, StringRef CandidateTriple,
@@ -2817,6 +2857,7 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
TargetTriple.getVendor() == llvm::Triple::Freescale ||
TargetTriple.getVendor() == llvm::Triple::OpenEmbedded}};
+ SmallVector<GCCInstallCandidate, 3> Installations;
for (auto &Suffix : Suffixes) {
if (!Suffix.Active)
continue;
@@ -2834,23 +2875,31 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
continue; // Saw this path before; no need to look at it again.
if (CandidateVersion.isOlderThan(4, 1, 1))
continue;
- if (CandidateVersion <= Version)
+ if (CandidateVersion <= SelectedInstallation.Version && IsValid)
continue;
if (!ScanGCCForMultilibs(TargetTriple, Args, LI->path(),
NeedsBiarchSuffix))
continue;
- Version = CandidateVersion;
- GCCTriple.setTriple(CandidateTriple);
+ GCCInstallCandidate Installation;
+ Installation.Version = CandidateVersion;
+ Installation.GCCTriple.setTriple(CandidateTriple);
// FIXME: We hack together the directory name here instead of
// using LI to ensure stable path separators across Windows and
// Linux.
- GCCInstallPath = (LibDir + "/" + LibSuffix + "/" + VersionText).str();
- GCCParentLibPath = (GCCInstallPath + "/../" + Suffix.ReversePath).str();
- IsValid = true;
+ Installation.GCCInstallPath =
+ (LibDir + "/" + LibSuffix + "/" + VersionText).str();
+ Installation.GCCParentLibPath =
+ (Installation.GCCInstallPath + "/../" + Suffix.ReversePath).str();
+ Installation.SelectedMultilib = getMultilib();
+
+ Installations.push_back(Installation);
}
}
+
+ IsValid |=
+ SelectGCCInstallationDirectory(Installations, Args, SelectedInstallation);
}
bool Generic_GCC::GCCInstallationDetector::ScanGentooConfigs(
@@ -2928,10 +2977,12 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig(
NeedsBiarchSuffix))
continue;
- Version = GCCVersion::Parse(ActiveVersion.second);
- GCCInstallPath = GentooPath;
- GCCParentLibPath = GentooPath + std::string("/../../..");
- GCCTriple.setTriple(ActiveVersion.first);
+ SelectedInstallation.Version =
+ GCCVersion::Parse(ActiveVersion.second);
+ SelectedInstallation.GCCInstallPath = GentooPath;
+ SelectedInstallation.GCCParentLibPath =
+ GentooPath + std::string("/../../..");
+ SelectedInstallation.GCCTriple.setTriple(ActiveVersion.first);
IsValid = true;
return true;
}
@@ -3134,8 +3185,9 @@ void Generic_GCC::AddMultilibIncludeArgs(const ArgList &DriverArgs,
// gcc TOOL_INCLUDE_DIR.
const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
std::string LibPath(GCCInstallation.getParentLibPath());
- addSystemInclude(DriverArgs, CC1Args,
- Twine(LibPath) + "/../" + GCCTriple.str() + "/include");
+ ToolChain::addSystemInclude(DriverArgs, CC1Args,
+ Twine(LibPath) + "/../" + GCCTriple.str() +
+ "/include");
const auto &Callback = Multilibs.includeDirsCallback();
if (Callback) {
@@ -3222,12 +3274,14 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
return;
}
-bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
- Twine IncludeSuffix,
- const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- bool DetectDebian) const {
- if (!getVFS().exists(IncludeDir))
+static bool addLibStdCXXIncludePaths(llvm::vfs::FileSystem &vfs,
+ Twine IncludeDir, StringRef Triple,
+ Twine IncludeSuffix,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ bool DetectDebian = false) {
+
+ if (!vfs.exists(IncludeDir))
return false;
// Debian native gcc uses g++-multiarch-incdir.diff which uses
@@ -3239,39 +3293,48 @@ bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
std::string Path =
(Include + "/" + Triple + Dir.substr(Include.size()) + IncludeSuffix)
.str();
- if (DetectDebian && !getVFS().exists(Path))
+ if (DetectDebian && !vfs.exists(Path))
return false;
// GPLUSPLUS_INCLUDE_DIR
- addSystemInclude(DriverArgs, CC1Args, IncludeDir);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir);
// GPLUSPLUS_TOOL_INCLUDE_DIR. If Triple is not empty, add a target-dependent
// include directory.
if (DetectDebian)
- addSystemInclude(DriverArgs, CC1Args, Path);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, Path);
else if (!Triple.empty())
- addSystemInclude(DriverArgs, CC1Args,
- IncludeDir + "/" + Triple + IncludeSuffix);
+ ToolChain::addSystemInclude(DriverArgs, CC1Args,
+ IncludeDir + "/" + Triple + IncludeSuffix);
// GPLUSPLUS_BACKWARD_INCLUDE_DIR
- addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
+ ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
return true;
}
-bool Generic_GCC::addGCCLibStdCxxIncludePaths(
- const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
- StringRef DebianMultiarch) const {
- assert(GCCInstallation.isValid());
+bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
+ Twine IncludeSuffix,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ bool DetectDebian) const {
+ return ::addLibStdCXXIncludePaths(getVFS(), IncludeDir, Triple, IncludeSuffix,
+ DriverArgs, CC1Args, DetectDebian);
+}
+
+bool Generic_GCC::GCCInstallCandidate::addGCCLibStdCxxIncludePaths(
+ llvm::vfs::FileSystem &vfs, const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args, StringRef DebianMultiarch) const {
// By default, look for the C++ headers in an include directory adjacent to
// the lib directory of the GCC installation. Note that this is expect to be
// equivalent to '/usr/include/c++/X.Y' in almost all cases.
- StringRef LibDir = GCCInstallation.getParentLibPath();
- StringRef InstallDir = GCCInstallation.getInstallPath();
- StringRef TripleStr = GCCInstallation.getTriple().str();
- const Multilib &Multilib = GCCInstallation.getMultilib();
- const GCCVersion &Version = GCCInstallation.getVersion();
+ StringRef LibDir = getParentLibPath();
+ StringRef InstallDir = getInstallPath();
+ StringRef TripleStr = getTriple().str();
+ const Multilib &Multilib = getMultilib();
+ const GCCVersion &Version = getVersion();
// Try /../$triple/include/c++/$version (gcc --print-multiarch is not empty).
- if (addLibStdCXXIncludePaths(
+ if (::addLibStdCXXIncludePaths(
+ vfs,
LibDir.str() + "/../" + TripleStr + "/include/c++/" + Version.Text,
TripleStr, Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
@@ -3279,22 +3342,24 @@ bool Generic_GCC::addGCCLibStdCxxIncludePaths(
// Try /gcc/$triple/$version/include/c++/ (gcc --print-multiarch is not
// empty). Like above but for GCC built with
// --enable-version-specific-runtime-libs.
- if (addLibStdCXXIncludePaths(LibDir.str() + "/gcc/" + TripleStr + "/" +
- Version.Text + "/include/c++/",
- TripleStr, Multilib.includeSuffix(), DriverArgs,
- CC1Args))
+ if (::addLibStdCXXIncludePaths(vfs,
+ LibDir.str() + "/gcc/" + TripleStr + "/" +
+ Version.Text + "/include/c++/",
+ TripleStr, Multilib.includeSuffix(),
+ DriverArgs, CC1Args))
return true;
// Detect Debian g++-multiarch-incdir.diff.
- if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
- DebianMultiarch, Multilib.includeSuffix(),
- DriverArgs, CC1Args, /*Debian=*/true))
+ if (::addLibStdCXXIncludePaths(
+ vfs, LibDir.str() + "/../include/c++/" + Version.Text,
+ DebianMultiarch, Multilib.includeSuffix(), DriverArgs, CC1Args,
+ /*Debian=*/true))
return true;
// Try /../include/c++/$version (gcc --print-multiarch is empty).
- if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
- TripleStr, Multilib.includeSuffix(), DriverArgs,
- CC1Args))
+ if (::addLibStdCXXIncludePaths(
+ vfs, LibDir.str() + "/../include/c++/" + Version.Text, TripleStr,
+ Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
// Otherwise, fall back on a bunch of options which don't use multiarch
@@ -3309,20 +3374,50 @@ bool Generic_GCC::addGCCLibStdCxxIncludePaths(
};
for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {
- if (addLibStdCXXIncludePaths(IncludePath, TripleStr,
- Multilib.includeSuffix(), DriverArgs, CC1Args))
+ if (::addLibStdCXXIncludePaths(vfs, IncludePath, TripleStr,
+ Multilib.includeSuffix(), DriverArgs,
+ CC1Args))
return true;
}
return false;
}
+bool Generic_GCC::GCCInstallationDetector::
+ GCCInstallationHasLibStdcxxIncludePaths(
+ const GCCInstallCandidate &GCCInstallation,
+ const llvm::opt::ArgList &DriverArgs) const {
+ StringRef DebianMultiarch =
+ TripleToDebianMultiarch(GCCInstallation.getTriple());
+
+ // The following function checks for libstdc++ include paths and
+ // adds them to the provided argument list. Here we just need the
+ // check.
+ llvm::opt::ArgStringList dummyCC1Args;
+ return GCCInstallation.addGCCLibStdCxxIncludePaths(
+ D.getVFS(), DriverArgs, dummyCC1Args, DebianMultiarch);
+}
+
+bool Generic_GCC::addGCCLibStdCxxIncludePaths(
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const {
+ assert(GCCInstallation.isValid());
+
+ // Detect Debian g++-multiarch-incdir.diff.
+ StringRef DebianMultiarch =
+ GCCInstallation.TripleToDebianMultiarch(GCCInstallation.getTriple());
+
+ return GCCInstallation.getSelectedInstallation().addGCCLibStdCxxIncludePaths(
+ getVFS(), DriverArgs, CC1Args, DebianMultiarch);
+}
+
void
Generic_GCC::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
- if (GCCInstallati...
[truncated]
|
Contributor
Author
|
Ping. |
Collaborator
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
The changes should come with a release note in clang/docs/ReleaseNotes.rst, probably in the potentially breaking changes section.
| InGroup<SourceUsesOpenACC>; | ||
|
|
||
| def warn_drv_gcc_install_dir_libstdcxx : Warning< | ||
| "future releases of the clang compiler will prefer GCC installations " |
Collaborator
There was a problem hiding this comment.
Suggested change
| "future releases of the clang compiler will prefer GCC installations " | |
| "future releases of Clang will prefer GCC installations " |
Member
|
This looks quite good. I'm curious if others have any feedback. |
Collaborator
No additional feedback from me, but this is far enough outside of my wheelhouse that I don't feel my LG should be enough to land it on. |
Contributor
Author
Can anyone else help with a review (@jyknight, @tahonermann, ...)? |
Contributor
Author
|
Ping. |
AaronBallman
approved these changes
Aug 12, 2025
Collaborator
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
Signalling my support for the changes. It looks like @MaskRay is also happy with them. So I'd say let's give folks one more week for additional before landing the changes.
Contributor
Author
Thanks @AaronBallman! I am going to merge the PR tomorrow. |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/33199 Here is the relevant piece of the build log for the reference |
frederik-h
added a commit
that referenced
this pull request
Aug 19, 2025
Member
Without an explicit |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 19, 2025
…ection" (#154368) Reverts llvm/llvm-project#145056
frederik-h
added a commit
to frederik-h/llvm-project
that referenced
this pull request
Aug 20, 2025
The Generic_GCC::GCCInstallationDetector class picks the GCC installation directory with the largest version number. Since the location of the libstdc++ include directories is tied to the GCC version, this can break C++ compilation if the libstdc++ headers for this particular GCC version are not available. Linux distributions tend to package the libstdc++ headers separately from GCC. This frequently leads to situations in which a newer version of GCC gets installed as a dependency of another package without installing the corresponding libstdc++ package. Clang then fails to compile C++ code because it cannot find the libstdc++ headers. Since libstdc++ headers are in fact installed on the system, the GCC installation continues to work, the user may not be aware of the details of the GCC detection, and the compiler does not recognize the situation and emit a warning, this behavior can be hard to understand - as witnessed by many related bug reports over the years. The goal of this work is to change the GCC detection to prefer GCC installations that contain libstdc++ include directories over those which do not. This should happen regardless of the input language since picking different GCC installations for a build that mixes C and C++ might lead to incompatibilities. Any change to the GCC installation detection will probably have a negative impact on some users. For instance, for a C user who relies on using the GCC installation with the largest version number, it might become necessary to use the --gcc-install-dir option to ensure that this GCC version is selected. This seems like an acceptable trade-off given that the situation for users who do not have any special demands on the particular GCC installation directory would be improved significantly. This patch does not yet change the automatic GCC installation directory choice. Instead, it does introduce a warning that informs the user about the future change if the chosen GCC installation directory differs from the one that would be chosen if the libstdc++ headers are taken into account. See also this related Discourse discussion: https://discourse.llvm.org/t/rfc-take-libstdc-into-account-during-gcc-detection/86992.
Contributor
Author
Thank you @MaskRay! I must have copied the |
frederik-h
added a commit
that referenced
this pull request
Aug 22, 2025
…45056 (#154487) The Generic_GCC::GCCInstallationDetector class picks the GCC installation directory with the largest version number. Since the location of the libstdc++ include directories is tied to the GCC version, this can break C++ compilation if the libstdc++ headers for this particular GCC version are not available. Linux distributions tend to package the libstdc++ headers separately from GCC. This frequently leads to situations in which a newer version of GCC gets installed as a dependency of another package without installing the corresponding libstdc++ package. Clang then fails to compile C++ code because it cannot find the libstdc++ headers. Since libstdc++ headers are in fact installed on the system, the GCC installation continues to work, the user may not be aware of the details of the GCC detection, and the compiler does not recognize the situation and emit a warning, this behavior can be hard to understand - as witnessed by many related bug reports over the years. The goal of this work is to change the GCC detection to prefer GCC installations that contain libstdc++ include directories over those which do not. This should happen regardless of the input language since picking different GCC installations for a build that mixes C and C++ might lead to incompatibilities. Any change to the GCC installation detection will probably have a negative impact on some users. For instance, for a C user who relies on using the GCC installation with the largest version number, it might become necessary to use the --gcc-install-dir option to ensure that this GCC version is selected. This seems like an acceptable trade-off given that the situation for users who do not have any special demands on the particular GCC installation directory would be improved significantly. This patch does not yet change the automatic GCC installation directory choice. Instead, it does introduce a warning that informs the user about the future change if the chosen GCC installation directory differs from the one that would be chosen if the libstdc++ headers are taken into account. See also this related Discourse discussion: https://discourse.llvm.org/t/rfc-take-libstdc-into-account-during-gcc-detection/86992. This patch reapplies #145056. The test in the original PR did not specify a target in the clang RUN line and used a wrong way of piping to FileCheck.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Generic_GCC::GCCInstallationDetector class picks the GCC installation directory with the largest version number. Since the location of the libstdc++ include directories is tied to the GCC version, this can break C++ compilation if the libstdc++ headers for this particular GCC version are not available. Linux distributions tend to package the libstdc++ headers separately from GCC. This frequently leads to situations in which a newer version of GCC gets installed as a dependency of another package without installing the corresponding libstdc++ package. Clang then fails to compile C++ code because it cannot find the libstdc++ headers. Since libstdc++ headers are in fact installed on the system, the GCC installation continues to work, the user may not be aware of the details of the GCC detection, and the compiler does not recognize the situation and emit a warning, this behavior can be hard to understand - as witnessed by many related bug reports over the years.
The goal of this work is to change the GCC detection to prefer GCC installations that contain libstdc++ include directories over those which do not. This should happen regardless of the input language since picking different GCC installations for a build that mixes C and C++ might lead to incompatibilities.
Any change to the GCC installation detection will probably have a negative impact on some users. For instance, for a C user who relies on using the GCC installation with the largest version number, it might become necessary to use the --gcc-install-dir option to ensure that this GCC version is selected.
This seems like an acceptable trade-off given that the situation for users who do not have any special demands on the particular GCC installation directory would be improved significantly.
This patch does not yet change the automatic GCC installation directory choice. Instead, it does introduce a warning that informs the user about the future change if the chosen GCC installation directory differs from the one that would be chosen if the libstdc++ headers are taken into account.
See also this related Discourse discussion: https://discourse.llvm.org/t/rfc-take-libstdc-into-account-during-gcc-detection/86992.