Skip to content

[PAC][Driver] Support ptrauth flags only on ARM64 Darwin or with pauthtest ABI #113152

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

Open
wants to merge 7 commits into
base: users/kovdan01/pauthtest-linux-specific
Choose a base branch
from

Conversation

kovdan01
Copy link
Contributor

@kovdan01 kovdan01 commented Oct 21, 2024

Most ptrauth flags are ABI-affecting, so usually we do not want them to be
exposed to end users. Allow them only in the following cases:

  • ARM64 Darwin (under certain conditions, some ptrauth driver flags are
    intended to be used in this case);
  • pauthtest ABI (it's intended to be used for experimenting with signing schema
    and the signing schema is explicitly encoded in the pauth elf marking).

Leave -faarch64-jump-table-hardening available for all AArch64 targets
since it's not ABI-affecting.

Copy link
Contributor Author

kovdan01 commented Oct 21, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kovdan01 kovdan01 requested a review from asl October 21, 2024 10:45
@kovdan01 kovdan01 marked this pull request as ready for review October 21, 2024 17:40
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang

Author: Daniil Kovalev (kovdan01)

Changes

Most ptrauth flags are ABI-affecting, so allow them only for Linux and
Darwin OS which explicitly support pointer authentication. Leave
-faarch64-jump-table-hardening available for all AArch64 targets since
it's not ABI-affecting.


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

6 Files Affected:

  • (modified) clang/include/clang/Driver/ToolChain.h (+3)
  • (modified) clang/lib/Driver/ToolChain.cpp (+42)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (-28)
  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+3)
  • (modified) clang/lib/Driver/ToolChains/Linux.cpp (+6-2)
  • (modified) clang/test/Driver/aarch64-ptrauth.c (+43-24)
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 5347e29be91439..0c0fccec4b933b 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -240,6 +240,9 @@ class ToolChain {
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
 
+  static void addPointerAuthFlags(const llvm::opt::ArgList &DriverArgs,
+                                  llvm::opt::ArgStringList &CC1Args);
+
   static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
                             const Twine &C = "", const Twine &D = "");
   ///@}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 016db9469c6bec..9c53986756caaa 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1264,6 +1264,48 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
   }
 }
 
+/*static*/ void
+ToolChain::addPointerAuthFlags(const llvm::opt::ArgList &DriverArgs,
+                               llvm::opt::ArgStringList &CC1Args) {
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_intrinsics,
+                          options::OPT_fno_ptrauth_intrinsics);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_calls,
+                          options::OPT_fno_ptrauth_calls);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_returns,
+                          options::OPT_fno_ptrauth_returns);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_auth_traps,
+                          options::OPT_fno_ptrauth_auth_traps);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_vtable_pointer_address_discrimination,
+      options::OPT_fno_ptrauth_vtable_pointer_address_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_vtable_pointer_type_discrimination,
+      options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_type_info_vtable_pointer_discrimination,
+      options::OPT_fno_ptrauth_type_info_vtable_pointer_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_function_pointer_type_discrimination,
+      options::OPT_fno_ptrauth_function_pointer_type_discrimination);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_indirect_gotos,
+                          options::OPT_fno_ptrauth_indirect_gotos);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_init_fini,
+                          options::OPT_fno_ptrauth_init_fini);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_init_fini_address_discrimination,
+      options::OPT_fno_ptrauth_init_fini_address_discrimination);
+}
+
 /*static*/ std::string ToolChain::concat(StringRef Path, const Twine &A,
                                          const Twine &B, const Twine &C,
                                          const Twine &D) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 84bb77d451641a..fdfc0787098a48 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1784,34 +1784,6 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
 
   AddUnalignedAccessWarning(CmdArgs);
 
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_intrinsics,
-                    options::OPT_fno_ptrauth_intrinsics);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_calls,
-                    options::OPT_fno_ptrauth_calls);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_returns,
-                    options::OPT_fno_ptrauth_returns);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_auth_traps,
-                    options::OPT_fno_ptrauth_auth_traps);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_vtable_pointer_address_discrimination,
-      options::OPT_fno_ptrauth_vtable_pointer_address_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_vtable_pointer_type_discrimination,
-      options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_type_info_vtable_pointer_discrimination,
-      options::OPT_fno_ptrauth_type_info_vtable_pointer_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_function_pointer_type_discrimination,
-      options::OPT_fno_ptrauth_function_pointer_type_discrimination);
-
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_indirect_gotos,
-                    options::OPT_fno_ptrauth_indirect_gotos);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_init_fini,
-                    options::OPT_fno_ptrauth_init_fini);
-  Args.addOptInFlag(CmdArgs,
-                    options::OPT_fptrauth_init_fini_address_discrimination,
-                    options::OPT_fno_ptrauth_init_fini_address_discrimination);
   Args.addOptInFlag(CmdArgs, options::OPT_faarch64_jump_table_hardening,
                     options::OPT_fno_aarch64_jump_table_hardening);
 }
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 87380869f6fdab..3874cc3d98e80a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -3137,6 +3137,9 @@ void Darwin::addClangTargetOptions(
     if (!RequiresSubdirectorySearch)
       CC1Args.push_back("-fno-modulemap-allow-subdirectory-search");
   }
+
+  if (getTriple().isAArch64())
+    addPointerAuthFlags(DriverArgs, CC1Args);
 }
 
 void Darwin::addClangCC1ASTargetOptions(
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index ae5abf44fb5566..9f38adbd691c27 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -529,8 +529,12 @@ void Linux::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                                   llvm::opt::ArgStringList &CC1Args,
                                   Action::OffloadKind DeviceOffloadKind) const {
   llvm::Triple Triple(ComputeEffectiveClangTriple(DriverArgs));
-  if (Triple.isAArch64() && Triple.getEnvironment() == llvm::Triple::PAuthTest)
-    handlePAuthABI(getDriver(), DriverArgs, CC1Args);
+  if (Triple.isAArch64()) {
+    addPointerAuthFlags(DriverArgs, CC1Args);
+    if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
+      handlePAuthABI(getDriver(), DriverArgs, CC1Args);
+  }
+
   Generic_ELF::addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadKind);
 }
 
diff --git a/clang/test/Driver/aarch64-ptrauth.c b/clang/test/Driver/aarch64-ptrauth.c
index 88841ee0b0b7bb..b16fb1ea3b1aca 100644
--- a/clang/test/Driver/aarch64-ptrauth.c
+++ b/clang/test/Driver/aarch64-ptrauth.c
@@ -4,7 +4,7 @@
 // NONE:     "-cc1"
 // NONE-NOT: "-fptrauth-
 
-// RUN: %clang -### -c --target=aarch64 \
+// RUN: %clang -### -c --target=aarch64-linux \
 // RUN:   -fno-ptrauth-intrinsics -fptrauth-intrinsics \
 // RUN:   -fno-ptrauth-calls -fptrauth-calls \
 // RUN:   -fno-ptrauth-returns -fptrauth-returns \
@@ -17,7 +17,20 @@
 // RUN:   -fno-ptrauth-init-fini-address-discrimination -fptrauth-init-fini-address-discrimination \
 // RUN:   -fno-aarch64-jump-table-hardening -faarch64-jump-table-hardening \
 // RUN:   %s 2>&1 | FileCheck %s --check-prefix=ALL
-// ALL: "-cc1"{{.*}} "-fptrauth-intrinsics" "-fptrauth-calls" "-fptrauth-returns" "-fptrauth-auth-traps" "-fptrauth-vtable-pointer-address-discrimination" "-fptrauth-vtable-pointer-type-discrimination" "-fptrauth-type-info-vtable-pointer-discrimination" "-fptrauth-indirect-gotos" "-fptrauth-init-fini" "-fptrauth-init-fini-address-discrimination" "-faarch64-jump-table-hardening"
+// RUN: %clang -### -c --target=arm64-darwin \
+// RUN:   -fno-ptrauth-intrinsics -fptrauth-intrinsics \
+// RUN:   -fno-ptrauth-calls -fptrauth-calls \
+// RUN:   -fno-ptrauth-returns -fptrauth-returns \
+// RUN:   -fno-ptrauth-auth-traps -fptrauth-auth-traps \
+// RUN:   -fno-ptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-address-discrimination \
+// RUN:   -fno-ptrauth-vtable-pointer-type-discrimination -fptrauth-vtable-pointer-type-discrimination \
+// RUN:   -fno-ptrauth-type-info-vtable-pointer-discrimination -fptrauth-type-info-vtable-pointer-discrimination \
+// RUN:   -fno-ptrauth-indirect-gotos -fptrauth-indirect-gotos \
+// RUN:   -fno-ptrauth-init-fini -fptrauth-init-fini \
+// RUN:   -fno-ptrauth-init-fini-address-discrimination -fptrauth-init-fini-address-discrimination \
+// RUN:   -fno-aarch64-jump-table-hardening -faarch64-jump-table-hardening \
+// RUN:   %s 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL: "-cc1"{{.*}} "-fptrauth-intrinsics" "-fptrauth-calls" "-fptrauth-returns" "-fptrauth-auth-traps" "-fptrauth-vtable-pointer-address-discrimination" "-fptrauth-vtable-pointer-type-discrimination" "-fptrauth-type-info-vtable-pointer-discrimination" "-fptrauth-indirect-gotos" "-fptrauth-init-fini" "-fptrauth-init-fini-address-discrimination"{{.*}} "-faarch64-jump-table-hardening"
 
 // RUN: %clang -### -c --target=aarch64-linux -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI1
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI1
@@ -40,7 +53,7 @@
 // RUN:   -fno-aarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI2
 
 //// Non-linux OS: pauthtest ABI has no effect in terms of passing ptrauth cc1 flags.
-//// An error about unsupported ABI will be emitted later in pipeline (see ERR2 below)
+//// An error about unsupported ABI will be emitted later in pipeline (see ERR3 below)
 // RUN: %clang -### -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI2
 
 // PAUTHABI2:      "-cc1"
@@ -55,7 +68,13 @@
 // PAUTHABI3-NOT:  "-fptrauth-
 // PAUTHABI3-NOT: "-faarch64-jump-table-hardening"
 
-// RUN: not %clang -### -c --target=x86_64 -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
+//// Non-AArch64.
+// RUN: not %clang -### -c --target=x86_64-linux -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
+// RUN:   -fptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-type-discrimination \
+// RUN:   -fptrauth-type-info-vtable-pointer-discrimination -fptrauth-indirect-gotos -fptrauth-init-fini \
+// RUN:   -fptrauth-init-fini-address-discrimination -faarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefixes=ERR1,ERR2
+//// Non-linux and non-Darwin OS.
+// RUN: not %clang -### -c --target=aarch64 -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
 // RUN:   -fptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-type-discrimination \
 // RUN:   -fptrauth-type-info-vtable-pointer-discrimination -fptrauth-indirect-gotos -fptrauth-init-fini \
 // RUN:   -fptrauth-init-fini-address-discrimination -faarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=ERR1
@@ -69,50 +88,50 @@
 // ERR1-NEXT: error: unsupported option '-fptrauth-indirect-gotos' for target '{{.*}}'
 // ERR1-NEXT: error: unsupported option '-fptrauth-init-fini' for target '{{.*}}'
 // ERR1-NEXT: error: unsupported option '-fptrauth-init-fini-address-discrimination' for target '{{.*}}'
-// ERR1-NEXT: error: unsupported option '-faarch64-jump-table-hardening' for target '{{.*}}'
+// ERR2-NEXT: error: unsupported option '-faarch64-jump-table-hardening' for target '{{.*}}'
 
 
-// RUN: not %clang -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR2
+// RUN: not %clang -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR3
 //// The ABI is not specified explicitly, and for non-Linux pauthtest environment does not correspond
 //// to pauthtest ABI (each OS target defines this behavior separately). Do not emit an error.
 // RUN:     %clang -c --target=aarch64-pauthtest       %s -o /dev/null
-// ERR2: error: unknown target ABI 'pauthtest'
+// ERR3: error: unknown target ABI 'pauthtest'
 
 //// PAuth ABI is encoded as environment part of the triple, so don't allow to explicitly set other environments.
-// RUN: not %clang -### -c --target=aarch64-linux-gnu -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR3
-// ERR3: error: unsupported option '-mabi=pauthtest' for target 'aarch64-unknown-linux-gnu'
+// RUN: not %clang -### -c --target=aarch64-linux-gnu -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR4
+// ERR4: error: unsupported option '-mabi=pauthtest' for target 'aarch64-unknown-linux-gnu'
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest -mabi=pauthtest %s
 
 //// The only branch protection option compatible with PAuthABI is BTI.
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=pac-ret %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR4
+// RUN:   FileCheck %s --check-prefix=ERR5
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=pac-ret %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR4
-// ERR4: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR5
+// ERR5: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=gcs %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR5
+// RUN:   FileCheck %s --check-prefix=ERR6
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=gcs %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR5
-// ERR5: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR6
+// ERR6: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=standard %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR6
+// RUN:   FileCheck %s --check-prefix=ERR7
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=standard %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR6
-// ERR6: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR7
+// ERR7: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=all %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR7
+// RUN:   FileCheck %s --check-prefix=ERR8
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=all %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR7
-// ERR7: error: unsupported option '-msign-return-address=all' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR8
+// ERR8: error: unsupported option '-msign-return-address=all' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=non-leaf %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR8
+// RUN:   FileCheck %s --check-prefix=ERR9
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=non-leaf %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR8
-// ERR8: error: unsupported option '-msign-return-address=non-leaf' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR9
+// ERR9: error: unsupported option '-msign-return-address=non-leaf' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=none %s
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=none %s

@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang-driver

Author: Daniil Kovalev (kovdan01)

Changes

Most ptrauth flags are ABI-affecting, so allow them only for Linux and
Darwin OS which explicitly support pointer authentication. Leave
-faarch64-jump-table-hardening available for all AArch64 targets since
it's not ABI-affecting.


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

6 Files Affected:

  • (modified) clang/include/clang/Driver/ToolChain.h (+3)
  • (modified) clang/lib/Driver/ToolChain.cpp (+42)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (-28)
  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+3)
  • (modified) clang/lib/Driver/ToolChains/Linux.cpp (+6-2)
  • (modified) clang/test/Driver/aarch64-ptrauth.c (+43-24)
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 5347e29be91439..0c0fccec4b933b 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -240,6 +240,9 @@ class ToolChain {
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
 
+  static void addPointerAuthFlags(const llvm::opt::ArgList &DriverArgs,
+                                  llvm::opt::ArgStringList &CC1Args);
+
   static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
                             const Twine &C = "", const Twine &D = "");
   ///@}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 016db9469c6bec..9c53986756caaa 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1264,6 +1264,48 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
   }
 }
 
+/*static*/ void
+ToolChain::addPointerAuthFlags(const llvm::opt::ArgList &DriverArgs,
+                               llvm::opt::ArgStringList &CC1Args) {
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_intrinsics,
+                          options::OPT_fno_ptrauth_intrinsics);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_calls,
+                          options::OPT_fno_ptrauth_calls);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_returns,
+                          options::OPT_fno_ptrauth_returns);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_auth_traps,
+                          options::OPT_fno_ptrauth_auth_traps);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_vtable_pointer_address_discrimination,
+      options::OPT_fno_ptrauth_vtable_pointer_address_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_vtable_pointer_type_discrimination,
+      options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_type_info_vtable_pointer_discrimination,
+      options::OPT_fno_ptrauth_type_info_vtable_pointer_discrimination);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_function_pointer_type_discrimination,
+      options::OPT_fno_ptrauth_function_pointer_type_discrimination);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_indirect_gotos,
+                          options::OPT_fno_ptrauth_indirect_gotos);
+
+  DriverArgs.addOptInFlag(CC1Args, options::OPT_fptrauth_init_fini,
+                          options::OPT_fno_ptrauth_init_fini);
+
+  DriverArgs.addOptInFlag(
+      CC1Args, options::OPT_fptrauth_init_fini_address_discrimination,
+      options::OPT_fno_ptrauth_init_fini_address_discrimination);
+}
+
 /*static*/ std::string ToolChain::concat(StringRef Path, const Twine &A,
                                          const Twine &B, const Twine &C,
                                          const Twine &D) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 84bb77d451641a..fdfc0787098a48 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1784,34 +1784,6 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
 
   AddUnalignedAccessWarning(CmdArgs);
 
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_intrinsics,
-                    options::OPT_fno_ptrauth_intrinsics);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_calls,
-                    options::OPT_fno_ptrauth_calls);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_returns,
-                    options::OPT_fno_ptrauth_returns);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_auth_traps,
-                    options::OPT_fno_ptrauth_auth_traps);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_vtable_pointer_address_discrimination,
-      options::OPT_fno_ptrauth_vtable_pointer_address_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_vtable_pointer_type_discrimination,
-      options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_type_info_vtable_pointer_discrimination,
-      options::OPT_fno_ptrauth_type_info_vtable_pointer_discrimination);
-  Args.addOptInFlag(
-      CmdArgs, options::OPT_fptrauth_function_pointer_type_discrimination,
-      options::OPT_fno_ptrauth_function_pointer_type_discrimination);
-
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_indirect_gotos,
-                    options::OPT_fno_ptrauth_indirect_gotos);
-  Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_init_fini,
-                    options::OPT_fno_ptrauth_init_fini);
-  Args.addOptInFlag(CmdArgs,
-                    options::OPT_fptrauth_init_fini_address_discrimination,
-                    options::OPT_fno_ptrauth_init_fini_address_discrimination);
   Args.addOptInFlag(CmdArgs, options::OPT_faarch64_jump_table_hardening,
                     options::OPT_fno_aarch64_jump_table_hardening);
 }
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 87380869f6fdab..3874cc3d98e80a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -3137,6 +3137,9 @@ void Darwin::addClangTargetOptions(
     if (!RequiresSubdirectorySearch)
       CC1Args.push_back("-fno-modulemap-allow-subdirectory-search");
   }
+
+  if (getTriple().isAArch64())
+    addPointerAuthFlags(DriverArgs, CC1Args);
 }
 
 void Darwin::addClangCC1ASTargetOptions(
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index ae5abf44fb5566..9f38adbd691c27 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -529,8 +529,12 @@ void Linux::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                                   llvm::opt::ArgStringList &CC1Args,
                                   Action::OffloadKind DeviceOffloadKind) const {
   llvm::Triple Triple(ComputeEffectiveClangTriple(DriverArgs));
-  if (Triple.isAArch64() && Triple.getEnvironment() == llvm::Triple::PAuthTest)
-    handlePAuthABI(getDriver(), DriverArgs, CC1Args);
+  if (Triple.isAArch64()) {
+    addPointerAuthFlags(DriverArgs, CC1Args);
+    if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
+      handlePAuthABI(getDriver(), DriverArgs, CC1Args);
+  }
+
   Generic_ELF::addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadKind);
 }
 
diff --git a/clang/test/Driver/aarch64-ptrauth.c b/clang/test/Driver/aarch64-ptrauth.c
index 88841ee0b0b7bb..b16fb1ea3b1aca 100644
--- a/clang/test/Driver/aarch64-ptrauth.c
+++ b/clang/test/Driver/aarch64-ptrauth.c
@@ -4,7 +4,7 @@
 // NONE:     "-cc1"
 // NONE-NOT: "-fptrauth-
 
-// RUN: %clang -### -c --target=aarch64 \
+// RUN: %clang -### -c --target=aarch64-linux \
 // RUN:   -fno-ptrauth-intrinsics -fptrauth-intrinsics \
 // RUN:   -fno-ptrauth-calls -fptrauth-calls \
 // RUN:   -fno-ptrauth-returns -fptrauth-returns \
@@ -17,7 +17,20 @@
 // RUN:   -fno-ptrauth-init-fini-address-discrimination -fptrauth-init-fini-address-discrimination \
 // RUN:   -fno-aarch64-jump-table-hardening -faarch64-jump-table-hardening \
 // RUN:   %s 2>&1 | FileCheck %s --check-prefix=ALL
-// ALL: "-cc1"{{.*}} "-fptrauth-intrinsics" "-fptrauth-calls" "-fptrauth-returns" "-fptrauth-auth-traps" "-fptrauth-vtable-pointer-address-discrimination" "-fptrauth-vtable-pointer-type-discrimination" "-fptrauth-type-info-vtable-pointer-discrimination" "-fptrauth-indirect-gotos" "-fptrauth-init-fini" "-fptrauth-init-fini-address-discrimination" "-faarch64-jump-table-hardening"
+// RUN: %clang -### -c --target=arm64-darwin \
+// RUN:   -fno-ptrauth-intrinsics -fptrauth-intrinsics \
+// RUN:   -fno-ptrauth-calls -fptrauth-calls \
+// RUN:   -fno-ptrauth-returns -fptrauth-returns \
+// RUN:   -fno-ptrauth-auth-traps -fptrauth-auth-traps \
+// RUN:   -fno-ptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-address-discrimination \
+// RUN:   -fno-ptrauth-vtable-pointer-type-discrimination -fptrauth-vtable-pointer-type-discrimination \
+// RUN:   -fno-ptrauth-type-info-vtable-pointer-discrimination -fptrauth-type-info-vtable-pointer-discrimination \
+// RUN:   -fno-ptrauth-indirect-gotos -fptrauth-indirect-gotos \
+// RUN:   -fno-ptrauth-init-fini -fptrauth-init-fini \
+// RUN:   -fno-ptrauth-init-fini-address-discrimination -fptrauth-init-fini-address-discrimination \
+// RUN:   -fno-aarch64-jump-table-hardening -faarch64-jump-table-hardening \
+// RUN:   %s 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL: "-cc1"{{.*}} "-fptrauth-intrinsics" "-fptrauth-calls" "-fptrauth-returns" "-fptrauth-auth-traps" "-fptrauth-vtable-pointer-address-discrimination" "-fptrauth-vtable-pointer-type-discrimination" "-fptrauth-type-info-vtable-pointer-discrimination" "-fptrauth-indirect-gotos" "-fptrauth-init-fini" "-fptrauth-init-fini-address-discrimination"{{.*}} "-faarch64-jump-table-hardening"
 
 // RUN: %clang -### -c --target=aarch64-linux -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI1
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI1
@@ -40,7 +53,7 @@
 // RUN:   -fno-aarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI2
 
 //// Non-linux OS: pauthtest ABI has no effect in terms of passing ptrauth cc1 flags.
-//// An error about unsupported ABI will be emitted later in pipeline (see ERR2 below)
+//// An error about unsupported ABI will be emitted later in pipeline (see ERR3 below)
 // RUN: %clang -### -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI2
 
 // PAUTHABI2:      "-cc1"
@@ -55,7 +68,13 @@
 // PAUTHABI3-NOT:  "-fptrauth-
 // PAUTHABI3-NOT: "-faarch64-jump-table-hardening"
 
-// RUN: not %clang -### -c --target=x86_64 -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
+//// Non-AArch64.
+// RUN: not %clang -### -c --target=x86_64-linux -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
+// RUN:   -fptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-type-discrimination \
+// RUN:   -fptrauth-type-info-vtable-pointer-discrimination -fptrauth-indirect-gotos -fptrauth-init-fini \
+// RUN:   -fptrauth-init-fini-address-discrimination -faarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefixes=ERR1,ERR2
+//// Non-linux and non-Darwin OS.
+// RUN: not %clang -### -c --target=aarch64 -fptrauth-intrinsics -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps \
 // RUN:   -fptrauth-vtable-pointer-address-discrimination -fptrauth-vtable-pointer-type-discrimination \
 // RUN:   -fptrauth-type-info-vtable-pointer-discrimination -fptrauth-indirect-gotos -fptrauth-init-fini \
 // RUN:   -fptrauth-init-fini-address-discrimination -faarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=ERR1
@@ -69,50 +88,50 @@
 // ERR1-NEXT: error: unsupported option '-fptrauth-indirect-gotos' for target '{{.*}}'
 // ERR1-NEXT: error: unsupported option '-fptrauth-init-fini' for target '{{.*}}'
 // ERR1-NEXT: error: unsupported option '-fptrauth-init-fini-address-discrimination' for target '{{.*}}'
-// ERR1-NEXT: error: unsupported option '-faarch64-jump-table-hardening' for target '{{.*}}'
+// ERR2-NEXT: error: unsupported option '-faarch64-jump-table-hardening' for target '{{.*}}'
 
 
-// RUN: not %clang -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR2
+// RUN: not %clang -c --target=aarch64 -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR3
 //// The ABI is not specified explicitly, and for non-Linux pauthtest environment does not correspond
 //// to pauthtest ABI (each OS target defines this behavior separately). Do not emit an error.
 // RUN:     %clang -c --target=aarch64-pauthtest       %s -o /dev/null
-// ERR2: error: unknown target ABI 'pauthtest'
+// ERR3: error: unknown target ABI 'pauthtest'
 
 //// PAuth ABI is encoded as environment part of the triple, so don't allow to explicitly set other environments.
-// RUN: not %clang -### -c --target=aarch64-linux-gnu -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR3
-// ERR3: error: unsupported option '-mabi=pauthtest' for target 'aarch64-unknown-linux-gnu'
+// RUN: not %clang -### -c --target=aarch64-linux-gnu -mabi=pauthtest %s 2>&1 | FileCheck %s --check-prefix=ERR4
+// ERR4: error: unsupported option '-mabi=pauthtest' for target 'aarch64-unknown-linux-gnu'
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest -mabi=pauthtest %s
 
 //// The only branch protection option compatible with PAuthABI is BTI.
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=pac-ret %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR4
+// RUN:   FileCheck %s --check-prefix=ERR5
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=pac-ret %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR4
-// ERR4: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR5
+// ERR5: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=gcs %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR5
+// RUN:   FileCheck %s --check-prefix=ERR6
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=gcs %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR5
-// ERR5: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR6
+// ERR6: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=standard %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR6
+// RUN:   FileCheck %s --check-prefix=ERR7
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -mbranch-protection=standard %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR6
-// ERR6: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR7
+// ERR7: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=all %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR7
+// RUN:   FileCheck %s --check-prefix=ERR8
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=all %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR7
-// ERR7: error: unsupported option '-msign-return-address=all' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR8
+// ERR8: error: unsupported option '-msign-return-address=all' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=non-leaf %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR8
+// RUN:   FileCheck %s --check-prefix=ERR9
 // RUN: not %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=non-leaf %s 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=ERR8
-// ERR8: error: unsupported option '-msign-return-address=non-leaf' for target 'aarch64-unknown-linux-pauthtest'
+// RUN:   FileCheck %s --check-prefix=ERR9
+// ERR9: error: unsupported option '-msign-return-address=non-leaf' for target 'aarch64-unknown-linux-pauthtest'
 
 // RUN: %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=none %s
 // RUN: %clang -### -c --target=aarch64-linux-pauthtest       -msign-return-address=none %s

@kovdan01
Copy link
Contributor Author

Would be glad to see everyone's feedback on the changes.

1 similar comment
@kovdan01
Copy link
Contributor Author

kovdan01 commented Nov 9, 2024

Would be glad to see everyone's feedback on the changes.

@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from 418ead4 to 5fb854a Compare November 19, 2024 17:53
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 8878834 to e610387 Compare November 19, 2024 17:54
@kovdan01 kovdan01 changed the title [PAC][Driver] Support ptrauth flags only on AArch64 Linux and ARM64 Darwin [PAC][Driver] Support ptrauth flags only on ARM64 Darwin Nov 19, 2024
@kovdan01
Copy link
Contributor Author

@MaskRay As discussed in #96160 (comment), this drops support for ptrauth driver flags, but with an exception for apple targets - to the best of my knowledge, they have use cases for them. Also tagging @ahmedbougacha

Note: as stated in PR description, the jump table hardening flag (#113149) is left intact since it's not ABI-affecting.

@asl
Copy link
Collaborator

asl commented Nov 20, 2024

I think we'd need to keep driver flags for pauthtest ABI as the intention is to enable easier bringup of pointer authentication ABI on a new target:

  • pauthest is intended to be used for experimenting with signing schema w/o changing compiler
  • the signing schema is explicitly encoded in the pauth elf marking

Though certainly for other named ABIs (say, platform dependent) they should be disabled by default and it is up to platform to decide on signing schema.

@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from 5fb854a to c4de0f5 Compare November 24, 2024 21:54
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from e610387 to 515574c Compare November 24, 2024 21:54
@kovdan01
Copy link
Contributor Author

I think we'd need to keep driver flags for pauthtest ABI as the intention is to enable easier bringup of pointer authentication ABI on a new target

@asl Thanks for suggestion, addressed in 515574c, and updated the PR description correspondingly.

@kovdan01 kovdan01 changed the title [PAC][Driver] Support ptrauth flags only on ARM64 Darwin [PAC][Driver] Support ptrauth flags only on ARM64 Darwin or with pauthtest ABI Nov 24, 2024
@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from c4de0f5 to 9d72205 Compare November 25, 2024 07:53
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 515574c to 226cfcf Compare November 25, 2024 07:53
@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from 9d72205 to 4ff3cdb Compare December 5, 2024 08:37
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 226cfcf to 35c39c5 Compare December 5, 2024 08:38
@@ -543,12 +543,59 @@ static void handlePAuthABI(const Driver &D, const ArgList &DriverArgs,
CC1Args.push_back("-faarch64-jump-table-hardening");
}

static void addPointerAuthFlags(const llvm::opt::ArgList &DriverArgs,
Copy link
Member

@MaskRay MaskRay Dec 7, 2024

Choose a reason for hiding this comment

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

There is a lot of duplication with Darwin.cpp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestion, moved common code to ToolChain::addPointerAuthFlags, see 3afcc78

@@ -1808,34 +1808,6 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,

AddUnalignedAccessWarning(CmdArgs);

Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_intrinsics,
Copy link
Member

Choose a reason for hiding this comment

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

https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#target-specific-options

You can make these options TargetSpecific and only handle them for Linux and Darwin. Then these options will lead to errors for other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can make these options TargetSpecific and only handle them for Linux and Darwin.

@MaskRay It looks like they are already declared as TargetSpecific - see https://github.com/llvm/llvm-project/blob/41cde46/clang/include/clang/Driver/Options.td#L4337-L4357. Please let me know if I'm missing smth and you meant smth else.

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid that you missed the point. Keep the code in Clang.cpp but do the following

// Clang.cpp
if (Linux pauth abi or isDarwin) {
  claim these pauth options
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MaskRay This would explicitly require adding each platform that would want to add pauth here. I don't think this is something desired (do we really want to add further, say, OpenBSD, FreeBSD and other platforms in the condition?)

Copy link
Member

Choose a reason for hiding this comment

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

Yes for such straightforward CC1 forwarding options. The alternative requires a new function in CommonArgs.cpp and changes to various ToolChains/XX.cpp files, which is worse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaskRay Applied your suggestion in 346d1ef

I'm afraid that you missed the point. Keep the code in Clang.cpp but do the following

// Clang.cpp
if (Linux pauth abi or isDarwin) {
  claim these pauth options
}

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

.

@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from 4ff3cdb to de4d000 Compare December 7, 2024 21:16
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 35c39c5 to 3afcc78 Compare December 7, 2024 21:17
@kovdan01 kovdan01 requested a review from MaskRay December 7, 2024 21:18
@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from de4d000 to 491c455 Compare December 16, 2024 08:30
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 3afcc78 to 346d1ef Compare December 16, 2024 08:30
@kovdan01
Copy link
Contributor Author

@MaskRay Previous comments should now be addressed, would be glad to see your feedback on the new version of the PR


// RUN: %clang -### -c --target=aarch64-linux -mabi=pauthtest -fno-ptrauth-intrinsics \
// RUN: -fno-ptrauth-calls -fno-ptrauth-returns -fno-ptrauth-auth-traps \
// RUN: -fno-ptrauth-vtable-pointer-address-discrimination -fno-ptrauth-vtable-pointer-type-discrimination \
// RUN: -fno-ptrauth-type-info-vtable-pointer-discrimination -fno-ptrauth-indirect-gotos \
// RUN: -fno-ptrauth-init-fini -fno-ptrauth-init-fini-address-discrimination \
// RUN: -fno-aarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=PAUTHABI2
// RUN: -fno-aarch64-jump-table-hardening %s 2>&1 | FileCheck %s --check-prefix=PAUTHTEST2
Copy link
Member

Choose a reason for hiding this comment

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

It's not necessary to change PAUTHABI test prefixes to PAUTHTEST. Unnecessary churn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed back to PAUTHABI, thanks

Most ptrauth flags are ABI-affecting, so they should not be exposed to
end users. Under certain conditions, some ptrauth driver flags are intended
to be used for ARM64 Darwin, so allow them in this case.

Leave `-faarch64-jump-table-hardening` available for all AArch64 targets
since it's not ABI-affecting.
@kovdan01 kovdan01 force-pushed the users/kovdan01/pauthtest-linux-specific branch from 491c455 to 74c5de1 Compare December 17, 2024 20:29
@kovdan01 kovdan01 force-pushed the users/kovdan01/ptrauth-flags-only-on-linux-and-darwin branch from 346d1ef to 1e731d3 Compare December 17, 2024 20:30
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
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants