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 <dkova...@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" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits