Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Driver] Don't pass -Z to ld for ELF platforms #69120

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Oct 15, 2023

-Z is an Apple ld64 option. ELF linkers don't recognize -Z, except OpenBSD which patched GNU ld to add -Z for zmagic (seems unused)

-Z Produce 'Standard' executables, disables Writable XOR Executable features in resulting binaries.

Some ToolChains have -Z due to copy-and-paste mistakes.

-Z is an Apple ld64 option. ELF linkers don't recognize -Z.
Some ToolChains have -Z due to copy-and-paste mistakes.
@MaskRay MaskRay requested a review from brad0 October 15, 2023 19:08
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 15, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2023

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

-Z is an Apple ld64 option. ELF linkers don't recognize -Z.
Some ToolChains have -Z due to copy-and-paste mistakes.


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

9 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/CSKYToolChain.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/FreeBSD.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/Haiku.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/MinGW.cpp (-1)
  • (modified) clang/lib/Driver/ToolChains/NetBSD.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/OpenBSD.cpp (+2-3)
  • (modified) clang/lib/Driver/ToolChains/RISCVToolchain.cpp (+2-3)
  • (modified) clang/test/Driver/openbsd.c (-4)
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index f363d277a7b71d3..842061c1e1488b0 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -452,9 +452,8 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
   }
 
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_L, options::OPT_T_Group, options::OPT_s,
-                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
+                            options::OPT_s, options::OPT_t, options::OPT_r});
 
   TC.AddFilePathLibArgs(Args, CmdArgs);
 
diff --git a/clang/lib/Driver/ToolChains/CSKYToolChain.cpp b/clang/lib/Driver/ToolChains/CSKYToolChain.cpp
index 2bd91e63fdd5a42..0c280347b2af6a1 100644
--- a/clang/lib/Driver/ToolChains/CSKYToolChain.cpp
+++ b/clang/lib/Driver/ToolChains/CSKYToolChain.cpp
@@ -169,9 +169,8 @@ void CSKY::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_T_Group, options::OPT_s, options::OPT_t,
-                   options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t, options::OPT_r});
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index c936fb88d18ccd9..7a61159ba4a7308 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -262,9 +262,8 @@ void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_T_Group, options::OPT_s, options::OPT_t,
-                   options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t, options::OPT_r});
 
   if (D.isUsingLTO()) {
     assert(!Inputs.empty() && "Must have at least one input.");
diff --git a/clang/lib/Driver/ToolChains/Haiku.cpp b/clang/lib/Driver/ToolChains/Haiku.cpp
index c2653a4a2022edf..9f56a0ea5d612d0 100644
--- a/clang/lib/Driver/ToolChains/Haiku.cpp
+++ b/clang/lib/Driver/ToolChains/Haiku.cpp
@@ -80,9 +80,8 @@ void haiku::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("init_term_dyn.o")));
   }
 
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_L, options::OPT_T_Group, options::OPT_s,
-                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
+                            options::OPT_s, options::OPT_t, options::OPT_r});
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index d3d829a8ddbdb95..39d767795445dbe 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -201,7 +201,6 @@ void tools::MinGW::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddLastArg(CmdArgs, options::OPT_s);
   Args.AddLastArg(CmdArgs, options::OPT_t);
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
-  Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
   // Add asan_dynamic as the first import lib before other libs. This allows
   // asan to be initialized as early as possible to increase its instrumentation
diff --git a/clang/lib/Driver/ToolChains/NetBSD.cpp b/clang/lib/Driver/ToolChains/NetBSD.cpp
index 316e4d56c242acc..1c901f70f72ca2e 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -266,9 +266,8 @@ void netbsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_L, options::OPT_T_Group, options::OPT_s,
-                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
+                            options::OPT_s, options::OPT_t, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
diff --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index 5a9a8584cccb278..2508ef57f827ccf 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -195,9 +195,8 @@ void openbsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_T_Group, options::OPT_s, options::OPT_t,
-                   options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
diff --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index c98f43f6e05eb4b..7e6abd144428783 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -193,9 +193,8 @@ void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_u});
 
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_T_Group, options::OPT_s, options::OPT_t,
-                   options::OPT_Z_Flag, options::OPT_r});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t, options::OPT_r});
 
   // TODO: add C++ includes and libs if compiling C++.
 
diff --git a/clang/test/Driver/openbsd.c b/clang/test/Driver/openbsd.c
index 05d290a309c40c0..c84b54f24fdc24c 100644
--- a/clang/test/Driver/openbsd.c
+++ b/clang/test/Driver/openbsd.c
@@ -30,8 +30,6 @@
 // RUN:   | FileCheck --check-prefix=CHECK-LD-S %s
 // RUN: %clang --target=i686-pc-openbsd -t -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-T %s
-// RUN: %clang --target=i686-pc-openbsd -Z -### %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-LD-Z %s
 // RUN: %clang --target=mips64-unknown-openbsd -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64-LD %s
 // RUN: %clang --target=mips64el-unknown-openbsd -### %s 2>&1 \
@@ -44,8 +42,6 @@
 // CHECK-LD-S: ld{{.*}}" "-e" "__start" "--eh-frame-hdr" "-Bdynamic" "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o" "{{.*}}crtbegin.o" "-L{{.*}}" "-s" "{{.*}}.o" "-lcompiler_rt" "-lc" "-lcompiler_rt" "{{.*}}crtend.o"
 // CHECK-LD-T: "-cc1" "-triple" "i686-pc-openbsd"
 // CHECK-LD-T: ld{{.*}}" "-e" "__start" "--eh-frame-hdr" "-Bdynamic" "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o" "{{.*}}crtbegin.o" "-L{{.*}}" "-t" "{{.*}}.o" "-lcompiler_rt" "-lc" "-lcompiler_rt" "{{.*}}crtend.o"
-// CHECK-LD-Z: "-cc1" "-triple" "i686-pc-openbsd"
-// CHECK-LD-Z: ld{{.*}}" "-e" "__start" "--eh-frame-hdr" "-Bdynamic" "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o" "{{.*}}crtbegin.o" "-L{{.*}}" "-Z" "{{.*}}.o" "-lcompiler_rt" "-lc" "-lcompiler_rt" "{{.*}}crtend.o"
 // CHECK-MIPS64-LD: "-cc1" "-triple" "mips64-unknown-openbsd"
 // CHECK-MIPS64-LD: ld{{.*}}" "-EB" "-e" "__start" "--eh-frame-hdr" "-Bdynamic" "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o" "{{.*}}crtbegin.o" "-L{{.*}}" "{{.*}}.o" "-lcompiler_rt" "-lc" "-lcompiler_rt" "{{.*}}crtend.o"
 // CHECK-MIPS64EL-LD: "-cc1" "-triple" "mips64el-unknown-openbsd"

@brad0
Copy link
Contributor

brad0 commented Oct 15, 2023

OpenBSD has a -Z flag for our BFD linker.

-Z Produce 'Standard' executables, disables Writable XOR Executable features in resulting binaries.

openbsd/src@0abdc37
openbsd/src@58cb065
openbsd/src@0bd5fc2

But it was never ported to our copy of LLD, which is what almost all of our Clang switched archs use, plus I can't seem to find any use of the flag anywhere nowadays.

@MaskRay
Copy link
Member Author

MaskRay commented Oct 16, 2023

OpenBSD has a -Z flag for our BFD linker.

-Z Produce 'Standard' executables, disables Writable XOR Executable features in resulting binaries.

openbsd/src@0abdc37 openbsd/src@58cb065 openbsd/src@0bd5fc2

But it was never ported to our copy of LLD, which is what almost all of our Clang switched archs use, plus I can't seem to find any use of the flag anywhere nowadays.

Thanks for the info! Interesting. I'll update the description.

Another question is whether we want to use clang -Z or clang -Wl,-Z. For newer options we probably should recommend -Wl, more.

@MaskRay MaskRay merged commit 993e839 into llvm:main Oct 16, 2023
@MaskRay MaskRay deleted the drv-Z branch October 16, 2023 02:12
@brad0
Copy link
Contributor

brad0 commented Oct 16, 2023

Another question is whether we want to use clang -Z or clang -Wl,-Z. For newer options we probably should recommend -Wl, more.

Yes, I was thinking if this is used anywhere via the driver that it could be changed to using -Wl.

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

Successfully merging this pull request may close these issues.

3 participants