[Clang][Driver] Revise Cygwin ToolChain to call linker directly #147960
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. |
llvmbot
added
clang
|
@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 |
tyan0
force-pushed
the
main
branch
2 times, most recently
from
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. |
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 |
There was a problem hiding this comment.
I believe that for 32-bit the ms need to be js 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.
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.
I see. This should be for all 32bit platforms.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
GlobalDecl isn't accessible from this context. The Driver component can reference only the Basic component.
However, I found a way to see what size_t does mean on target triple (see diff; edit: don't add _).
diff
diff --git a/clang/lib/Driver/ToolChains/Cygwin.cpp b/clang/lib/Driver/ToolChains/Cygwin.cpp
index 079911b0f0ac..e2561172d386 100644
--- a/clang/lib/Driver/ToolChains/Cygwin.cpp
+++ b/clang/lib/Driver/ToolChains/Cygwin.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Cygwin.h"
+#include "clang/Basic/TargetInfo.h"
#include "clang/Config/config.h"
#include "clang/Driver/CommonArgs.h"
#include "clang/Driver/Compilation.h"
@@ -177,24 +178,29 @@ void cygwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("console");
}
- if (ToolChain.getTriple().isArch32Bit()) {
- CmdArgs.push_back("--wrap=_Znwj");
- CmdArgs.push_back("--wrap=_Znaj");
- CmdArgs.push_back("--wrap=_ZdlPv");
- CmdArgs.push_back("--wrap=_ZdaPv");
- CmdArgs.push_back("--wrap=_ZnwjRKSt9nothrow_t");
- CmdArgs.push_back("--wrap=_ZnajRKSt9nothrow_t");
- CmdArgs.push_back("--wrap=_ZdlPvRKSt9nothrow_t");
- CmdArgs.push_back("--wrap=_ZdaPvRKSt9nothrow_t");
- } else {
- 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");
+ auto TI = targets::AllocateTarget(ToolChain.getTriple(), {});
+ const auto TypeChar = [&]() {
+ switch (TI->getSizeType()) {
+ case TransferrableTargetInfo::UnsignedShort:
+ return 't';
+ case TransferrableTargetInfo::UnsignedInt:
+ return 'j';
+ case TransferrableTargetInfo::UnsignedLong:
+ return 'm';
+ case TransferrableTargetInfo::UnsignedLongLong:
+ return 'y';
+ default:
+ llvm_unreachable("Invalid SizeType");
+ }
+ }();
+ const auto *const LabelPrefix = TI->getUserLabelPrefix();
+ for (std::string S : {"_Znw%", "_Zna%", "_ZdlPv", "_ZdaPv",
+ "_Znw%RKSt9nothrow_t", "_Zna%RKSt9nothrow_t",
+ "_ZdlPvRKSt9nothrow_t", "_ZdaPvRKSt9nothrow_t"}) {
+ auto Pos = S.find('%');
+ if (Pos != S.npos)
+ S[Pos] = TypeChar;
+ CmdArgs.push_back(Args.MakeArgString("--wrap=" + S));
}
if (Args.hasArg(options::OPT_mdll))BTW, gcc -m32 doesn't wrap new properly (perhaps the old i686-native gcc can?) and the different mangling rule between new and delete is found from libstdxx_wrapper.cc in cygwin1.dll.
$ echo 'int main() { auto p = new int(3); delete p; }' | g++ -m64 -xc++ - -nostartfiles -nodefaultlibs -Wl,--no-demangle,--entry, -shared -fno-sized-deallocation
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccc30TZw.o:<stdin>:(.text+0x9): undefined reference to `__main'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccc30TZw.o:<stdin>:(.text+0x13): undefined reference to `__wrap__Znwm'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccc30TZw.o:<stdin>:(.text+0x2e): undefined reference to `__wrap__ZdlPv'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccc30TZw.o:<stdin>:(.text+0x3f): undefined reference to `__wrap__ZdlPv'
collect2: エラー: ld はステータス 1 で終了しました
$ echo 'int main() { auto p = new int(3); delete p; }' | g++ -m32 -xc++ - -nostartfiles -nodefaultlibs -Wl,--no-demangle,--entry, -shared -fno-sized-deallocation
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccAA6tBm.o:<stdin>:(.text+0x7): undefined reference to `___main'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccAA6tBm.o:<stdin>:(.text+0x13): undefined reference to `__Znwj'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccAA6tBm.o:<stdin>:(.text+0x2d): undefined reference to `___wrap__ZdlPv'
/usr/lib/gcc/x86_64-pc-cygwin/16/../../../../x86_64-pc-cygwin/bin/ld: /tmp/ccAA6tBm.o:<stdin>:(.text+0x3c): undefined reference to `___wrap__ZdlPv'
collect2: エラー: ld はステータス 1 で終了しました
There was a problem hiding this comment.
That's interesting. I like that in the context of having to add more overloads eventually (once Cygwin and GCC catch up):
++ CmdArgs.push_back("--wrap=_ZdaPvm");
++ CmdArgs.push_back("--wrap=_ZdlPvm");
++ CmdArgs.push_back("--wrap=_ZnwmSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZnamSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZdlPvSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZdlPvmSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZdaPvSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZdaPvmSt11align_val_t");
++ CmdArgs.push_back("--wrap=_ZnwmSt11align_val_tRKSt9nothrow_t");
++ CmdArgs.push_back("--wrap=_ZnamSt11align_val_tRKSt9nothrow_t");
++ CmdArgs.push_back("--wrap=_ZdlPvSt11align_val_tRKSt9nothrow_t");
++ CmdArgs.push_back("--wrap=_ZdaPvSt11align_val_tRKSt9nothrow_t");Oh, also it doesn't seem that you need to prefix _ on these --wrap arguments on i686.
There was a problem hiding this comment.
gcc -m32doesn't wrapnewproperly
I found that gcc tries to wrap operator new(unsigned long) even if -m32.
Oh, also it doesn't seem that you need to prefix
_on these--wraparguments on i686.
Ah, true. _ will be added by the linker after wrapping. This mangling confuses me so much...
There was a problem hiding this comment.
It's probably an artifact of using -m32 - cygwin.h has it right, but cygwin-w64.h has the 64-bit variant with m instead of j, I guess it's using that even with -m32.
...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]>
| if (!saw_high_entropy_va) | ||
| CmdArgs.push_back("--disable-high-entropy-va"); |
There was a problem hiding this comment.
Got an error on 32-bit due to the presence of this option. Should only be added on 64-bit.
There was a problem hiding this comment.
| if (!saw_high_entropy_va) | |
| CmdArgs.push_back("--disable-high-entropy-va"); | |
| if (!saw_high_entropy_va && ToolChain.getTriple().isArch64Bit()) | |
| CmdArgs.push_back("--disable-high-entropy-va"); |
|
There seems to be some other issue with this patch on 32-bit that I need to track down: when building llvm/clang/compiler-rt 21 with this patch with a clang 21 with this patch, I get errors building compiler-rt builtins (which uses the freshly-built clang). The fact that this only happens on the second build is confusing, because it does use the freshly-built clang in this case. I'm thinking it must be something to do with linker flags. |
The |
Don't add --disable-high-entropy-va on 32-bit, it's only supported on 64-bit For executables, add --large-address-aware for 32-bit, and --tsaware for all, to match gcc. Map -rdynamic to --export-all-symbols to match https://cygwin.com/cgit/cygwin-packages/gcc/tree/0004-Cygwin-add-dummy-pthread-tsaware-and-large-address-a.patch
|
I'm planning to put a new PR to port #68571 to Cygwin, which is needed to make |
I can try to have a look; previously when there was activity on this PR I felt that there still were things that you and @jeremyd2019 were discussing around this, but if you've converged to something you all are comfortable with now, I can try to do a proper review for potential merge. |
Thanks. |
| if (Triple.isOSDarwin()) | ||
| return "darwin"; | ||
| if (Triple.isWindowsCygwinEnvironment()) | ||
| return "cygwin"; |
There was a problem hiding this comment.
What does the fallback (getOS() which defaults to Triple.getOSName()) return here, compared with this new value?
And I'm a little unsure how this change fits in with the rest of this change; isn't this change orthogonal to the rest? The rest of the change is about directly calling the linker (while still using the same libgcc etc as otherwise?), while this change is needed for e.g. getting the right directory for compiler-rt builtins and similar? I.e. this change might be ok in itself, but it feels to me like it's a separate change for a separate PR?
| 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); |
There was a problem hiding this comment.
From which toolchain file is this bit copied? We don't have these bits in the mingw toolchain. Has someone tested and used cygwin with -rtlib=compiler-rt already, or is this untested theoretical code?
There was a problem hiding this comment.
I have local patches to build compiler-rt (@tyan0 might have another work), and succeeded to compile a program with --rtlib=compiler-rt (aside from usefulness in current Cygwin toolchain environment).
FWIW, the built binaries are in https://github.com/kikairoya/llvm-cygwin-patches/actions/runs/18615011929 , artifacts named package-*-cygwin-fullpatch.
| bool saw_nxcompat = false; | ||
| for (const char *Arg : CmdArgs) { | ||
| if (StringRef(Arg) == "-high-entropy-va" || | ||
| StringRef(Arg) == "--high-entropy-va" || |
There was a problem hiding this comment.
I wonder if there's a more clever way to express this, without having to repeat every constant twice, for checking for one or two leading dashes? And the StringRef(Arg) wrapping maybe could be stored in a variable instead of remade in each comparison?
There was a problem hiding this comment.
thinking about this more, these are last-wins options, so couldn't we just add our intended defaults before the AddLinkerInputs and be done with it? Or am I forgetting the reason why that wouldn't work?
5 participants
...so that
libc++,compiler-rt, andlibunwindcan be used by the options:-stdlib=libc++,-rtlib=compiler-rt, and-unwindlib=libunwindrespectively. Along with this change, the test for this driver is also trimmed a bit.This is a followup patch for 52924a2.