[clang] [PAC] Do not support some values of branch-protection with ptrauth-returns (PR #125280)
Daniil Kovalev via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 31 12:05:25 PST 2025
https://github.com/kovdan01 created https://github.com/llvm/llvm-project/pull/125280
This patch does two things.
1. Previously, when checking driver arguments, we emitted an error for unsupported values of `-mbranch-protection` when using pauthtest ABI. The reason for that was ptrauth-returns being enabled as part of pauthtest. This patch changes the check against pauthtest to a check against ptrauth-returns.
2. Similarly, check against values of the following function attribute which are unsupported with ptrauth-returns: `__attribute__((target("branch-protection=XXX`. Note that existing `validateBranchProtection` function is used, and current behavior is to ignore the unsupported attribute value, so no error is emitted.
>From 174c0f3a8893fc886d0c1152bef9620abeace6b8 Mon Sep 17 00:00:00 2001
From: Daniil Kovalev <dkovalev at accesssoftek.com>
Date: Fri, 31 Jan 2025 22:57:10 +0300
Subject: [PATCH] [PAC] Do not support some values of branch-protection with
ptrauth-returns
This patch does two things.
1. Previously, when checking driver arguments, we emitted an error for
unsupported values of `-mbranch-protection` when using pauthtest ABI. The
reason for that was ptrauth-returns being enabled as part of pauthtest.
This patch changes the check against pauthtest to a check against
ptrauth-returns.
2. Similarly, check against values of the following function attribute which
are unsupported with ptrauth-returns:
`__attribute__((target("branch-protection=XXX`. Note that existing
`validateBranchProtection` function is used, and current behavior is
to ignore the unsupported attribute value, so no error is emitted.
---
clang/include/clang/Basic/TargetInfo.h | 1 +
clang/lib/Basic/Targets/AArch64.cpp | 8 ++++
clang/lib/Basic/Targets/AArch64.h | 1 +
clang/lib/Basic/Targets/ARM.cpp | 1 +
clang/lib/Basic/Targets/ARM.h | 1 +
clang/lib/CodeGen/Targets/AArch64.cpp | 4 +-
clang/lib/CodeGen/Targets/ARM.cpp | 4 +-
clang/lib/Driver/ToolChains/Clang.cpp | 46 ++++++++++---------
clang/lib/Sema/SemaDeclAttr.cpp | 3 +-
clang/test/Driver/aarch64-ptrauth.c | 27 +++++++----
...rch64-ignore-branch-protection-attribute.c | 28 +++++++++++
11 files changed, 88 insertions(+), 36 deletions(-)
create mode 100644 clang/test/Frontend/aarch64-ignore-branch-protection-attribute.c
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 43c09cf1f973e3..03f7b704110d20 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1468,6 +1468,7 @@ class TargetInfo : public TransferrableTargetInfo,
/// specification
virtual bool validateBranchProtection(StringRef Spec, StringRef Arch,
BranchProtectionInfo &BPI,
+ const LangOptions &LO,
StringRef &Err) const {
Err = "";
return false;
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 0b899137bbb5c7..176aeb7b426be5 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -253,11 +253,19 @@ bool AArch64TargetInfo::validateGlobalRegisterVariable(
bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
BranchProtectionInfo &BPI,
+ const LangOptions &LO,
StringRef &Err) const {
llvm::ARM::ParsedBranchProtection PBP;
if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err, HasPAuthLR))
return false;
+ // GCS is currently untested with ptrauth-returns, but enabling this could be
+ // allowed in future after testing with a suitable system.
+ if (LO.PointerAuthReturns &&
+ (PBP.Scope != "none" || PBP.BranchProtectionPAuthLR ||
+ PBP.GuardedControlStack))
+ return false;
+
BPI.SignReturnAddr =
llvm::StringSwitch<LangOptions::SignReturnAddressScopeKind>(PBP.Scope)
.Case("non-leaf", LangOptions::SignReturnAddressScopeKind::NonLeaf)
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 8695c0750ee32d..357205a153ae5c 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -132,6 +132,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
bool validateBranchProtection(StringRef Spec, StringRef Arch,
BranchProtectionInfo &BPI,
+ const LangOptions &LO,
StringRef &Err) const override;
bool isValidCPUName(StringRef Name) const override;
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 5aa2baeb81b731..29be9a6eb34890 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -405,6 +405,7 @@ bool ARMTargetInfo::isBranchProtectionSupportedArch(StringRef Arch) const {
bool ARMTargetInfo::validateBranchProtection(StringRef Spec, StringRef Arch,
BranchProtectionInfo &BPI,
+ const LangOptions &LO,
StringRef &Err) const {
llvm::ARM::ParsedBranchProtection PBP;
if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err))
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 5f4acce7af5a46..12fd7ed7fcc92e 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -155,6 +155,7 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
bool isBranchProtectionSupportedArch(StringRef Arch) const override;
bool validateBranchProtection(StringRef Spec, StringRef Arch,
BranchProtectionInfo &BPI,
+ const LangOptions &LO,
StringRef &Err) const override;
// FIXME: This should be based on Arch attributes, not CPU names.
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index e2e434815d43af..4922b082cf09ca 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -147,8 +147,8 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
CGM.getTarget().parseTargetAttr(TA->getFeaturesStr());
if (!Attr.BranchProtection.empty()) {
StringRef Error;
- (void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
- Attr.CPU, BPI, Error);
+ (void)CGM.getTarget().validateBranchProtection(
+ Attr.BranchProtection, Attr.CPU, BPI, CGM.getLangOpts(), Error);
assert(Error.empty());
}
}
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 2d858fa2f3c3a3..57458bb4f180b9 100644
--- a/clang/lib/CodeGen/Targets/ARM.cpp
+++ b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -148,8 +148,8 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
StringRef DiagMsg;
StringRef Arch =
Attr.CPU.empty() ? CGM.getTarget().getTargetOpts().CPU : Attr.CPU;
- if (!CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
- Arch, BPI, DiagMsg)) {
+ if (!CGM.getTarget().validateBranchProtection(
+ Attr.BranchProtection, Arch, BPI, CGM.getLangOpts(), DiagMsg)) {
CGM.getDiags().Report(
D->getLocation(),
diag::warn_target_unsupported_branch_protection_attribute)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 9b5132c5625faa..27de34634660c3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1618,32 +1618,34 @@ static void CollectARMPACBTIOptions(const ToolChain &TC, const ArgList &Args,
GuardedControlStack = PBP.GuardedControlStack;
}
- CmdArgs.push_back(
- Args.MakeArgString(Twine("-msign-return-address=") + Scope));
- if (Scope != "none") {
+ bool HasPtrauthReturns = llvm::any_of(CmdArgs, [](const char *Arg) {
+ return StringRef(Arg) == "-fptrauth-returns";
+ });
+ // GCS is currently untested with ptrauth-returns, but enabling this could be
+ // allowed in future after testing with a suitable system.
+ if (HasPtrauthReturns &&
+ (Scope != "none" || BranchProtectionPAuthLR || GuardedControlStack)) {
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << Triple.getTriple();
+ else
+ D.Diag(diag::err_drv_incompatible_options)
+ << A->getAsString(Args) << "-fptrauth-returns";
+ }
+
+ CmdArgs.push_back(
+ Args.MakeArgString(Twine("-msign-return-address=") + Scope));
+ if (Scope != "none")
CmdArgs.push_back(
Args.MakeArgString(Twine("-msign-return-address-key=") + Key));
- }
- if (BranchProtectionPAuthLR) {
- if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
- D.Diag(diag::err_drv_unsupported_opt_for_target)
- << A->getAsString(Args) << Triple.getTriple();
+ if (BranchProtectionPAuthLR)
CmdArgs.push_back(
Args.MakeArgString(Twine("-mbranch-protection-pauth-lr")));
- }
if (IndirectBranches)
CmdArgs.push_back("-mbranch-target-enforce");
- // GCS is currently untested with PAuthABI, but enabling this could be allowed
- // in future after testing with a suitable system.
- if (GuardedControlStack) {
- if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
- D.Diag(diag::err_drv_unsupported_opt_for_target)
- << A->getAsString(Args) << Triple.getTriple();
+
+ if (GuardedControlStack)
CmdArgs.push_back("-mguarded-control-stack");
- }
}
void Clang::AddARMTargetArgs(const llvm::Triple &Triple, const ArgList &Args,
@@ -1822,12 +1824,6 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
CmdArgs.push_back("-aarch64-enable-global-merge=true");
}
- // Enable/disable return address signing and indirect branch targets.
- CollectARMPACBTIOptions(getToolChain(), Args, CmdArgs, true /*isAArch64*/);
-
- if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
- handlePAuthABI(Args, CmdArgs);
-
// Handle -msve_vector_bits=<bits>
if (Arg *A = Args.getLastArg(options::OPT_msve_vector_bits_EQ)) {
StringRef Val = A->getValue();
@@ -1896,6 +1892,12 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
options::OPT_fno_ptrauth_init_fini_address_discrimination);
Args.addOptInFlag(CmdArgs, options::OPT_faarch64_jump_table_hardening,
options::OPT_fno_aarch64_jump_table_hardening);
+
+ if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
+ handlePAuthABI(Args, CmdArgs);
+
+ // Enable/disable return address signing and indirect branch targets.
+ CollectARMPACBTIOptions(getToolChain(), Args, CmdArgs, true /*isAArch64*/);
}
void Clang::AddLoongArchTargetArgs(const ArgList &Args,
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 9d7d22590bce4b..f351663c6824e3 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3073,7 +3073,8 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
if (ParsedAttrs.BranchProtection.empty())
return false;
if (!Context.getTargetInfo().validateBranchProtection(
- ParsedAttrs.BranchProtection, ParsedAttrs.CPU, BPI, DiagMsg)) {
+ ParsedAttrs.BranchProtection, ParsedAttrs.CPU, BPI,
+ Context.getLangOpts(), DiagMsg)) {
if (DiagMsg.empty())
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << "branch-protection" << Target;
diff --git a/clang/test/Driver/aarch64-ptrauth.c b/clang/test/Driver/aarch64-ptrauth.c
index d036189e614983..1d2993f4c60c4b 100644
--- a/clang/test/Driver/aarch64-ptrauth.c
+++ b/clang/test/Driver/aarch64-ptrauth.c
@@ -64,22 +64,31 @@
//// 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=ERR4_1
// 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=ERR4_1
+// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=pac-ret %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ERR4_2
+// ERR4_1: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
+// ERR4_2: error: the combination of '-mbranch-protection=pac-ret' and '-fptrauth-returns' is incompatible
// 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=ERR5_1
// 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=ERR5_1
+// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=gcs %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ERR5_2
+// ERR5_1: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
+// ERR5_2: error: the combination of '-mbranch-protection=gcs' and '-fptrauth-returns' is incompatible
// 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=ERR6_1
// 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=ERR6_1
+// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=standard %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ERR6_2
+// ERR6_1: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
+// ERR6_2: error: the combination of '-mbranch-protection=standard' and '-fptrauth-returns' is incompatible
// RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=all %s 2>&1 | \
// RUN: FileCheck %s --check-prefix=ERR7
diff --git a/clang/test/Frontend/aarch64-ignore-branch-protection-attribute.c b/clang/test/Frontend/aarch64-ignore-branch-protection-attribute.c
new file mode 100644
index 00000000000000..234c530d42862c
--- /dev/null
+++ b/clang/test/Frontend/aarch64-ignore-branch-protection-attribute.c
@@ -0,0 +1,28 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang -target aarch64-linux-pauthtest %s -S -emit-llvm -o - 2>&1 | FileCheck --implicit-check-not=warning: %s
+// RUN: %clang -target aarch64 -fptrauth-returns %s -S -emit-llvm -o - 2>&1 | FileCheck --implicit-check-not=warning: %s
+
+/// Unsupported with pauthtest, warning emitted
+__attribute__((target("branch-protection=pac-ret"))) void f1() {}
+// CHECK: warning: unsupported 'branch-protection' in the 'target' attribute string; 'target' attribute ignored [-Wignored-attributes]
+// CHECK-NEXT: __attribute__((target("branch-protection=pac-ret"))) void f1() {}
+__attribute__((target("branch-protection=gcs"))) void f2() {}
+// CHECK: warning: unsupported 'branch-protection' in the 'target' attribute string; 'target' attribute ignored [-Wignored-attributes]
+// CHECK-NEXT: __attribute__((target("branch-protection=gcs"))) void f2() {}
+__attribute__((target("branch-protection=standard"))) void f3() {}
+// CHECK: warning: unsupported 'branch-protection' in the 'target' attribute string; 'target' attribute ignored [-Wignored-attributes]
+// CHECK-NEXT: __attribute__((target("branch-protection=standard"))) void f3() {}
+
+/// Supported with pauthtest, no warning emitted
+__attribute__((target("branch-protection=bti"))) void f4() {}
+
+/// Check there are no branch protection function attributes which are unsupported with pauthtest
+
+// CHECK-NOT: attributes {{.*}} "sign-return-address"
+// CHECK-NOT: attributes {{.*}} "sign-return-address-key"
+// CHECK-NOT: attributes {{.*}} "branch-protection-pauth-lr"
+// CHECK-NOT: attributes {{.*}} "guarded-control-stack"
+
+/// Check function attributes which are supported with pauthtest
+// CHECK: attributes {{.*}} "branch-target-enforcement"
More information about the cfe-commits
mailing list