[Clang][Solaris] Support --ld-path in Solaris driver #163000
Open
+141
−41
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
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Daniel Levin (daniel-levin) ChangesOverviewThis change fixes and refactors the code that decides which linker to use on Solaris (and derivatives like Illumos). It turns out that the TestingIn addition to running the Full diff: https://github.com/llvm/llvm-project/pull/163000.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 1425714d34110..753742067fe8c 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -491,6 +491,8 @@ class ToolChain {
}
/// GetDefaultLinker - Get the default linker to use.
+ /// Note: this is distinct from the 'preferred' linker, which is optionally
+ /// set at compile time using CLANG_DEFAULT_LINKER.
virtual const char *getDefaultLinker() const { return "ld"; }
/// GetDefaultRuntimeLibType - Get the default runtime library variant to use.
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 16cc1db0a2235..87a6f9a89b904 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1480,11 +1480,11 @@ static void addSanitizerRuntime(const ToolChain &TC, const ArgList &Args,
static bool addSanitizerDynamicList(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs,
StringRef Sanitizer) {
- bool LinkerIsGnuLd = solaris::isLinkerGnuLd(TC, Args);
+ bool LinkerIsSolarisLinkEditor = solaris::isLinkerSolarisLinkEditor(TC, Args);
// Solaris ld defaults to --export-dynamic behaviour but doesn't support
// the option, so don't try to pass it.
- if (TC.getTriple().isOSSolaris() && !LinkerIsGnuLd)
+ if (TC.getTriple().isOSSolaris() && LinkerIsSolarisLinkEditor)
return true;
SmallString<128> SanRT(TC.getCompilerRT(Args, Sanitizer));
if (llvm::sys::fs::exists(SanRT + ".syms")) {
@@ -1500,14 +1500,14 @@ void tools::addAsNeededOption(const ToolChain &TC,
bool as_needed) {
assert(!TC.getTriple().isOSAIX() &&
"AIX linker does not support any form of --as-needed option yet.");
- bool LinkerIsGnuLd = solaris::isLinkerGnuLd(TC, Args);
+ bool LinkerIsSolarisLinkEditor = solaris::isLinkerSolarisLinkEditor(TC, Args);
// While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
// for the native forms -z ignore/-z record, they are missing in Illumos,
// so always use the native form.
// GNU ld doesn't support -z ignore/-z record, so don't use them even on
// Solaris.
- if (TC.getTriple().isOSSolaris() && !LinkerIsGnuLd) {
+ if (TC.getTriple().isOSSolaris() && LinkerIsSolarisLinkEditor) {
CmdArgs.push_back("-z");
CmdArgs.push_back(as_needed ? "ignore" : "record");
} else {
diff --git a/clang/lib/Driver/ToolChains/Solaris.cpp b/clang/lib/Driver/ToolChains/Solaris.cpp
index 02aa59817449d..edd794d19cbe5 100644
--- a/clang/lib/Driver/ToolChains/Solaris.cpp
+++ b/clang/lib/Driver/ToolChains/Solaris.cpp
@@ -36,11 +36,12 @@ void solaris::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
gnutools::Assembler::ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
}
-bool solaris::isLinkerGnuLd(const ToolChain &TC, const ArgList &Args) {
- // Only used if targetting Solaris.
- const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
- StringRef UseLinker = A ? A->getValue() : TC.getDriver().getPreferredLinker();
- return UseLinker == "bfd" || UseLinker == "gld";
+bool solaris::isLinkerSolarisLinkEditor(const ToolChain &TC,
+ const ArgList &Args) {
+ auto Determination =
+ solaris::LinkerDetermination::make(TC, Args, /* EmitDiagnostics */ false);
+ return Determination.Expectations ==
+ solaris::LinkerExpectations::SolarisLinkEditor;
}
static bool getPIE(const ArgList &Args, const ToolChain &TC) {
@@ -52,30 +53,11 @@ static bool getPIE(const ArgList &Args, const ToolChain &TC) {
TC.isPIEDefault(Args));
}
-// FIXME: Need to handle PreferredLinker here?
std::string solaris::Linker::getLinkerPath(const ArgList &Args) const {
- const ToolChain &ToolChain = getToolChain();
- if (const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
- StringRef UseLinker = A->getValue();
- if (!UseLinker.empty()) {
- if (llvm::sys::path::is_absolute(UseLinker) &&
- llvm::sys::fs::can_execute(UseLinker))
- return std::string(UseLinker);
-
- // Accept 'bfd' and 'gld' as aliases for the GNU linker.
- if (UseLinker == "bfd" || UseLinker == "gld")
- // FIXME: Could also use /usr/bin/gld here.
- return "/usr/gnu/bin/ld";
+ auto Determination =
+ solaris::LinkerDetermination::make(getToolChain(), Args, true);
- // Accept 'ld' as alias for the default linker
- if (UseLinker != "ld")
- ToolChain.getDriver().Diag(diag::err_drv_invalid_linker_name)
- << A->getAsString(Args);
- }
- }
-
- // getDefaultLinker() always returns an absolute path.
- return ToolChain.getDefaultLinker();
+ return Determination.Linker;
}
void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
@@ -87,11 +69,11 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const Driver &D = ToolChain.getDriver();
const llvm::Triple::ArchType Arch = ToolChain.getArch();
const bool IsPIE = getPIE(Args, ToolChain);
- const bool LinkerIsGnuLd = isLinkerGnuLd(ToolChain, Args);
+ const bool LinkerIsSolarisLd = isLinkerSolarisLinkEditor(ToolChain, Args);
ArgStringList CmdArgs;
// Demangle C++ names in errors. GNU ld already defaults to --demangle.
- if (!LinkerIsGnuLd)
+ if (LinkerIsSolarisLd)
CmdArgs.push_back("-C");
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_shared,
@@ -101,7 +83,7 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
if (IsPIE) {
- if (LinkerIsGnuLd) {
+ if (!LinkerIsSolarisLd) {
CmdArgs.push_back("-pie");
} else {
CmdArgs.push_back("-z");
@@ -122,7 +104,7 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
Args.ClaimAllArgs(options::OPT_pthreads);
}
- if (LinkerIsGnuLd) {
+ if (!LinkerIsSolarisLd) {
// Set the correct linker emulation for 32- and 64-bit Solaris.
switch (Arch) {
case llvm::Triple::x86:
@@ -256,7 +238,7 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
if (Arch == llvm::Triple::x86_64 &&
(SA.needsAsanRt() || SA.needsStatsRt() ||
(SA.needsUbsanRt() && !SA.requiresMinimalRuntime())) &&
- !LinkerIsGnuLd) {
+ LinkerIsSolarisLd) {
CmdArgs.push_back("-z");
CmdArgs.push_back("relax=transtls");
}
@@ -344,10 +326,10 @@ SanitizerMask Solaris::getSupportedSanitizers() const {
}
const char *Solaris::getDefaultLinker() const {
- // FIXME: Only handle Solaris ld and GNU ld here.
- return llvm::StringSwitch<const char *>(getDriver().getPreferredLinker())
- .Cases("bfd", "gld", "/usr/gnu/bin/ld")
- .Default("/usr/bin/ld");
+ // The default linker on Solaris is _always_ the Solaris Link Editor.
+ // Recall that the driver's _default_ linker is distinct from the compile-time
+ // _preferred_ linker setting CLANG_DEFAULT_LINKER, which may even be empty.
+ return "/usr/bin/ld";
}
Tool *Solaris::buildAssembler() const {
@@ -423,3 +405,68 @@ void Solaris::addLibStdCxxIncludePaths(
TripleStr, Multilib.includeSuffix(), DriverArgs,
CC1Args);
}
+
+solaris::LinkerDetermination
+solaris::LinkerDetermination::make(const ToolChain &TC, const ArgList &Args,
+ bool EmitDiagnostics) {
+ // First, check --ld-path, then -fuse-ld, then the compile-time
+ // preferred linker (CLANG_DEFAULT_LINKER), then finally fall back to the
+ // platform's default - the Solaris Link Editor. This behavior is consonant
+ // with the other platforms' drivers.
+
+ auto InferExpectations =
+ [](const std::string &s) -> solaris::LinkerExpectations {
+ if (s == "ld" || s == "/bin/ld" || s == "/usr/bin/ld")
+ return solaris::LinkerExpectations::SolarisLinkEditor;
+ return solaris::LinkerExpectations::GnuLdCompatibleArgParser;
+ };
+
+ if (const Arg *A = Args.getLastArg(options::OPT_ld_path_EQ)) {
+ StringRef UseLinker = A->getValue();
+ if (!UseLinker.empty()) {
+ auto LinkerPath = std::string(UseLinker);
+ if (llvm::sys::fs::can_execute(LinkerPath))
+ return solaris::LinkerDetermination(LinkerPath,
+ InferExpectations(LinkerPath));
+ }
+ if (EmitDiagnostics)
+ TC.getDriver().Diag(diag::err_drv_invalid_linker_name)
+ << A->getAsString(Args);
+ } else if (const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
+ StringRef UseLinker = A->getValue();
+ if (!UseLinker.empty()) {
+ if (llvm::sys::path::is_absolute(UseLinker) &&
+ llvm::sys::fs::can_execute(UseLinker)) {
+ auto LinkerPath = std::string(UseLinker);
+ return solaris::LinkerDetermination(LinkerPath,
+ InferExpectations(LinkerPath));
+ }
+
+ // Accept 'bfd' and 'gld' as aliases for the GNU linker.
+ if (UseLinker == "bfd" || UseLinker == "gld")
+ return solaris::LinkerDetermination(
+ "/usr/gnu/bin/ld",
+ solaris::LinkerExpectations::GnuLdCompatibleArgParser);
+
+ // Accept 'ld' as an alias for the default linker
+ if (UseLinker == "ld")
+ return solaris::LinkerDetermination(
+ "/usr/bin/ld", solaris::LinkerExpectations::SolarisLinkEditor);
+
+ if (EmitDiagnostics)
+ TC.getDriver().Diag(diag::err_drv_invalid_linker_name)
+ << A->getAsString(Args);
+ }
+ }
+
+ auto CompileTimePreferredLinker = TC.getDriver().getPreferredLinker();
+ if (!CompileTimePreferredLinker.empty()) {
+ auto LinkerPath = std::string(CompileTimePreferredLinker);
+ return solaris::LinkerDetermination(LinkerPath,
+ InferExpectations(LinkerPath));
+ }
+
+ return solaris::LinkerDetermination(
+ std::string("/usr/bin/ld"),
+ solaris::LinkerExpectations::SolarisLinkEditor);
+}
diff --git a/clang/lib/Driver/ToolChains/Solaris.h b/clang/lib/Driver/ToolChains/Solaris.h
index 9ec83b773da44..0d3a78ff09189 100644
--- a/clang/lib/Driver/ToolChains/Solaris.h
+++ b/clang/lib/Driver/ToolChains/Solaris.h
@@ -31,7 +31,8 @@ class LLVM_LIBRARY_VISIBILITY Assembler final : public gnutools::Assembler {
const char *LinkingOutput) const override;
};
-bool isLinkerGnuLd(const ToolChain &TC, const llvm::opt::ArgList &Args);
+bool isLinkerSolarisLinkEditor(const ToolChain &TC,
+ const llvm::opt::ArgList &Args);
class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
public:
@@ -46,6 +47,34 @@ class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};
+
+enum class LinkerExpectations {
+ GnuLdCompatibleArgParser,
+ /// The formal name for the built-in ld on Solaris.
+ SolarisLinkEditor,
+};
+
+/// We use Solaris's built-in linker by default. It has a unique command line
+/// syntax and specific limitations. By contrast, other linkers such as lld,
+/// Mold, and Wild are compatible with GNU ld's command line syntax. Knowing
+/// _which_ linker to use is sufficient to determine the expectations of that
+/// linker. Rather than spread ad-hoc string comparisons all over the driver, we
+/// encapsulate the details of differences in the chosen linker here.
+class LinkerDetermination final {
+ LinkerDetermination(std::string Linker, LinkerExpectations Expectations)
+ : Linker(Linker), Expectations(Expectations) {}
+
+public:
+ std::string Linker;
+ LinkerExpectations Expectations;
+
+ /// Choose the correct linker based on arguments and compile-time options
+ /// recorded in the ToolChain.
+ static LinkerDetermination make(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ bool EmitDiagnostics);
+};
+
} // end namespace solaris
} // end namespace tools
|
|
Friendly ping @MaskRay. I would appreciate it very much if you could take a look. |
MaskRay
reviewed
Oct 22, 2025
| const char *LinkingOutput) const override; | ||
| }; | ||
|
|
||
| enum class LinkerExpectations { |
There was a problem hiding this comment.
This enum class looks over engineered. We probably just need boolean variables IsSolarisLd in call sites
There was a problem hiding this comment.
It is over-engineered in its final form here. There were a few other fields that I wound up not needing. Good call out.
I won't be able to look into this for a while: I'm off to a two-week vacation soon. However, first things first: which of the non-GNU ld linkers you talk about have actually been ported to Solaris? I know of none (but haven't looked closely). AFAIK Besides, talking of Illumos there's still Issue #53919 which hasn't been addressed by the Illumos community in 3 1/2 years. If they are finally starting to contribute, it would be good if this could be taken care of. Not a requirement, but worth considering. |
I'm happy to say that Wild works on Solaris/Illumos - but you have to symlink it to GNU ld which is terrible (because the Clang driver assumes there are only two linkers on Solaris/Illumos). With this driver change, I've managed to link Clang itself and a host of Rust programs including RipGrep. The system linker on Solaris/Illumos is dreadfully slow. This change will make contributing to LLVM on Solaris/Illumos much easier, as it'll be possible to build without using the leaden system linker. |
Added five cases. |
daniel-levin
force-pushed
the
solarisfix
branch
from
8677479 to
dd443a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Overview
This change fixes and refactors the code that decides which linker to use on Solaris (and derivatives like Illumos). It turns out that the
--ld-pathoption has never worked on Solaris. This commit also fixes threeFIXMEs along the way.The primary logical change in this commit is the inversion of checking if we're using GNU ld to checking if we're using Solaris's build-in linker.
Benefits
Users can now use third-party GNU-ld compatible linkers such as Wild.
Testing
In addition to running the
llvm-littests affected by this change (.e.g/test/Driver/solaris-ld), I manually tested every branch that makes a determination of which linker to use.