[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: chill, peter.smith, simon_tatham, stuij.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some options are only claimed in AddARMTargetArgs/AddAArch64TargetArgs/etc (called by
Clang::RenderTargetOptions).
For assembler input, `Add*TargetArgs` is not called. If an option is
unclaimed, it either leads to a -Wunused-command-line-argument warning
or an error (if `TargetSpecific` is set)

  // clang '-###' --target=aarch64 -mbranch-protection=bti -c a.s
  clang: error: unsupported option '-mbranch-protection=' for target 'aarch64'

It seems that ignoring the diagnostics is most useful as it matches GCC
and users can do

  clang --target=aarch64 -mbranch-protection=bti -S a.c
  clang --target=aarch64 -mbranch-protection=bti -c a.s

without changing the options.

Planned for main and release/17.x


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158688

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/aarch64-security-options.c
  clang/test/Driver/arm-security-options.c


Index: clang/test/Driver/arm-security-options.c
===================================================================
--- clang/test/Driver/arm-security-options.c
+++ clang/test/Driver/arm-security-options.c
@@ -75,6 +75,9 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv7-r -c %s -### -mbranch-protection=bti 2>&1 | \
 // RUN: FileCheck %s --check-prefix=INCOMPATIBLE-ARCH
 
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror --target=arm-arm-none-eabi -march=armv7-m -x assembler -c %s -mbranch-protection=pac-ret
+
 // RA-OFF: "-msign-return-address=none"
 // RA-NON-LEAF: "-msign-return-address=non-leaf"
 // RA-ALL: "-msign-return-address=all"
Index: clang/test/Driver/aarch64-security-options.c
===================================================================
--- clang/test/Driver/aarch64-security-options.c
+++ clang/test/Driver/aarch64-security-options.c
@@ -27,8 +27,12 @@
 // RUN: not %clang --target=aarch64 -c %s -### -mbranch-protection=bar     2>&1 | \
 // RUN: FileCheck %s --check-prefix=BAD-BP-PROTECTION --check-prefix=WARN
 
-// RUN: %clang --target=aarch64 -### -o /dev/null -mbranch-protection=standard /dev/null 2>&1 | \
-// RUN: FileCheck --allow-empty %s --check-prefix=LINKER-DRIVER
+/// Check that the linker driver doesn't warn about -mbranch-protection=standard
+/// as an unused option.
+// RUN: %clang --target=aarch64 -Werror -### -o /dev/null -mbranch-protection=standard /dev/null
+
+/// Don't warn for assembler input.
+// RUN: %clang -### --target=aarch64 -Werror -c -x assembler %s -mbranch-protection=standard -msign-return-address=all
 
 // WARN-NOT: warning: ignoring '-mbranch-protection=' option because the 'aarch64' architecture does not support it [-Wbranch-protection]
 
@@ -49,7 +53,3 @@
 
 // BAD-B-KEY-COMBINATION: unsupported argument 'b-key' to option '-mbranch-protection='
 // BAD-LEAF-COMBINATION: unsupported argument 'leaf' to option '-mbranch-protection='
-
-// Check that the linker driver doesn't warn about -mbranch-protection=standard
-// as an unused option.
-// LINKER-DRIVER-NOT: warning: argument unused during compilation: '-mbranch-protection=standard'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -539,6 +539,10 @@
       }
     }
 
+    // Some target-specific options are only handled in AddARMTargetArgs, not
+    // called for assembler input. Claim them.
+    Args.claimAllArgs(options::OPT_mbranch_protection_EQ);
+
     // The integrated assembler doesn't implement e_flags setting behavior for
     // -meabi=gnu (gcc -mabi={apcs-gnu,atpcs} passes -meabi=gnu to gas). For
     // compatibility we accept but warn.
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -259,12 +259,18 @@
   // Enable NEON by default.
   Features.push_back("+neon");
   llvm::StringRef WaMArch;
-  if (ForAS)
+  if (ForAS) {
+    // Some target-specific options are only handled in AddAArch64TargetArgs,
+    // not called for assembler input. Claim them.
+    Args.claimAllArgs(options::OPT_mbranch_protection_EQ,
+                                options::OPT_msign_return_address_EQ);
+
     for (const auto *A :
          Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
       for (StringRef Value : A->getValues())
         if (Value.startswith("-march="))
           WaMArch = Value.substr(7);
+  }
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158688.552941.patch
Type: text/x-patch
Size: 3807 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230824/32c111c6/attachment.bin>