Skip to content

[Driver] -fno-plt: warn for unsupported targets #124081

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

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jan 23, 2025

-fno-plt is an ELF specific option that is only implemented for x86 (for
a long time) and AArch64 (#78890). GCC doesn't bother to give a
diagnostic on Windows. -fno-plt is somewhat popular and we've been
ignoring it for unsupported targets for a while, so just report a
warning for unsupported targets.

Created using spr 1.3.5-bogner
@MaskRay MaskRay requested a review from mstorsjo January 23, 2025 07:03
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 23, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

Changes

-fno-plt is an ELF specific option that GCC doesn't bother to give a
diagnostic on Windows. This option is somewhat popular and we've been
ignoring it for unsupported targets for a while (x86 support for a
while, #78890 for AArch64 support), so just report a warning for
unsupported object file formats.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2-3)
  • (added) clang/test/Driver/fno-plt.c (+7)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 33f08cf28feca1..5df58005a52373 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6141,9 +6141,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-fno-direct-access-external-data");
   }
 
-  if (Args.hasFlag(options::OPT_fno_plt, options::OPT_fplt, false)) {
-    CmdArgs.push_back("-fno-plt");
-  }
+  if (Triple.isOSBinFormatELF())
+    Args.addOptOutFlag(CmdArgs, options::OPT_fplt, options::OPT_fno_plt);
 
   // -fhosted is default.
   // TODO: Audit uses of KernelOrKext and see where it'd be more appropriate to
diff --git a/clang/test/Driver/fno-plt.c b/clang/test/Driver/fno-plt.c
new file mode 100644
index 00000000000000..d5189e2b508b2a
--- /dev/null
+++ b/clang/test/Driver/fno-plt.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -c --target=aarch64 -fno-plt -Werror %s 2>&1 | FileCheck %s
+// RUN: %clang -### -c --target=aarch64 -fno-plt -fplt -Werror %s 2>&1 | FileCheck %s --check-prefix=NO
+// RUN: %clang -### -c --target=aarch64-windows -fno-plt %s 2>&1 | FileCheck %s --check-prefixes=WARN,NO
+
+// WARN: warning: argument unused during compilation: '-fno-plt' [-Wunused-command-line-argument]
+// CHECK: "-fno-plt"
+// NO-NOT: "-fno-plt"

.
Created using spr 1.3.5-bogner
@MaskRay MaskRay changed the title [Driver] -fno-plt: warn for non-ELF [Driver] -fno-plt: warn for unsupported targets Jan 23, 2025
Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! This seems to nicely fix the issue and make the situation clearer.

CC @Martchus.

@@ -0,0 +1,9 @@
// RUN: %clang -### -c --target=aarch64 -fno-plt -Werror %s 2>&1 | FileCheck %s
// RUN: %clang -### -c --target=x86_64 -fno-plt -Werror %s 2>&1 | FileCheck %s
// RUN: %clang -### -c --target=aarch64 -fno-plt -fplt -Werror %s 2>&1 | FileCheck %s --check-prefix=NO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's quite many negations involved in this check line, can you rename the NO prefix to e.g. PLT or something like that, to signify the case when -fplt is in effect? Then again, using --check-prefix=PLT for the aarch64-windows target feels a bit misleading as well. Perhaps if we'd rename CHECK into NOPLT and NO into DEFAULT?

Martchus added a commit to Martchus/PKGBUILDs that referenced this pull request Jan 23, 2025
This is an elf-only flag and specifying it on PE targets has no effect -
except that it actually has a quite negative effect when used with LLVM/Clang
as of LLVM 19 (see
llvm/llvm-project#78890 (comment)).
So it is probably better to avoid this flag (regardless of the used toolchain).
LLVM will also warn about it in the future (see
llvm/llvm-project#124081).
Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 2f76e2b into main Jan 23, 2025
4 of 6 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/driver-fno-plt-warn-for-non-elf branch January 23, 2025 17:00
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 23, 2025
-fno-plt is an ELF specific option that is only implemented for x86 (for
a long time) and AArch64 (#78890). GCC doesn't bother to give a
diagnostic on Windows. -fno-plt is somewhat popular and we've been
ignoring it for unsupported targets for a while, so just report a
warning for unsupported targets.

Pull Request: llvm/llvm-project#124081
@ziyao233 ziyao233 mentioned this pull request May 8, 2025
14 tasks
ziyao233 added a commit to ziyao233/eweos-packages that referenced this pull request May 8, 2025
LLVM doesn't support -fno-plt on RISC-V and LoongArch and starts
emitting a warning if it's supplied on unsupported targets since LLVM
20, causing various errors when configuring packages. Let's remove the
flag on riscv64 and loongarch64.

As we have started moving to LLVM 20, the problem must be mitigated by
stripping -fno-plt away from CFLAGS/LDFLAGS on riscv64 and loongarch64.

Link: llvm/llvm-project#124081
Reference: eweOS#3594 (comment)
ziyao233 added a commit to eweOS/packages that referenced this pull request May 8, 2025
LLVM doesn't support -fno-plt on RISC-V and LoongArch and starts
emitting a warning if it's supplied on unsupported targets since LLVM
20, causing various errors when configuring packages. Let's remove the
flag on riscv64 and loongarch64.

As we have started moving to LLVM 20, the problem must be mitigated by
stripping -fno-plt away from CFLAGS/LDFLAGS on riscv64 and loongarch64.

Link: llvm/llvm-project#124081
Reference: #3594 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants