Skip to content

Commit 84b0c12

Browse files
kovdan01kovdan01
authored
[PAC] Do not support some values of branch-protection with ptrauth-returns (#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.
1 parent ad38c4c commit 84b0c12

File tree

11 files changed

+91
-36
lines changed

11 files changed

+91
-36
lines changed

‎clang/include/clang/Basic/TargetInfo.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,7 @@ class TargetInfo : public TransferrableTargetInfo,
14711471
/// specification
14721472
virtual bool validateBranchProtection(StringRef Spec, StringRef Arch,
14731473
BranchProtectionInfo &BPI,
1474+
const LangOptions &LO,
14741475
StringRef &Err) const {
14751476
Err = "";
14761477
return false;

‎clang/lib/Basic/Targets/AArch64.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,11 +323,19 @@ bool AArch64TargetInfo::validateGlobalRegisterVariable(
323323

324324
bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
325325
BranchProtectionInfo &BPI,
326+
const LangOptions &LO,
326327
StringRef &Err) const {
327328
llvm::ARM::ParsedBranchProtection PBP;
328329
if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err, HasPAuthLR))
329330
return false;
330331

332+
// GCS is currently untested with ptrauth-returns, but enabling this could be
333+
// allowed in future after testing with a suitable system.
334+
if (LO.PointerAuthReturns &&
335+
(PBP.Scope != "none" || PBP.BranchProtectionPAuthLR ||
336+
PBP.GuardedControlStack))
337+
return false;
338+
331339
BPI.SignReturnAddr =
332340
llvm::StringSwitch<LangOptions::SignReturnAddressScopeKind>(PBP.Scope)
333341
.Case("non-leaf", LangOptions::SignReturnAddressScopeKind::NonLeaf)

‎clang/lib/Basic/Targets/AArch64.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
132132

133133
bool validateBranchProtection(StringRef Spec, StringRef Arch,
134134
BranchProtectionInfo &BPI,
135+
const LangOptions &LO,
135136
StringRef &Err) const override;
136137

137138
bool isValidCPUName(StringRef Name) const override;

‎clang/lib/Basic/Targets/ARM.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ bool ARMTargetInfo::isBranchProtectionSupportedArch(StringRef Arch) const {
405405

406406
bool ARMTargetInfo::validateBranchProtection(StringRef Spec, StringRef Arch,
407407
BranchProtectionInfo &BPI,
408+
const LangOptions &LO,
408409
StringRef &Err) const {
409410
llvm::ARM::ParsedBranchProtection PBP;
410411
if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err))

‎clang/lib/Basic/Targets/ARM.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
155155
bool isBranchProtectionSupportedArch(StringRef Arch) const override;
156156
bool validateBranchProtection(StringRef Spec, StringRef Arch,
157157
BranchProtectionInfo &BPI,
158+
const LangOptions &LO,
158159
StringRef &Err) const override;
159160

160161
// FIXME: This should be based on Arch attributes, not CPU names.

‎clang/lib/CodeGen/Targets/AArch64.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
147147
CGM.getTarget().parseTargetAttr(TA->getFeaturesStr());
148148
if (!Attr.BranchProtection.empty()) {
149149
StringRef Error;
150-
(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
151-
Attr.CPU, BPI, Error);
150+
(void)CGM.getTarget().validateBranchProtection(
151+
Attr.BranchProtection, Attr.CPU, BPI, CGM.getLangOpts(), Error);
152152
assert(Error.empty());
153153
}
154154
}

‎clang/lib/CodeGen/Targets/ARM.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
149149
StringRef DiagMsg;
150150
StringRef Arch =
151151
Attr.CPU.empty() ? CGM.getTarget().getTargetOpts().CPU : Attr.CPU;
152-
if (!CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
153-
Arch, BPI, DiagMsg)) {
152+
if (!CGM.getTarget().validateBranchProtection(
153+
Attr.BranchProtection, Arch, BPI, CGM.getLangOpts(), DiagMsg)) {
154154
CGM.getDiags().Report(
155155
D->getLocation(),
156156
diag::warn_target_unsupported_branch_protection_attribute)

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,32 +1618,34 @@ static void CollectARMPACBTIOptions(const ToolChain &TC, const ArgList &Args,
16181618
GuardedControlStack = PBP.GuardedControlStack;
16191619
}
16201620

1621-
CmdArgs.push_back(
1622-
Args.MakeArgString(Twine("-msign-return-address=") + Scope));
1623-
if (Scope != "none") {
1621+
bool HasPtrauthReturns = llvm::any_of(CmdArgs, [](const char *Arg) {
1622+
return StringRef(Arg) == "-fptrauth-returns";
1623+
});
1624+
// GCS is currently untested with ptrauth-returns, but enabling this could be
1625+
// allowed in future after testing with a suitable system.
1626+
if (HasPtrauthReturns &&
1627+
(Scope != "none" || BranchProtectionPAuthLR || GuardedControlStack)) {
16241628
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
16251629
D.Diag(diag::err_drv_unsupported_opt_for_target)
16261630
<< A->getAsString(Args) << Triple.getTriple();
1631+
else
1632+
D.Diag(diag::err_drv_incompatible_options)
1633+
<< A->getAsString(Args) << "-fptrauth-returns";
1634+
}
1635+
1636+
CmdArgs.push_back(
1637+
Args.MakeArgString(Twine("-msign-return-address=") + Scope));
1638+
if (Scope != "none")
16271639
CmdArgs.push_back(
16281640
Args.MakeArgString(Twine("-msign-return-address-key=") + Key));
1629-
}
1630-
if (BranchProtectionPAuthLR) {
1631-
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
1632-
D.Diag(diag::err_drv_unsupported_opt_for_target)
1633-
<< A->getAsString(Args) << Triple.getTriple();
1641+
if (BranchProtectionPAuthLR)
16341642
CmdArgs.push_back(
16351643
Args.MakeArgString(Twine("-mbranch-protection-pauth-lr")));
1636-
}
16371644
if (IndirectBranches)
16381645
CmdArgs.push_back("-mbranch-target-enforce");
1639-
// GCS is currently untested with PAuthABI, but enabling this could be allowed
1640-
// in future after testing with a suitable system.
1641-
if (GuardedControlStack) {
1642-
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
1643-
D.Diag(diag::err_drv_unsupported_opt_for_target)
1644-
<< A->getAsString(Args) << Triple.getTriple();
1646+
1647+
if (GuardedControlStack)
16451648
CmdArgs.push_back("-mguarded-control-stack");
1646-
}
16471649
}
16481650

16491651
void Clang::AddARMTargetArgs(const llvm::Triple &Triple, const ArgList &Args,
@@ -1822,12 +1824,6 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
18221824
CmdArgs.push_back("-aarch64-enable-global-merge=true");
18231825
}
18241826

1825-
// Enable/disable return address signing and indirect branch targets.
1826-
CollectARMPACBTIOptions(getToolChain(), Args, CmdArgs, true /*isAArch64*/);
1827-
1828-
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
1829-
handlePAuthABI(Args, CmdArgs);
1830-
18311827
// Handle -msve_vector_bits=<bits>
18321828
if (Arg *A = Args.getLastArg(options::OPT_msve_vector_bits_EQ)) {
18331829
StringRef Val = A->getValue();
@@ -1896,6 +1892,12 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
18961892
options::OPT_fno_ptrauth_init_fini_address_discrimination);
18971893
Args.addOptInFlag(CmdArgs, options::OPT_faarch64_jump_table_hardening,
18981894
options::OPT_fno_aarch64_jump_table_hardening);
1895+
1896+
if (Triple.getEnvironment() == llvm::Triple::PAuthTest)
1897+
handlePAuthABI(Args, CmdArgs);
1898+
1899+
// Enable/disable return address signing and indirect branch targets.
1900+
CollectARMPACBTIOptions(getToolChain(), Args, CmdArgs, true /*isAArch64*/);
18991901
}
19001902

19011903
void Clang::AddLoongArchTargetArgs(const ArgList &Args,

‎clang/lib/Sema/SemaDeclAttr.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3073,7 +3073,8 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
30733073
if (ParsedAttrs.BranchProtection.empty())
30743074
return false;
30753075
if (!Context.getTargetInfo().validateBranchProtection(
3076-
ParsedAttrs.BranchProtection, ParsedAttrs.CPU, BPI, DiagMsg)) {
3076+
ParsedAttrs.BranchProtection, ParsedAttrs.CPU, BPI,
3077+
Context.getLangOpts(), DiagMsg)) {
30773078
if (DiagMsg.empty())
30783079
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
30793080
<< Unsupported << None << "branch-protection" << Target;

‎clang/test/Driver/aarch64-ptrauth.c‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,31 @@
6464

6565
//// The only branch protection option compatible with PAuthABI is BTI.
6666
// RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=pac-ret %s 2>&1 | \
67-
// RUN: FileCheck %s --check-prefix=ERR4
67+
// RUN: FileCheck %s --check-prefix=ERR4_1
6868
// RUN: not %clang -### -c --target=aarch64-linux-pauthtest -mbranch-protection=pac-ret %s 2>&1 | \
69-
// RUN: FileCheck %s --check-prefix=ERR4
70-
// ERR4: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
69+
// RUN: FileCheck %s --check-prefix=ERR4_1
70+
// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=pac-ret %s 2>&1 | \
71+
// RUN: FileCheck %s --check-prefix=ERR4_2
72+
// ERR4_1: error: unsupported option '-mbranch-protection=pac-ret' for target 'aarch64-unknown-linux-pauthtest'
73+
// ERR4_2: error: the combination of '-mbranch-protection=pac-ret' and '-fptrauth-returns' is incompatible
7174

7275
// RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=gcs %s 2>&1 | \
73-
// RUN: FileCheck %s --check-prefix=ERR5
76+
// RUN: FileCheck %s --check-prefix=ERR5_1
7477
// RUN: not %clang -### -c --target=aarch64-linux-pauthtest -mbranch-protection=gcs %s 2>&1 | \
75-
// RUN: FileCheck %s --check-prefix=ERR5
76-
// ERR5: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
78+
// RUN: FileCheck %s --check-prefix=ERR5_1
79+
// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=gcs %s 2>&1 | \
80+
// RUN: FileCheck %s --check-prefix=ERR5_2
81+
// ERR5_1: error: unsupported option '-mbranch-protection=gcs' for target 'aarch64-unknown-linux-pauthtest'
82+
// ERR5_2: error: the combination of '-mbranch-protection=gcs' and '-fptrauth-returns' is incompatible
7783

7884
// RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -mbranch-protection=standard %s 2>&1 | \
79-
// RUN: FileCheck %s --check-prefix=ERR6
85+
// RUN: FileCheck %s --check-prefix=ERR6_1
8086
// RUN: not %clang -### -c --target=aarch64-linux-pauthtest -mbranch-protection=standard %s 2>&1 | \
81-
// RUN: FileCheck %s --check-prefix=ERR6
82-
// ERR6: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
87+
// RUN: FileCheck %s --check-prefix=ERR6_1
88+
// RUN: not %clang -### -c --target=aarch64 -fptrauth-returns -mbranch-protection=standard %s 2>&1 | \
89+
// RUN: FileCheck %s --check-prefix=ERR6_2
90+
// ERR6_1: error: unsupported option '-mbranch-protection=standard' for target 'aarch64-unknown-linux-pauthtest'
91+
// ERR6_2: error: the combination of '-mbranch-protection=standard' and '-fptrauth-returns' is incompatible
8392

8493
// RUN: not %clang -### -c --target=aarch64-linux -mabi=pauthtest -msign-return-address=all %s 2>&1 | \
8594
// RUN: FileCheck %s --check-prefix=ERR7

0 commit comments

Comments
 (0)