[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/nickdesaulniers commented: This still has "foo"="true" style function attributes, which are problematic. Is the plan to change that? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -116,37 +116,22 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { if (!FD) return; -const auto *TA = FD->getAttr(); -if (TA == nullptr) - return; - -ParsedTargetAttr Attr = -CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); -if (Attr.BranchProtection.empty()) - return; - -TargetInfo::BranchProtectionInfo BPI; -StringRef Error; -(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, - Attr.CPU, BPI, Error); -assert(Error.empty()); - -auto *Fn = cast(GV); -Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); - -if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { - Fn->addFnAttr("sign-return-address-key", -BPI.SignKey == LangOptions::SignReturnAddressKeyKind::AKey -? "a_key" -: "b_key"); +TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + +if (const auto *TA = FD->getAttr()) { + ParsedTargetAttr Attr = + CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); + if (!Attr.BranchProtection.empty()) { +StringRef Error; +(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, + Attr.CPU, BPI, Error); +#ifndef NDEBUG +assert(Error.empty()); +#endif nickdesaulniers wrote: No need for the preprocessor guard here, as `assert` only does anything if NDEBUG is not defined. [My earlier suggestion](https://github.com/llvm/llvm-project/pull/82819#discussion_r1508064741) was to put it around this if statement, but I think that's a bad suggestion now since we want validateBranchProtection to run regardless of NDEBUG. https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -43,4 +49,4 @@ // BTE-NOT: !"sign-return-address-with-bkey" // B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1} -void foo() {} +void foo() {} nickdesaulniers wrote: retain newline at EOF https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1398,6 +1400,42 @@ class TargetInfo : public TransferrableTargetInfo, } llvm_unreachable("Unexpected SignReturnAddressKeyKind"); } + + public: +BranchProtectionInfo() = default; nickdesaulniers wrote: Do we still need the default constructor, or no? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1398,6 +1400,42 @@ class TargetInfo : public TransferrableTargetInfo, } llvm_unreachable("Unexpected SignReturnAddressKeyKind"); } + + public: +BranchProtectionInfo() = default; DanielKristofKiss wrote: We need the default constructor: e.g https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L3481 https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/Targets/ARM.cpp#L144 https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
DanielKristofKiss wrote: > This still has "foo"="true" style function attributes, which are problematic. > Is the plan to change that? Can be changed to just "foo" just makes the function/module attribute importer more complicated to handle the old IR. https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/DanielKristofKiss created https://github.com/llvm/llvm-project/pull/82819 Branch protection, sign return address, guarded control stack attributes are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't consider the module flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off for all functions due to the functions take the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected. On ARM target only when the target supports these protections. On AArch64 always added as the instructions are always available. >From f1de474eb318c06272a2ac8008af5de6762b7617 Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Fri, 23 Feb 2024 17:12:26 +0100 Subject: [PATCH 1/2] [NFC][ARM][AArch64] Deduplicated code. Add the SignReturnAddressScopeKind to the BranchProtectionInfo class. --- clang/include/clang/Basic/TargetInfo.h | 21 ++--- clang/lib/CodeGen/Targets/AArch64.cpp | 3 +-- clang/lib/CodeGen/Targets/ARM.cpp | 8 +--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..2eb4f0e2ca42a6 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,20 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0af87b00b91f4 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -125,8 +125,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { assert(Error.empty()); auto *Fn = cast(GV); -static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; -Fn->addFnAttr("sign-return-address", SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { Fn->addFnAttr("sign-return-address-key", diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index d7d175ff1724f7..5d42e6286e525b 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -152,13 +152,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Arch; } else { - static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; - assert(static_cast(BPI.SignReturnAddr) <= 2 && - "Unexpected SignReturnAddressScopeKind"); - Fn->addFnAttr( - "sign-return-address", - SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); - + Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); Fn->addFnAttr("branch-target-enforcement", BPI.BranchTargetEnforcement ? "true" : "false"); } >From 85c2be8767729561f7be66e06661b225877b096f Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Mon, 22 Jan 2024 11:33:15 +0100 Subject: [PATCH 2/2] [Clang][ARM][AArch64] Emit attributes for functions always. Branch protection, sign return address, guarded control stack flags are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't see the flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI the
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
llvmbot wrote: @llvm/pr-subscribers-backend-arm @llvm/pr-subscribers-clang Author: Dani (DanielKristofKiss) Changes Branch protection, sign return address, guarded control stack attributes are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't consider the module flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off for all functions due to the functions take the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected. On ARM target only when the target supports these protections. On AArch64 always added as the instructions are always available. --- Patch is 52.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82819.diff 12 Files Affected: - (modified) clang/include/clang/Basic/TargetInfo.h (+29-7) - (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+13-17) - (modified) clang/lib/CodeGen/Targets/ARM.cpp (+8-7) - (modified) clang/test/CodeGen/aarch64-cpu-supports-target.c (+3-3) - (modified) clang/test/CodeGen/aarch64-sign-return-address.c (+22-4) - (modified) clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp (+12-12) - (modified) clang/test/CodeGen/arm-branch-protection-attr-2.c (+2-4) - (modified) clang/test/CodeGen/attr-target-clones-aarch64.c (+16-16) - (modified) clang/test/CodeGen/attr-target-version.c (+29-29) - (modified) clang/test/CodeGenCXX/attr-target-clones-aarch64.cpp (+56-10) - (modified) clang/test/CodeGenCXX/attr-target-version.cpp (+5-5) - (added) clang/test/Frontend/arm-branch-protection-lto.c (+16) ``diff diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..5eaf34ea581f32 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,35 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +BranchProtectionInfo(const LangOptions &LangOpts) { + SignReturnAddr = + LangOpts.hasSignReturnAddress() + ? (LangOpts.isSignReturnAddressScopeAll() + ? LangOptions::SignReturnAddressScopeKind::All + : LangOptions::SignReturnAddressScopeKind::NonLeaf) + : LangOptions::SignReturnAddressScopeKind::None; + SignKey = LangOpts.isSignReturnAddressWithAKey() +? LangOptions::SignReturnAddressKeyKind::AKey +: LangOptions::SignReturnAddressKeyKind::BKey; + BranchTargetEnforcement = LangOpts.BranchTargetEnforcement; + BranchProtectionPAuthLR = LangOpts.BranchProtectionPAuthLR; + GuardedControlStack = LangOpts.GuardedControlStack; +} + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0c3513a0cece0 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -109,24 +109,20 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { if (!FD) return; -const auto *TA = FD->getAttr(); -if (TA == nullptr) - return; - -ParsedTargetAttr Attr = -CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); -if (Attr.BranchProtection.empty()) - return; - -TargetInfo::BranchProtectionInfo BPI; -StringRef Error; -(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, - Attr.CPU, BPI, Error); -assert(Error.empty()); - +TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + +if (const auto *TA = FD->getAttr()) { + ParsedTargetAttr Attr =
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: Dani (DanielKristofKiss) Changes Branch protection, sign return address, guarded control stack attributes are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't consider the module flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off for all functions due to the functions take the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected. On ARM target only when the target supports these protections. On AArch64 always added as the instructions are always available. --- Patch is 52.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82819.diff 12 Files Affected: - (modified) clang/include/clang/Basic/TargetInfo.h (+29-7) - (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+13-17) - (modified) clang/lib/CodeGen/Targets/ARM.cpp (+8-7) - (modified) clang/test/CodeGen/aarch64-cpu-supports-target.c (+3-3) - (modified) clang/test/CodeGen/aarch64-sign-return-address.c (+22-4) - (modified) clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp (+12-12) - (modified) clang/test/CodeGen/arm-branch-protection-attr-2.c (+2-4) - (modified) clang/test/CodeGen/attr-target-clones-aarch64.c (+16-16) - (modified) clang/test/CodeGen/attr-target-version.c (+29-29) - (modified) clang/test/CodeGenCXX/attr-target-clones-aarch64.cpp (+56-10) - (modified) clang/test/CodeGenCXX/attr-target-version.cpp (+5-5) - (added) clang/test/Frontend/arm-branch-protection-lto.c (+16) ``diff diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..5eaf34ea581f32 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,35 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +BranchProtectionInfo(const LangOptions &LangOpts) { + SignReturnAddr = + LangOpts.hasSignReturnAddress() + ? (LangOpts.isSignReturnAddressScopeAll() + ? LangOptions::SignReturnAddressScopeKind::All + : LangOptions::SignReturnAddressScopeKind::NonLeaf) + : LangOptions::SignReturnAddressScopeKind::None; + SignKey = LangOpts.isSignReturnAddressWithAKey() +? LangOptions::SignReturnAddressKeyKind::AKey +: LangOptions::SignReturnAddressKeyKind::BKey; + BranchTargetEnforcement = LangOpts.BranchTargetEnforcement; + BranchProtectionPAuthLR = LangOpts.BranchProtectionPAuthLR; + GuardedControlStack = LangOpts.GuardedControlStack; +} + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0c3513a0cece0 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -109,24 +109,20 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { if (!FD) return; -const auto *TA = FD->getAttr(); -if (TA == nullptr) - return; - -ParsedTargetAttr Attr = -CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); -if (Attr.BranchProtection.empty()) - return; - -TargetInfo::BranchProtectionInfo BPI; -StringRef Error; -(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, - Attr.CPU, BPI, Error); -assert(Error.empty()); - +TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + +if (const auto *TA = FD->getAttr()) { + ParsedTargetAttr Attr = + CGM.getTarget
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
DanielKristofKiss wrote: RFC as I'm not sure the right approach is to emit that many attribute always. LLVM's Function::createWithDefaultAttr to be extended too with propagation of the module flags to the function. https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -0,0 +1,16 @@ +// REQUIRES: arm-registered-target + +// RUN: %clang -flto -target thumbv7m-unknown-unknown-eabi -mbranch-protection=pac-ret %s -S -o - 2>&1 | FileCheck %s MaskRay wrote: prefer `--target=` for new tests For codegen tests we prefer `%clang_cc1` to `%clang` https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -138,58 +163,79 @@ void run_foo_tml() { // CHECK: resolver_return1: // CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv._Mfrintts // CHECK: resolver_else2: -// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv +// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv.default +// +// // CHECK-LABEL: @_ZN7MyClassIfsE7foo_tmlEv( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 3 +// +// // CHECK-LABEL: @_ZN7MyClassIdfE7foo_tmlEv( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 4 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv._Mfrintts( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv._MssbsMsme-f64f64( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv.default( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv._Mfrintts( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv._MssbsMsme-f64f64( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv.default( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 - -// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon" } -// CHECK: attributes #1 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" } -// CHECK: attributes #2 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+ls64" } -// CHECK: attributes #3 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fptoint" } -// CHECK: attributes #4 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+bf16,+sme,+sme-f64f64" } +// +//. +// CHECK: attributes #[[ATTR0:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon" } +// CHECK: attributes #[[ATTR1:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" } +// CHECK: attributes #[[ATTR2:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+ls64" } +// CHECK: attributes #[[ATTR3:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+fptoint" } +// CHECK: attributes #[[ATTR4:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -138,58 +163,79 @@ void run_foo_tml() { // CHECK: resolver_return1: // CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv._Mfrintts // CHECK: resolver_else2: -// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv +// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv.default +// +// // CHECK-LABEL: @_ZN7MyClassIfsE7foo_tmlEv( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 3 +// MaskRay wrote: Unneeded changes? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/DanielKristofKiss updated https://github.com/llvm/llvm-project/pull/82819 >From c5c2d720e822624fa7966297087b04e6b2fc2a86 Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Fri, 23 Feb 2024 17:12:26 +0100 Subject: [PATCH 1/3] [NFC][ARM][AArch64] Deduplicated code. Add the SignReturnAddressScopeKind to the BranchProtectionInfo class. --- clang/include/clang/Basic/TargetInfo.h | 21 ++--- clang/lib/CodeGen/Targets/AArch64.cpp | 3 +-- clang/lib/CodeGen/Targets/ARM.cpp | 8 +--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..2eb4f0e2ca42a6 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,20 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0af87b00b91f4 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -125,8 +125,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { assert(Error.empty()); auto *Fn = cast(GV); -static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; -Fn->addFnAttr("sign-return-address", SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { Fn->addFnAttr("sign-return-address-key", diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index d7d175ff1724f7..5d42e6286e525b 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -152,13 +152,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Arch; } else { - static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; - assert(static_cast(BPI.SignReturnAddr) <= 2 && - "Unexpected SignReturnAddressScopeKind"); - Fn->addFnAttr( - "sign-return-address", - SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); - + Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); Fn->addFnAttr("branch-target-enforcement", BPI.BranchTargetEnforcement ? "true" : "false"); } >From c82cb029dc73130f46df79cf6c1228f690ef71af Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Mon, 22 Jan 2024 11:33:15 +0100 Subject: [PATCH 2/3] [Clang][ARM][AArch64] Emit attributes for functions always. Branch protection, sign return address, guarded control stack flags are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't see the flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off on all functions due to the functions takes the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected but in LTO case this might not happen. This patch adds the flags to functions in case of an LTO build therefore they don't need to rely on the module flag. --- clang/include/clang/Basic/TargetInfo.h| 15 + clang/lib/CodeGen/Targets/AArch64.cpp | 27 clang/lib/CodeGen/Targets/ARM.cpp | 7 ++ .../CodeGen/aarch64-cpu-supports-target.c | 6 +- .../CodeGen/aarch64-sign-return-address.c | 26 ++-- .../aarch64-sme-attrs.cpp | 24 +++ .../
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -0,0 +1,16 @@ +// REQUIRES: arm-registered-target + +// RUN: %clang -flto -target thumbv7m-unknown-unknown-eabi -mbranch-protection=pac-ret %s -S -o - 2>&1 | FileCheck %s DanielKristofKiss wrote: Done https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -138,58 +163,79 @@ void run_foo_tml() { // CHECK: resolver_return1: // CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv._Mfrintts // CHECK: resolver_else2: -// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv +// CHECK-NEXT:ret ptr @_ZN7MyClassIisE7foo_tmlEv.default +// +// // CHECK-LABEL: @_ZN7MyClassIfsE7foo_tmlEv( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 3 +// +// // CHECK-LABEL: @_ZN7MyClassIdfE7foo_tmlEv( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 4 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv._Mfrintts( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv._MssbsMsme-f64f64( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIssE7foo_tmlEv.default( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 1 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv._Mfrintts( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv._MssbsMsme-f64f64( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 +// +// // CHECK-LABEL: @_ZN7MyClassIisE7foo_tmlEv.default( // CHECK-NEXT: entry: // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK-NEXT:store ptr [[THIS:%.*]], ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:[[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK-NEXT:ret i32 2 - -// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon" } -// CHECK: attributes #1 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" } -// CHECK: attributes #2 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+ls64" } -// CHECK: attributes #3 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fptoint" } -// CHECK: attributes #4 = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+bf16,+sme,+sme-f64f64" } +// +//. +// CHECK: attributes #[[ATTR0:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon" } +// CHECK: attributes #[[ATTR1:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" } +// CHECK: attributes #[[ATTR2:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+ls64" } +// CHECK: attributes #[[ATTR3:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="false" "branch-target-enforcement"="false" "guarded-control-stack"="false" "no-trapping-math"="true" "sign-return-address"="none" "stack-protector-buffer-size"="8" "target-features"="+fptoint" } +// CHECK: attributes #[[ATTR4:[0-9]+]] = { mustprogress noinline nounwind optnone "branch-protection-pauth-lr"="
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/DanielKristofKiss updated https://github.com/llvm/llvm-project/pull/82819 >From c5c2d720e822624fa7966297087b04e6b2fc2a86 Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Fri, 23 Feb 2024 17:12:26 +0100 Subject: [PATCH 1/2] [NFC][ARM][AArch64] Deduplicated code. Add the SignReturnAddressScopeKind to the BranchProtectionInfo class. --- clang/include/clang/Basic/TargetInfo.h | 21 ++--- clang/lib/CodeGen/Targets/AArch64.cpp | 3 +-- clang/lib/CodeGen/Targets/ARM.cpp | 8 +--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..2eb4f0e2ca42a6 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,20 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0af87b00b91f4 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -125,8 +125,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { assert(Error.empty()); auto *Fn = cast(GV); -static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; -Fn->addFnAttr("sign-return-address", SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { Fn->addFnAttr("sign-return-address-key", diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index d7d175ff1724f7..5d42e6286e525b 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -152,13 +152,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Arch; } else { - static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; - assert(static_cast(BPI.SignReturnAddr) <= 2 && - "Unexpected SignReturnAddressScopeKind"); - Fn->addFnAttr( - "sign-return-address", - SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); - + Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); Fn->addFnAttr("branch-target-enforcement", BPI.BranchTargetEnforcement ? "true" : "false"); } >From eef710df41ca59f3eee47f271b234a311e0fe11d Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Mon, 22 Jan 2024 11:33:15 +0100 Subject: [PATCH 2/2] Emit attributes for functions always. Branch protection, sign return address, guarded control stack flags are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't see the flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off on all functions due to the functions takes the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected but in LTO case this might not happen. This patch adds the flags to functions in case of an LTO build therefore they don't need to rely on the module flag. --- clang/include/clang/Basic/TargetInfo.h| 15 +++ clang/lib/CodeGen/Targets/AArch64.cpp | 43 --- clang/lib/CodeGen/Targets/ARM.cpp | 7 +++ .../CodeGen/aarch64-branch-protection-attr.c | 24 +-- .../CodeGen/aarch64-sign-return-address.c | 14 -- .../CodeGen/arm-branch-protection-attr-2.c| 13 -- .../tes
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
DanielKristofKiss wrote: Function attributes are only attached when they are set. #83153 ensures the synthetic function are also gets the right attributes so the backend doesn't need to use the module attributes #83154. https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
DanielKristofKiss wrote: This handles the synthetic function in clang: #83277 https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/DanielKristofKiss updated https://github.com/llvm/llvm-project/pull/82819 >From c5c2d720e822624fa7966297087b04e6b2fc2a86 Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Fri, 23 Feb 2024 17:12:26 +0100 Subject: [PATCH 1/4] [NFC][ARM][AArch64] Deduplicated code. Add the SignReturnAddressScopeKind to the BranchProtectionInfo class. --- clang/include/clang/Basic/TargetInfo.h | 21 ++--- clang/lib/CodeGen/Targets/AArch64.cpp | 3 +-- clang/lib/CodeGen/Targets/ARM.cpp | 8 +--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 48e9cec482755c..2eb4f0e2ca42a6 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1369,13 +1369,20 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; + +const char *getSignReturnAddrStr() const { + static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; + assert(static_cast(SignReturnAddr) <= 2 && + "Unexpected SignReturnAddressScopeKind"); + return SignReturnAddrStr[static_cast(SignReturnAddr)]; +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 94f8e7be2ee6eb..f0af87b00b91f4 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -125,8 +125,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { assert(Error.empty()); auto *Fn = cast(GV); -static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; -Fn->addFnAttr("sign-return-address", SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { Fn->addFnAttr("sign-return-address-key", diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index d7d175ff1724f7..5d42e6286e525b 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -152,13 +152,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Arch; } else { - static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; - assert(static_cast(BPI.SignReturnAddr) <= 2 && - "Unexpected SignReturnAddressScopeKind"); - Fn->addFnAttr( - "sign-return-address", - SignReturnAddrStr[static_cast(BPI.SignReturnAddr)]); - + Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); Fn->addFnAttr("branch-target-enforcement", BPI.BranchTargetEnforcement ? "true" : "false"); } >From eef710df41ca59f3eee47f271b234a311e0fe11d Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Mon, 22 Jan 2024 11:33:15 +0100 Subject: [PATCH 2/4] Emit attributes for functions always. Branch protection, sign return address, guarded control stack flags are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't see the flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off on all functions due to the functions takes the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected but in LTO case this might not happen. This patch adds the flags to functions in case of an LTO build therefore they don't need to rely on the module flag. --- clang/include/clang/Basic/TargetInfo.h| 15 +++ clang/lib/CodeGen/Targets/AArch64.cpp | 43 --- clang/lib/CodeGen/Targets/ARM.cpp | 7 +++ .../CodeGen/aarch64-branch-protection-attr.c | 24 +-- .../CodeGen/aarch64-sign-return-address.c | 14 -- .../CodeGen/arm-branch-protection-attr-2.c| 13 -- .../tes
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1369,13 +1369,50 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; nickdesaulniers wrote: Is this constructor used anywhere? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1369,13 +1369,50 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; nickdesaulniers wrote: Consider if it's feasible making this a class and having the constructors and methods be public. https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -109,38 +109,29 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { if (!FD) return; -const auto *TA = FD->getAttr(); -if (TA == nullptr) - return; - -ParsedTargetAttr Attr = -CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); -if (Attr.BranchProtection.empty()) - return; - -TargetInfo::BranchProtectionInfo BPI; -StringRef Error; -(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, - Attr.CPU, BPI, Error); -assert(Error.empty()); - +TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + +if (const auto *TA = FD->getAttr()) { + ParsedTargetAttr Attr = + CGM.getTarget().parseTargetAttr(TA->getFeaturesStr()); + if (!Attr.BranchProtection.empty()) { +StringRef Error; +(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection, + Attr.CPU, BPI, Error); +assert(Error.empty()); + } +} nickdesaulniers wrote: Should this all be in a `#ifndef NDEBUG` block? Seems like the return value and contents of error are not used otherwise? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -173,6 +167,13 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Attr.CPU; } +} else if (CGM.getTarget().isBranchProtectionSupportedArch( + CGM.getTarget().getTargetOpts().CPU)) { + TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); + if (BPI.BranchTargetEnforcement) +Fn->addFnAttr("branch-target-enforcement", "true"); nickdesaulniers wrote: On L157 this fn attr gets set to `false`. Does that need to happen on this branch, too? --- Consider adding a `getBranchTargetEnforcementStr` method to `BranchProtectionInfo` to encapsulate this. Perhaps the same for branch-protection-pauth-lr and guarded-control-stack (L129-L134 of clang/lib/CodeGen/Targets/AArch64.cpp). --- I wonder if `BranchProtectionInfo` should just have one method that accepts an `llvm::Function` and sets all appropriate fn attrs there? Then what looks somewhat like duplicated logic (between ARM.cpp and AArch64.cpp) to me could be shared there? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -173,6 +167,13 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo { diag::warn_target_unsupported_branch_protection_attribute) << Attr.CPU; } +} else if (CGM.getTarget().isBranchProtectionSupportedArch( + CGM.getTarget().getTargetOpts().CPU)) { + TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts()); + if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) +Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr()); + if (BPI.BranchTargetEnforcement) +Fn->addFnAttr("branch-target-enforcement", "true"); nickdesaulniers wrote: > Consider adding a `getBranchTargetEnforcementStr` method to > `BranchProtectionInfo` to encapsulate this. Looks like that might also be useful in the child commit https://github.com/llvm/llvm-project/pull/83277 ? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1369,13 +1369,50 @@ class TargetInfo : public TransferrableTargetInfo, } struct BranchProtectionInfo { -LangOptions::SignReturnAddressScopeKind SignReturnAddr = -LangOptions::SignReturnAddressScopeKind::None; -LangOptions::SignReturnAddressKeyKind SignKey = -LangOptions::SignReturnAddressKeyKind::AKey; -bool BranchTargetEnforcement = false; -bool BranchProtectionPAuthLR = false; -bool GuardedControlStack = false; +LangOptions::SignReturnAddressScopeKind SignReturnAddr; +LangOptions::SignReturnAddressKeyKind SignKey; +bool BranchTargetEnforcement; +bool BranchProtectionPAuthLR; +bool GuardedControlStack; + +BranchProtectionInfo() = default; DanielKristofKiss wrote: yes in `Sema::checkTargetAttr` https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/DanielKristofKiss updated https://github.com/llvm/llvm-project/pull/82819 >From 9e844b9e61ad1c81983aafd7ac9b3fb9e1939787 Mon Sep 17 00:00:00 2001 From: Daniel Kiss Date: Mon, 22 Jan 2024 11:33:15 +0100 Subject: [PATCH] Emit attributes for functions always. Branch protection, sign return address, guarded control stack flags are only emitted as module flags if not specified per function. The inliner might inline functions with different set of flags as it doesn't see the flags. In case of LTO build the module flags get merged with the `min` rule which means if one of the modules is not build with PAC/BTI then the features will be turned off on all functions due to the functions takes the branch-protection and sign-return-address features from the module flags. The sign-return-address is function level option therefore it is expected functions from files that are compiled with -mbranch-protection=pac-ret to be protected but in LTO case this might not happen. This patch adds the flags to functions in case of an LTO build therefore they don't need to rely on the module flag. --- clang/include/clang/Basic/TargetInfo.h| 44 -- clang/lib/CodeGen/Targets/AArch64.cpp | 45 +++ clang/lib/CodeGen/Targets/ARM.cpp | 8 ++-- .../CodeGen/aarch64-branch-protection-attr.c | 24 +- .../CodeGen/aarch64-sign-return-address.c | 14 -- .../CodeGen/arm-branch-protection-attr-1.c| 10 ++--- .../CodeGen/arm-branch-protection-attr-2.c| 13 -- .../test/Frontend/arm-branch-protection-lto.c | 22 + 8 files changed, 119 insertions(+), 61 deletions(-) create mode 100644 clang/test/Frontend/arm-branch-protection-lto.c diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 7682f84e491c7b..bd8bdaefe4eb1b 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -31,7 +31,9 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Frontend/OpenMP/OMPGridValues.h" +#include "llvm/IR/Attributes.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Function.h" #include "llvm/Support/DataTypes.h" #include "llvm/Support/Error.h" #include "llvm/Support/VersionTuple.h" @@ -1368,15 +1370,15 @@ class TargetInfo : public TransferrableTargetInfo, return StringRef(); } - struct BranchProtectionInfo { + class BranchProtectionInfo { + public: LangOptions::SignReturnAddressScopeKind SignReturnAddr; LangOptions::SignReturnAddressKeyKind SignKey; bool BranchTargetEnforcement; bool BranchProtectionPAuthLR; bool GuardedControlStack; -BranchProtectionInfo() = default; - + protected: const char *getSignReturnAddrStr() const { switch (SignReturnAddr) { case LangOptions::SignReturnAddressScopeKind::None: @@ -1398,6 +1400,42 @@ class TargetInfo : public TransferrableTargetInfo, } llvm_unreachable("Unexpected SignReturnAddressKeyKind"); } + + public: +BranchProtectionInfo() = default; +BranchProtectionInfo(const LangOptions &LangOpts) { + SignReturnAddr = + LangOpts.hasSignReturnAddress() + ? (LangOpts.isSignReturnAddressScopeAll() + ? LangOptions::SignReturnAddressScopeKind::All + : LangOptions::SignReturnAddressScopeKind::NonLeaf) + : LangOptions::SignReturnAddressScopeKind::None; + SignKey = LangOpts.isSignReturnAddressWithAKey() +? LangOptions::SignReturnAddressKeyKind::AKey +: LangOptions::SignReturnAddressKeyKind::BKey; + BranchTargetEnforcement = LangOpts.BranchTargetEnforcement; + BranchProtectionPAuthLR = LangOpts.BranchProtectionPAuthLR; + GuardedControlStack = LangOpts.GuardedControlStack; +} + +void setFnAttributes(llvm::Function &F) { + llvm::AttrBuilder FuncAttrs(F.getContext()); + setFnAttributes(FuncAttrs); + F.addFnAttrs(FuncAttrs); +} + +void setFnAttributes(llvm::AttrBuilder &FuncAttrs) { + if (SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) { +FuncAttrs.addAttribute("sign-return-address", getSignReturnAddrStr()); +FuncAttrs.addAttribute("sign-return-address-key", getSignKeyStr()); + } + if (BranchTargetEnforcement) +FuncAttrs.addAttribute("branch-target-enforcement", "true"); + if (BranchProtectionPAuthLR) +FuncAttrs.addAttribute("branch-protection-pauth-lr", "true"); + if (GuardedControlStack) +FuncAttrs.addAttribute("guarded-control-stack", "true"); +} }; /// Determine if the Architecture in this TargetInfo supports branch diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 725e8a70fddfe6..a62814ba34ef57 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -116,3