[PAC] Do not support some values of branch-protection with ptrauth-returns #125280
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kovdan01
requested review from
asl,
MaskRay,
DanielKristofKiss,
labrinea and
smithp35
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-arm Author: Daniil Kovalev (kovdan01) ChangesThis patch does two things.
Full diff: https://github.com/llvm/llvm-project/pull/125280.diff 11 Files Affected:
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"
|
DanielKristofKiss
approved these changes
Feb 3, 2025
| // CHECK-NEXT: __attribute__((target("branch-protection=standard"))) void f3() {} | ||
|
|
||
| /// Supported with pauthtest, no warning emitted | ||
| __attribute__((target("branch-protection=bti"))) void f4() {} |
There was a problem hiding this comment.
I'd add a test case of the "branch-protection=none".
There was a problem hiding this comment.
Added, thanks, see 49e8091
…turns
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.
kovdan01
force-pushed
the
unsupport-branch-protection-ptrauth-returns
branch
from
cf5d218 to
49e8091
Compare
|
/cherry-pick 84b0c12 |
|
/pull-request #126589 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 11, 2025
…turns (llvm#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. (cherry picked from commit 84b0c12)
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…turns (llvm#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch does two things.
Previously, when checking driver arguments, we emitted an error for unsupported values of
-mbranch-protectionwhen 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.Similarly, check against values of the following function attribute which are unsupported with ptrauth-returns:
__attribute__((target("branch-protection=XXX. Note that existingvalidateBranchProtectionfunction is used, and current behavior is to ignore the unsupported attribute value, so no error is emitted.