-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[Clang][Driver] Revise Cygwin ToolChain to call linker directly #147960
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
base: main
Are you sure you want to change the base?
Conversation
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. |
@llvm/pr-subscribers-clang Author: None (tyan0) Changes...so that Full diff: https://github.com/llvm/llvm-project/pull/147960.diff 9 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..3c9148ee51fd0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -684,6 +684,8 @@ static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
StringRef ToolChain::getOSLibName() const {
if (Triple.isOSDarwin())
return "darwin";
+ if (Triple.isWindowsCygwinEnvironment())
+ return "cygwin";
switch (Triple.getOS()) {
case llvm::Triple::FreeBSD:
@@ -1504,6 +1506,7 @@ void ToolChain::AddCXXStdlibLibArgs(const ArgList &Args,
switch (Type) {
case ToolChain::CST_Libcxx:
CmdArgs.push_back("-lc++");
+ CmdArgs.push_back("-lc++abi");
if (Args.hasArg(options::OPT_fexperimental_library))
CmdArgs.push_back("-lc++experimental");
break;
diff --git a/clang/lib/Driver/ToolChains/Cygwin.cpp b/clang/lib/Driver/ToolChains/Cygwin.cpp
index d9c16347daa34..82a88065b0c9d 100644
--- a/clang/lib/Driver/ToolChains/Cygwin.cpp
+++ b/clang/lib/Driver/ToolChains/Cygwin.cpp
@@ -9,6 +9,7 @@
#include "Cygwin.h"
#include "clang/Config/config.h"
#include "clang/Driver/CommonArgs.h"
+#include "clang/Driver/Compilation.h"
#include "clang/Driver/Driver.h"
#include "clang/Driver/Options.h"
#include "llvm/Support/Path.h"
@@ -30,6 +31,8 @@ Cygwin::Cygwin(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
Generic_GCC::PushPPaths(PPaths);
path_list &Paths = getFilePaths();
+ if (GCCInstallation.isValid())
+ Paths.push_back(GCCInstallation.getInstallPath().str());
Generic_GCC::AddMultiarchPaths(D, SysRoot, "lib", Paths);
@@ -107,3 +110,289 @@ void Cygwin::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include/w32api");
}
+
+static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
+ bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+ if (HasStaticPIE && Args.hasArg(options::OPT_no_pie)) {
+ const Driver &D = TC.getDriver();
+ const llvm::opt::OptTable &Opts = D.getOpts();
+ StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+ StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
+ D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+ }
+ return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList &Args) {
+ return Args.hasArg(options::OPT_static) &&
+ !Args.hasArg(options::OPT_static_pie);
+}
+
+void cygwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
+ const auto &ToolChain = static_cast<const Cygwin &>(getToolChain());
+ const Driver &D = ToolChain.getDriver();
+
+ const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
+ const bool IsVE = ToolChain.getTriple().isVE();
+ const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+ const bool IsStatic = getStatic(Args);
+
+ ArgStringList CmdArgs;
+
+ // Silence warning for "clang -g foo.o -o foo"
+ Args.ClaimAllArgs(options::OPT_g_Group);
+ // and "clang -emit-llvm foo.o -o foo"
+ Args.ClaimAllArgs(options::OPT_emit_llvm);
+ // and for "clang -w foo.o -o foo". Other warning options are already
+ // handled somewhere else.
+ Args.ClaimAllArgs(options::OPT_w);
+
+ if (!D.SysRoot.empty())
+ CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
+
+ if (Args.hasArg(options::OPT_s))
+ CmdArgs.push_back("-s");
+
+ CmdArgs.push_back("-m");
+ switch (ToolChain.getArch()) {
+ case llvm::Triple::x86:
+ CmdArgs.push_back("i386pe");
+ break;
+ case llvm::Triple::x86_64:
+ CmdArgs.push_back("i386pep");
+ break;
+ case llvm::Triple::arm:
+ case llvm::Triple::thumb:
+ // FIXME: this is incorrect for WinCE
+ CmdArgs.push_back("thumb2pe");
+ break;
+ case llvm::Triple::aarch64:
+ if (ToolChain.getEffectiveTriple().isWindowsArm64EC())
+ CmdArgs.push_back("arm64ecpe");
+ else
+ CmdArgs.push_back("arm64pe");
+ break;
+ default:
+ D.Diag(diag::err_target_unknown_triple) << ToolChain.getEffectiveTriple().str();
+ }
+
+ CmdArgs.push_back("--wrap=_Znwm");
+ CmdArgs.push_back("--wrap=_Znam");
+ CmdArgs.push_back("--wrap=_ZdlPv");
+ CmdArgs.push_back("--wrap=_ZdaPv");
+ CmdArgs.push_back("--wrap=_ZnwmRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZnamRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZdlPvRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZdaPvRKSt9nothrow_t");
+
+ const bool IsShared = Args.hasArg(options::OPT_shared);
+ if (IsShared)
+ CmdArgs.push_back("-shared");
+ bool IsPIE = false;
+ if (IsStaticPIE) {
+ CmdArgs.push_back("-static");
+ CmdArgs.push_back("-pie");
+ CmdArgs.push_back("--no-dynamic-linker");
+ CmdArgs.push_back("-z");
+ CmdArgs.push_back("text");
+ } else if (IsStatic) {
+ CmdArgs.push_back("-static");
+ } else if (!Args.hasArg(options::OPT_r)) {
+ if (Args.hasArg(options::OPT_rdynamic))
+ CmdArgs.push_back("-export-dynamic");
+ if (!IsShared) {
+ IsPIE = Args.hasFlag(options::OPT_pie, options::OPT_no_pie,
+ ToolChain.isPIEDefault(Args));
+ if (IsPIE)
+ CmdArgs.push_back("-pie");
+ }
+ }
+
+ CmdArgs.push_back("-o");
+ CmdArgs.push_back(Output.getFilename());
+
+ if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+ options::OPT_r)) {
+ if (IsVE) {
+ CmdArgs.push_back("-z");
+ CmdArgs.push_back("max-page-size=0x4000000");
+ }
+
+ if (IsShared) {
+ CmdArgs.push_back("-e");
+ CmdArgs.push_back("_cygwin_dll_entry");
+ CmdArgs.push_back("--enable-auto-image-base");
+ }
+
+ if (!IsShared)
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+ if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT) {
+ std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+ ToolChain::FT_Object);
+ if (ToolChain.getVFS().exists(crtbegin)) {
+ std::string P;
+ P = crtbegin;
+ CmdArgs.push_back(Args.MakeArgString(P));
+ }
+ }
+ if (IsShared)
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbeginS.o")));
+ else
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+
+ // Add crtfastmath.o if available and fast math is enabled.
+ ToolChain.addFastMathRuntimeIfAvailable(Args, CmdArgs);
+ }
+
+ Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_u});
+
+ ToolChain.AddFilePathLibArgs(Args, CmdArgs);
+
+ CmdArgs.push_back(Args.MakeArgString(
+ "-L" + ToolChain.getGCCInstallation().getInstallPath()));
+
+ if (D.isUsingLTO()) {
+ assert(!Inputs.empty() && "Must have at least one input.");
+ // Find the first filename InputInfo object.
+ auto Input = llvm::find_if(
+ Inputs, [](const InputInfo &II) -> bool { return II.isFilename(); });
+ if (Input == Inputs.end())
+ // For a very rare case, all of the inputs to the linker are
+ // InputArg. If that happens, just use the first InputInfo.
+ Input = Inputs.begin();
+
+ tools::addLTOOptions(ToolChain, Args, CmdArgs, Output, *Input,
+ D.getLTOMode() == LTOK_Thin);
+ }
+
+ if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+ CmdArgs.push_back("--no-demangle");
+
+ bool NeedsSanitizerDeps = tools::addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+ bool NeedsXRayDeps = tools::addXRayRuntime(ToolChain, Args, CmdArgs);
+ tools::addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
+ tools::AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
+ tools::addHIPRuntimeLibArgs(ToolChain, C, Args, CmdArgs);
+
+ // The profile runtime also needs access to system libraries.
+ getToolChain().addProfileRTLibs(Args, CmdArgs);
+
+ if (D.CCCIsCXX() &&
+ !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+ options::OPT_r)) {
+ if (ToolChain.ShouldLinkCXXStdlib(Args)) {
+ bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
+ !Args.hasArg(options::OPT_static);
+ if (OnlyLibstdcxxStatic)
+ CmdArgs.push_back("-Bstatic");
+ ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+ if (OnlyLibstdcxxStatic)
+ CmdArgs.push_back("-Bdynamic");
+ }
+ CmdArgs.push_back("-lm");
+ }
+
+ // Silence warnings when linking C code with a C++ '-stdlib' argument.
+ Args.ClaimAllArgs(options::OPT_stdlib_EQ);
+
+ // Additional linker set-up and flags for Fortran. This is required in order
+ // to generate executables. As Fortran runtime depends on the C runtime,
+ // these dependencies need to be listed before the C runtime below (i.e.
+ // AddRunTimeLibs).
+ if (D.IsFlangMode() &&
+ !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+ tools::addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
+ tools::addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
+ CmdArgs.push_back("-lm");
+ }
+
+ if (!Args.hasArg(options::OPT_nostdlib, options::OPT_r)) {
+ if (!Args.hasArg(options::OPT_nodefaultlibs)) {
+ if (IsStatic || IsStaticPIE)
+ CmdArgs.push_back("--start-group");
+
+ if (NeedsSanitizerDeps)
+ tools::linkSanitizerRuntimeDeps(ToolChain, Args, CmdArgs);
+
+ if (NeedsXRayDeps)
+ tools::linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+
+ bool WantPthread = Args.hasArg(options::OPT_pthread) ||
+ Args.hasArg(options::OPT_pthreads);
+
+ // Use the static OpenMP runtime with -static-openmp
+ bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
+ !Args.hasArg(options::OPT_static);
+
+ // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
+ // require librt. Most modern Linux platforms do, but some may not.
+ if (tools::addOpenMPRuntime(C, CmdArgs, ToolChain, Args, StaticOpenMP,
+ JA.isHostOffloading(Action::OFK_OpenMP),
+ /* GompNeedsRT= */ true))
+ // OpenMP runtimes implies pthreads when using the GNU toolchain.
+ // FIXME: Does this really make sense for all GNU toolchains?
+ WantPthread = true;
+
+ tools::AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
+
+ if (WantPthread)
+ CmdArgs.push_back("-lpthread");
+
+ if (Args.hasArg(options::OPT_fsplit_stack))
+ CmdArgs.push_back("--wrap=pthread_create");
+
+ if (!Args.hasArg(options::OPT_nolibc))
+ CmdArgs.push_back("-lc");
+
+ // Cygwin specific
+ CmdArgs.push_back("-lcygwin");
+ CmdArgs.push_back("-ladvapi32");
+ CmdArgs.push_back("-lshell32");
+ CmdArgs.push_back("-luser32");
+ CmdArgs.push_back("-lkernel32");
+
+ // Add IAMCU specific libs, if needed.
+ if (IsIAMCU)
+ CmdArgs.push_back("-lgloss");
+
+ if (IsStatic || IsStaticPIE)
+ CmdArgs.push_back("--end-group");
+ else
+ tools::AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
+
+ // Add IAMCU specific libs (outside the group), if needed.
+ if (IsIAMCU) {
+ CmdArgs.push_back("--as-needed");
+ CmdArgs.push_back("-lsoftfp");
+ CmdArgs.push_back("--no-as-needed");
+ }
+ }
+
+ if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
+ if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT) {
+ std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+ ToolChain::FT_Object);
+ if (ToolChain.getVFS().exists(crtend)) {
+ std::string P;
+ P = crtend;
+ CmdArgs.push_back(Args.MakeArgString(P));
+ }
+ }
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
+ }
+ }
+
+ Args.addAllArgs(CmdArgs, {options::OPT_T, options::OPT_t});
+
+ const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
+ C.addCommand(std::make_unique<Command>(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Exec, CmdArgs, Inputs, Output));
+}
+
+auto Cygwin::buildLinker() const -> Tool * { return new cygwin::Linker(*this); }
diff --git a/clang/lib/Driver/ToolChains/Cygwin.h b/clang/lib/Driver/ToolChains/Cygwin.h
index d2f72c10c3b01..b76069fba6741 100644
--- a/clang/lib/Driver/ToolChains/Cygwin.h
+++ b/clang/lib/Driver/ToolChains/Cygwin.h
@@ -27,7 +27,28 @@ class LLVM_LIBRARY_VISIBILITY Cygwin : public Generic_GCC {
void
AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
+
+ const GCCInstallationDetector &
+ getGCCInstallation() const { return GCCInstallation; };
+
+protected:
+ Tool *buildLinker() const override;
+};
+
+namespace cygwin {
+class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
+public:
+ Linker(const ToolChain &TC) : Tool("cygwin::Linker", "linker", TC) {}
+
+ bool hasIntegratedCPP() const override { return false; }
+ bool isLinkJob() const override { return true; }
+
+ void ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output, const InputInfoList &Inputs,
+ const llvm::opt::ArgList &TCArgs,
+ const char *LinkingOutput) const override;
};
+} // end namespace cygwin
} // end namespace toolchains
} // end namespace driver
diff --git a/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/i686-pc-msys/10/crtend.o b/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/i686-pc-msys/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/x86_64-pc-cygwin/10/crtend.o b/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/x86_64-pc-cygwin/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/crt0.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/crt0.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/i686-pc-cygwin/10/crtend.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/i686-pc-cygwin/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/x86_64-pc-msys/10/crtend.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/x86_64-pc-msys/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/cygwin.cpp b/clang/test/Driver/cygwin.cpp
index dd75c48ddc6b3..8677e84c033fd 100644
--- a/clang/test/Driver/cygwin.cpp
+++ b/clang/test/Driver/cygwin.cpp
@@ -15,19 +15,19 @@
// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include/w32api"
// CHECK-SAME: "-femulated-tls"
// CHECK-SAME: "-exception-model=dwarf"
-// CHECK: "{{.*}}gcc{{(\.exe)?}}"
-// CHECK-SAME: "-m32"
+// CHECK: "{{.*}}ld{{(\.exe)?}}"
+// CHECK-SAME: "-m" "i386pe"
// RUN: %clang -### %s --target=i686-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: --stdlib=platform -static 2>&1 | FileCheck --check-prefix=CHECK-STATIC %s
// CHECK-STATIC: "-cc1" "-triple" "i686-pc-windows-cygnus"
// CHECK-STATIC-SAME: "-static-define"
-// CHECK-STATIC: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-STATIC: "{{.*}}ld{{(\.exe)?}}"
// CHECK-STATIC-SAME: "-static"
// RUN: %clang -### %s --target=i686-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: -shared 2>&1 | FileCheck --check-prefix=CHECK-SHARED %s
-// CHECK-SHARED: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-SHARED: "{{.*}}ld{{(\.exe)?}}"
// CHECK-SHARED-SAME: "-shared"
// RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \
@@ -54,19 +54,19 @@
// CHECK-64-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include/w32api"
// CHECK-64-SAME: "-femulated-tls"
// CHECK-64-SAME: "-exception-model=seh"
-// CHECK-64: "{{.*}}gcc{{(\.exe)?}}"
-// CHECK-64-SAME: "-m64"
+// CHECK-64: "{{.*}}ld{{(\.exe)?}}"
+// CHECK-64-SAME: "-m" "i386pep"
// RUN: %clang -### %s --target=x86_64-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: --stdlib=platform -static 2>&1 | FileCheck --check-prefix=CHECK-64-STATIC %s
// CHECK-64-STATIC: "-cc1" "-triple" "x86_64-pc-windows-cygnus"
// CHECK-64-STATIC-SAME: "-static-define"
-// CHECK-64-STATIC: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-64-STATIC: "{{.*}}ld{{(\.exe)?}}"
// CHECK-64-STATIC-SAME: "-static"
// RUN: %clang -### %s --target=x86_64-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: -shared 2>&1 | FileCheck --check-prefix=CHECK-64-SHARED %s
-// CHECK-64-SHARED: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-64-SHARED: "{{.*}}ld{{(\.exe)?}}"
// CHECK-64-SHARED-SAME: "-shared"
// RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \
|
@llvm/pr-subscribers-clang-driver Author: None (tyan0) Changes...so that Full diff: https://github.com/llvm/llvm-project/pull/147960.diff 9 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..3c9148ee51fd0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -684,6 +684,8 @@ static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
StringRef ToolChain::getOSLibName() const {
if (Triple.isOSDarwin())
return "darwin";
+ if (Triple.isWindowsCygwinEnvironment())
+ return "cygwin";
switch (Triple.getOS()) {
case llvm::Triple::FreeBSD:
@@ -1504,6 +1506,7 @@ void ToolChain::AddCXXStdlibLibArgs(const ArgList &Args,
switch (Type) {
case ToolChain::CST_Libcxx:
CmdArgs.push_back("-lc++");
+ CmdArgs.push_back("-lc++abi");
if (Args.hasArg(options::OPT_fexperimental_library))
CmdArgs.push_back("-lc++experimental");
break;
diff --git a/clang/lib/Driver/ToolChains/Cygwin.cpp b/clang/lib/Driver/ToolChains/Cygwin.cpp
index d9c16347daa34..82a88065b0c9d 100644
--- a/clang/lib/Driver/ToolChains/Cygwin.cpp
+++ b/clang/lib/Driver/ToolChains/Cygwin.cpp
@@ -9,6 +9,7 @@
#include "Cygwin.h"
#include "clang/Config/config.h"
#include "clang/Driver/CommonArgs.h"
+#include "clang/Driver/Compilation.h"
#include "clang/Driver/Driver.h"
#include "clang/Driver/Options.h"
#include "llvm/Support/Path.h"
@@ -30,6 +31,8 @@ Cygwin::Cygwin(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
Generic_GCC::PushPPaths(PPaths);
path_list &Paths = getFilePaths();
+ if (GCCInstallation.isValid())
+ Paths.push_back(GCCInstallation.getInstallPath().str());
Generic_GCC::AddMultiarchPaths(D, SysRoot, "lib", Paths);
@@ -107,3 +110,289 @@ void Cygwin::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include/w32api");
}
+
+static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
+ bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+ if (HasStaticPIE && Args.hasArg(options::OPT_no_pie)) {
+ const Driver &D = TC.getDriver();
+ const llvm::opt::OptTable &Opts = D.getOpts();
+ StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+ StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
+ D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+ }
+ return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList &Args) {
+ return Args.hasArg(options::OPT_static) &&
+ !Args.hasArg(options::OPT_static_pie);
+}
+
+void cygwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
+ const auto &ToolChain = static_cast<const Cygwin &>(getToolChain());
+ const Driver &D = ToolChain.getDriver();
+
+ const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
+ const bool IsVE = ToolChain.getTriple().isVE();
+ const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+ const bool IsStatic = getStatic(Args);
+
+ ArgStringList CmdArgs;
+
+ // Silence warning for "clang -g foo.o -o foo"
+ Args.ClaimAllArgs(options::OPT_g_Group);
+ // and "clang -emit-llvm foo.o -o foo"
+ Args.ClaimAllArgs(options::OPT_emit_llvm);
+ // and for "clang -w foo.o -o foo". Other warning options are already
+ // handled somewhere else.
+ Args.ClaimAllArgs(options::OPT_w);
+
+ if (!D.SysRoot.empty())
+ CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
+
+ if (Args.hasArg(options::OPT_s))
+ CmdArgs.push_back("-s");
+
+ CmdArgs.push_back("-m");
+ switch (ToolChain.getArch()) {
+ case llvm::Triple::x86:
+ CmdArgs.push_back("i386pe");
+ break;
+ case llvm::Triple::x86_64:
+ CmdArgs.push_back("i386pep");
+ break;
+ case llvm::Triple::arm:
+ case llvm::Triple::thumb:
+ // FIXME: this is incorrect for WinCE
+ CmdArgs.push_back("thumb2pe");
+ break;
+ case llvm::Triple::aarch64:
+ if (ToolChain.getEffectiveTriple().isWindowsArm64EC())
+ CmdArgs.push_back("arm64ecpe");
+ else
+ CmdArgs.push_back("arm64pe");
+ break;
+ default:
+ D.Diag(diag::err_target_unknown_triple) << ToolChain.getEffectiveTriple().str();
+ }
+
+ CmdArgs.push_back("--wrap=_Znwm");
+ CmdArgs.push_back("--wrap=_Znam");
+ CmdArgs.push_back("--wrap=_ZdlPv");
+ CmdArgs.push_back("--wrap=_ZdaPv");
+ CmdArgs.push_back("--wrap=_ZnwmRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZnamRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZdlPvRKSt9nothrow_t");
+ CmdArgs.push_back("--wrap=_ZdaPvRKSt9nothrow_t");
+
+ const bool IsShared = Args.hasArg(options::OPT_shared);
+ if (IsShared)
+ CmdArgs.push_back("-shared");
+ bool IsPIE = false;
+ if (IsStaticPIE) {
+ CmdArgs.push_back("-static");
+ CmdArgs.push_back("-pie");
+ CmdArgs.push_back("--no-dynamic-linker");
+ CmdArgs.push_back("-z");
+ CmdArgs.push_back("text");
+ } else if (IsStatic) {
+ CmdArgs.push_back("-static");
+ } else if (!Args.hasArg(options::OPT_r)) {
+ if (Args.hasArg(options::OPT_rdynamic))
+ CmdArgs.push_back("-export-dynamic");
+ if (!IsShared) {
+ IsPIE = Args.hasFlag(options::OPT_pie, options::OPT_no_pie,
+ ToolChain.isPIEDefault(Args));
+ if (IsPIE)
+ CmdArgs.push_back("-pie");
+ }
+ }
+
+ CmdArgs.push_back("-o");
+ CmdArgs.push_back(Output.getFilename());
+
+ if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+ options::OPT_r)) {
+ if (IsVE) {
+ CmdArgs.push_back("-z");
+ CmdArgs.push_back("max-page-size=0x4000000");
+ }
+
+ if (IsShared) {
+ CmdArgs.push_back("-e");
+ CmdArgs.push_back("_cygwin_dll_entry");
+ CmdArgs.push_back("--enable-auto-image-base");
+ }
+
+ if (!IsShared)
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+ if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT) {
+ std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+ ToolChain::FT_Object);
+ if (ToolChain.getVFS().exists(crtbegin)) {
+ std::string P;
+ P = crtbegin;
+ CmdArgs.push_back(Args.MakeArgString(P));
+ }
+ }
+ if (IsShared)
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbeginS.o")));
+ else
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+
+ // Add crtfastmath.o if available and fast math is enabled.
+ ToolChain.addFastMathRuntimeIfAvailable(Args, CmdArgs);
+ }
+
+ Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_u});
+
+ ToolChain.AddFilePathLibArgs(Args, CmdArgs);
+
+ CmdArgs.push_back(Args.MakeArgString(
+ "-L" + ToolChain.getGCCInstallation().getInstallPath()));
+
+ if (D.isUsingLTO()) {
+ assert(!Inputs.empty() && "Must have at least one input.");
+ // Find the first filename InputInfo object.
+ auto Input = llvm::find_if(
+ Inputs, [](const InputInfo &II) -> bool { return II.isFilename(); });
+ if (Input == Inputs.end())
+ // For a very rare case, all of the inputs to the linker are
+ // InputArg. If that happens, just use the first InputInfo.
+ Input = Inputs.begin();
+
+ tools::addLTOOptions(ToolChain, Args, CmdArgs, Output, *Input,
+ D.getLTOMode() == LTOK_Thin);
+ }
+
+ if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+ CmdArgs.push_back("--no-demangle");
+
+ bool NeedsSanitizerDeps = tools::addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+ bool NeedsXRayDeps = tools::addXRayRuntime(ToolChain, Args, CmdArgs);
+ tools::addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
+ tools::AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
+ tools::addHIPRuntimeLibArgs(ToolChain, C, Args, CmdArgs);
+
+ // The profile runtime also needs access to system libraries.
+ getToolChain().addProfileRTLibs(Args, CmdArgs);
+
+ if (D.CCCIsCXX() &&
+ !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+ options::OPT_r)) {
+ if (ToolChain.ShouldLinkCXXStdlib(Args)) {
+ bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
+ !Args.hasArg(options::OPT_static);
+ if (OnlyLibstdcxxStatic)
+ CmdArgs.push_back("-Bstatic");
+ ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+ if (OnlyLibstdcxxStatic)
+ CmdArgs.push_back("-Bdynamic");
+ }
+ CmdArgs.push_back("-lm");
+ }
+
+ // Silence warnings when linking C code with a C++ '-stdlib' argument.
+ Args.ClaimAllArgs(options::OPT_stdlib_EQ);
+
+ // Additional linker set-up and flags for Fortran. This is required in order
+ // to generate executables. As Fortran runtime depends on the C runtime,
+ // these dependencies need to be listed before the C runtime below (i.e.
+ // AddRunTimeLibs).
+ if (D.IsFlangMode() &&
+ !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+ tools::addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
+ tools::addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
+ CmdArgs.push_back("-lm");
+ }
+
+ if (!Args.hasArg(options::OPT_nostdlib, options::OPT_r)) {
+ if (!Args.hasArg(options::OPT_nodefaultlibs)) {
+ if (IsStatic || IsStaticPIE)
+ CmdArgs.push_back("--start-group");
+
+ if (NeedsSanitizerDeps)
+ tools::linkSanitizerRuntimeDeps(ToolChain, Args, CmdArgs);
+
+ if (NeedsXRayDeps)
+ tools::linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+
+ bool WantPthread = Args.hasArg(options::OPT_pthread) ||
+ Args.hasArg(options::OPT_pthreads);
+
+ // Use the static OpenMP runtime with -static-openmp
+ bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
+ !Args.hasArg(options::OPT_static);
+
+ // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
+ // require librt. Most modern Linux platforms do, but some may not.
+ if (tools::addOpenMPRuntime(C, CmdArgs, ToolChain, Args, StaticOpenMP,
+ JA.isHostOffloading(Action::OFK_OpenMP),
+ /* GompNeedsRT= */ true))
+ // OpenMP runtimes implies pthreads when using the GNU toolchain.
+ // FIXME: Does this really make sense for all GNU toolchains?
+ WantPthread = true;
+
+ tools::AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
+
+ if (WantPthread)
+ CmdArgs.push_back("-lpthread");
+
+ if (Args.hasArg(options::OPT_fsplit_stack))
+ CmdArgs.push_back("--wrap=pthread_create");
+
+ if (!Args.hasArg(options::OPT_nolibc))
+ CmdArgs.push_back("-lc");
+
+ // Cygwin specific
+ CmdArgs.push_back("-lcygwin");
+ CmdArgs.push_back("-ladvapi32");
+ CmdArgs.push_back("-lshell32");
+ CmdArgs.push_back("-luser32");
+ CmdArgs.push_back("-lkernel32");
+
+ // Add IAMCU specific libs, if needed.
+ if (IsIAMCU)
+ CmdArgs.push_back("-lgloss");
+
+ if (IsStatic || IsStaticPIE)
+ CmdArgs.push_back("--end-group");
+ else
+ tools::AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
+
+ // Add IAMCU specific libs (outside the group), if needed.
+ if (IsIAMCU) {
+ CmdArgs.push_back("--as-needed");
+ CmdArgs.push_back("-lsoftfp");
+ CmdArgs.push_back("--no-as-needed");
+ }
+ }
+
+ if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
+ if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT) {
+ std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+ ToolChain::FT_Object);
+ if (ToolChain.getVFS().exists(crtend)) {
+ std::string P;
+ P = crtend;
+ CmdArgs.push_back(Args.MakeArgString(P));
+ }
+ }
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
+ }
+ }
+
+ Args.addAllArgs(CmdArgs, {options::OPT_T, options::OPT_t});
+
+ const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
+ C.addCommand(std::make_unique<Command>(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Exec, CmdArgs, Inputs, Output));
+}
+
+auto Cygwin::buildLinker() const -> Tool * { return new cygwin::Linker(*this); }
diff --git a/clang/lib/Driver/ToolChains/Cygwin.h b/clang/lib/Driver/ToolChains/Cygwin.h
index d2f72c10c3b01..b76069fba6741 100644
--- a/clang/lib/Driver/ToolChains/Cygwin.h
+++ b/clang/lib/Driver/ToolChains/Cygwin.h
@@ -27,7 +27,28 @@ class LLVM_LIBRARY_VISIBILITY Cygwin : public Generic_GCC {
void
AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
+
+ const GCCInstallationDetector &
+ getGCCInstallation() const { return GCCInstallation; };
+
+protected:
+ Tool *buildLinker() const override;
+};
+
+namespace cygwin {
+class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
+public:
+ Linker(const ToolChain &TC) : Tool("cygwin::Linker", "linker", TC) {}
+
+ bool hasIntegratedCPP() const override { return false; }
+ bool isLinkJob() const override { return true; }
+
+ void ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output, const InputInfoList &Inputs,
+ const llvm::opt::ArgList &TCArgs,
+ const char *LinkingOutput) const override;
};
+} // end namespace cygwin
} // end namespace toolchains
} // end namespace driver
diff --git a/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/i686-pc-msys/10/crtend.o b/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/i686-pc-msys/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/x86_64-pc-cygwin/10/crtend.o b/clang/test/Driver/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/x86_64-pc-cygwin/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/crt0.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/crt0.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/i686-pc-cygwin/10/crtend.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/i686-pc-cygwin/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/x86_64-pc-msys/10/crtend.o b/clang/test/Driver/Inputs/basic_cygwin_tree/usr/lib/gcc/x86_64-pc-msys/10/crtend.o
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/cygwin.cpp b/clang/test/Driver/cygwin.cpp
index dd75c48ddc6b3..8677e84c033fd 100644
--- a/clang/test/Driver/cygwin.cpp
+++ b/clang/test/Driver/cygwin.cpp
@@ -15,19 +15,19 @@
// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include/w32api"
// CHECK-SAME: "-femulated-tls"
// CHECK-SAME: "-exception-model=dwarf"
-// CHECK: "{{.*}}gcc{{(\.exe)?}}"
-// CHECK-SAME: "-m32"
+// CHECK: "{{.*}}ld{{(\.exe)?}}"
+// CHECK-SAME: "-m" "i386pe"
// RUN: %clang -### %s --target=i686-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: --stdlib=platform -static 2>&1 | FileCheck --check-prefix=CHECK-STATIC %s
// CHECK-STATIC: "-cc1" "-triple" "i686-pc-windows-cygnus"
// CHECK-STATIC-SAME: "-static-define"
-// CHECK-STATIC: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-STATIC: "{{.*}}ld{{(\.exe)?}}"
// CHECK-STATIC-SAME: "-static"
// RUN: %clang -### %s --target=i686-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: -shared 2>&1 | FileCheck --check-prefix=CHECK-SHARED %s
-// CHECK-SHARED: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-SHARED: "{{.*}}ld{{(\.exe)?}}"
// CHECK-SHARED-SAME: "-shared"
// RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \
@@ -54,19 +54,19 @@
// CHECK-64-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include/w32api"
// CHECK-64-SAME: "-femulated-tls"
// CHECK-64-SAME: "-exception-model=seh"
-// CHECK-64: "{{.*}}gcc{{(\.exe)?}}"
-// CHECK-64-SAME: "-m64"
+// CHECK-64: "{{.*}}ld{{(\.exe)?}}"
+// CHECK-64-SAME: "-m" "i386pep"
// RUN: %clang -### %s --target=x86_64-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: --stdlib=platform -static 2>&1 | FileCheck --check-prefix=CHECK-64-STATIC %s
// CHECK-64-STATIC: "-cc1" "-triple" "x86_64-pc-windows-cygnus"
// CHECK-64-STATIC-SAME: "-static-define"
-// CHECK-64-STATIC: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-64-STATIC: "{{.*}}ld{{(\.exe)?}}"
// CHECK-64-STATIC-SAME: "-static"
// RUN: %clang -### %s --target=x86_64-pc-cygwin --sysroot=%S/Inputs/basic_cygwin_tree \
// RUN: -shared 2>&1 | FileCheck --check-prefix=CHECK-64-SHARED %s
-// CHECK-64-SHARED: "{{.*}}gcc{{(\.exe)?}}"
+// CHECK-64-SHARED: "{{.*}}ld{{(\.exe)?}}"
// CHECK-64-SHARED-SAME: "-shared"
// RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Thanks! This was on my mental TODO list but I haven't gotten back to looking at it. When I was thinking about this, I was thinking about modifying the MinGW linker with a couple of virtual functions and deriving from it for a Cygwin linker, to avoid so much code duplication. Maybe @mstorsjo can provide an opinion here? Also, the cygwin.cpp test was based on hurd.cpp, I'm thinking more of those tests are relevant now. |
Oh, another proposed implementation to look at: #74933 I was planning to refer to that, MinGW.cpp, and the GCC spec for Cygwin, and see if I could come up with a couple of places where virtual functions could be added to the MinGW::Linker class so that Cygwin::Linker could derive from it and override (I think the "wrap" stuff and list of default libraries) |
Could you let I know output of Does this patch consider to use of LLD? ( gcc and clang can use with LLD:
+1 |
I meant just the Linker class. I don't think it's really possible to avoid a dedicated Cygwin driver, but I'm hoping it can keep close to the Generic_GCC base class rather than redoing so much stuff as MinGW did. |
Most of differences are related to triple, recognizing path, library names and linker options so many of them affects to Linker, so it will have amounts of virtual functions or StringRef members. |
I confirmed 'clang++ -fuse-ld=lld` works. What is your concern about LLD? |
Thanks. It looks like some linker options might still be missing or need adjustments:
Also, a few options should be handled in both
Here are a few issues I've seen when using LLD:
It would be great if this patch could be adjusted to support bootstrapping without requiring extra linker flags. |
If we should consider cross-compiling, it's needed to be aware that |
I have been convinced by your points regarding However, as for |
0675c04
to
1bb281f
Compare
I was confused and not accurate earlier, sorry about that. Former, Latter, |
high-entropy-va would indeed break fork. I don't know about nxcompat. Here is a patch I had to lld to try to change defaults to match Cygwin defaults in BFD, but obviously it doesn't work out because lld can't tell if it's cygwin or mingw. I don't know if you want to try to handle all of these in the cygwin driver. It'd probably be the best place to do it since it does know that it is Cygwin and not MinGW. But it would mean that using gcc -fuse-ld=lld wouldn't work due to wrong defaults. |
I could not reproduce fork() failure even with high-entropy-va.
Edited: Ah, if I run a.exe for many times, fork() sometimes fails. Thanks! |
Added |
This will need more tests added to cover these additional flags that are now hooked up. Off the top of my head, Is the |
Do you have patched to LLD locally? This should fail with
LLD and Clang are both part of LLVM, so they should be expected to work together without requiring extra hacky options. For GCC, we can patch its driver downstream if necessary -- though ideally, this should be addressed upstream, assuming the GCC maintainers are open to it. |
CmdArgs.push_back("thumb2pe"); | ||
break; | ||
case llvm::Triple::aarch64: | ||
if (ToolChain.getEffectiveTriple().isWindowsArm64EC()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to list only targets that are supported by Cygwin or reflect the MinGW list.
MinGW now support a new arch:
llvm-project/clang/lib/Driver/ToolChains/MinGW.cpp
Lines 134 to 136 in a102342
case llvm::Triple::aarch64: | |
if (Args.hasArg(options::OPT_marm64x)) | |
CmdArgs.push_back("arm64xpe"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks.
this new driver should be passing all the necessary |
Thanks. Test updated. |
I noticed another bit missing from the MinGW driver: llvm-project/clang/lib/Driver/ToolChains/MinGW.cpp Lines 198 to 208 in f761d73
|
Ah, yes, I was wrong. Paths are added here that I missed. Sorry. llvm-project/clang/lib/Driver/ToolChains/Cygwin.cpp Lines 47 to 49 in dce445b
|
Thanks. Applied with additional test. |
This also happened today, and not only with LLD but also with BFD, so "due to different handling of |
I am not sure why the issue happens; however, I doubt whether -u symbol
--undefined=symbol
Force symbol to be entered in the output file as an undefined
symbol. Doing this may, for example, trigger linking of additional
modules from standard libraries. -u may be repeated with different
option arguments to enter additional undefined symbols. This option
is equivalent to the "EXTERN" linker script command.
If this option is being used to force additional modules to be
pulled into the link, and if it is an error for the symbol to remain
undefined, then the option --require-defined should be used instead. The symbol |
Do the same for dynamicbase and nxcompat as a precaution. |
I mean many parts of |
I see. _Znwm etc. are mangled name for
Therefore, So, what is the meaning of linking these symbols explicitly? Is your concern that these symbols might be unexpectedly nulled by --wrap? |
I was thinking that
|
I made a quick test and I'm seeing |
Nope, that's not the problem. The problem is that this relies on weak symbols, and that they end up as Test code: #include <stdio.h>
int main (void)
{
char *p = new char (0);
printf ("%p\n", p);
delete p;
return 0;
} With ld.bfd:
with ld.lld:
Notably, those ending in Weak symbols do not work out well in PE/COFF, but apparently Cygwin is relying on them working for this. (https://www.cygwin.com/cgit/newlib-cygwin/tree/winsup/cygwin/lib/_cygwin_crt0_common.cc) |
They work just fine in clang+lld environments for mingw targets, but there's a handful of known issues relating to them with gcc+ld.bfd, for mingw. But apparently those issues don't hit cygwin targets. |
Sorry for causing extra work -- it seems my explanation and investigation weren’t quite sufficient. Thank you for the elaboration.
Might That said, I also encountered the same issue with BFD-ld (when building libc++ with @tyan0's patches from cygwin-packages), so I should try to reproduce it again to be sure. It may take a few days to rebuild and investigate... |
dynamicbase is needed for LLD. |
What I saw is probably a bug in LLD, and not related to this PR anyway. |
only because --enable-auto-image-base and the differing default image bases for Cygwin are not implemented in LLD |
https://cygwin.com/pipermail/cygwin/2025-July/258547.html It seems you get NULLs as expected, not garbage, if dynamicbase is not set (so basically, garbage is due to relocations) |
Thanks for explanation. I'll drop the code to let |
CmdArgs.push_back("--wrap=_Znwm"); | ||
CmdArgs.push_back("--wrap=_Znam"); | ||
CmdArgs.push_back("--wrap=_ZdlPv"); | ||
CmdArgs.push_back("--wrap=_ZdaPv"); | ||
CmdArgs.push_back("--wrap=_ZnwmRKSt9nothrow_t"); | ||
CmdArgs.push_back("--wrap=_ZnamRKSt9nothrow_t"); | ||
CmdArgs.push_back("--wrap=_ZdlPvRKSt9nothrow_t"); | ||
CmdArgs.push_back("--wrap=_ZdaPvRKSt9nothrow_t"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that for 32-bit the m
s need to be j
s in the _Znam
and _Znwm
symbols. I guess they don't need an extra leading underscore, GCC doesn't have them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Is this true for all 32bit platform? I mean, also for 32bit arm? Or only for i686?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. This should be for all 32bit platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AArch64 uses yet another definition for size_t
(resulting in _Znwy
), but I think this can be left out of this PR. Supporting aarch64-cygwin would likely require fixing other issues in both the front-end and back-end anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_Znwy? not _Znwn? For unsigned long long, _Znwn is used IIUC. _Znwy is for unsigned short, isn't it?
Edited: No, I was wrong. _Znwy is new for unsigned long long indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ echo _Znwy _Znwn _Znwt | c++filt
operator new(unsigned long long) operator new(__int128) operator new(unsigned short)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is right for aarch64-w64-mingw32 but a cygwin target should override that because it is LP64 not LLP64 like Windows/mingw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a silly idea, but clang obviously knows how to mangle names - is this API accessible to the driver?
...so that libc++, compiler-rt, and libunwind can be used by the options: -stdlib=libc++, -rtlib=compiler-rt, and -unwindlib=libunwind respectively. Along with this change, the test for this driver is also trimmed a bit. This is a follow-up patch to the commit 52924a2. Signed-off-by: Takashi Yano <[email protected]>
...so that
libc++
,compiler-rt
, andlibunwind
can be used by the options:-stdlib=libc++
,-rtlib=compiler-rt
, and-unwindlib=libunwind
respectively. Along with this change, the test for this driver is also trimmed a bit.This is a followup patch for 52924a2.