Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update FEAT_PAuth_LR behaviour for AArch64 #90614

Merged
merged 4 commits into from
May 10, 2024

Conversation

Stylie777
Copy link
Contributor

Currently, LLVM enables -mbranch-protection=standard as bti+pac-ret. To align LLVM with the behaviour in GNU, this has been updated to bti+pac-ret+pc when FEAT_PAuth_LR is enabled as an optional feature via the -mcpu= options. If this is not enabled, then this will revert to the existing behaviour.

@Stylie777 Stylie777 changed the title Update FEA_PAuth_LR behaviour for AArch64 Update FEAT_PAuth_LR behaviour for AArch64 Apr 30, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-aarch64

Author: Jack Styles (Stylie777)

Changes

Currently, LLVM enables -mbranch-protection=standard as bti+pac-ret. To align LLVM with the behaviour in GNU, this has been updated to bti+pac-ret+pc when FEAT_PAuth_LR is enabled as an optional feature via the -mcpu= options. If this is not enabled, then this will revert to the existing behaviour.


Full diff: https://github.com/llvm/llvm-project/pull/90614.diff

8 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+1-1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+17-1)
  • (modified) clang/test/Preprocessor/aarch64-target-features.c (+4)
  • (modified) llvm/docs/ReleaseNotes.rst (+5)
  • (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+2)
  • (modified) llvm/include/llvm/TargetParser/ARMTargetParserCommon.h (+1-1)
  • (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+5)
  • (modified) llvm/lib/TargetParser/ARMTargetParserCommon.cpp (+2-1)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..5a46a3e6c6fd3e 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -224,7 +224,7 @@ bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
                                                  BranchProtectionInfo &BPI,
                                                  StringRef &Err) const {
   llvm::ARM::ParsedBranchProtection PBP;
-  if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err))
+  if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err, HasPAuthLR))
     return false;
 
   BPI.SignReturnAddr =
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1988a90a48ae6b..057a0d850ed9b3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1511,7 +1511,23 @@ static void CollectARMPACBTIOptions(const ToolChain &TC, const ArgList &Args,
   } else {
     StringRef DiagMsg;
     llvm::ARM::ParsedBranchProtection PBP;
-    if (!llvm::ARM::parseBranchProtection(A->getValue(), PBP, DiagMsg))
+
+    // To know if we need to enable PAuth-LR As part of the standard branch
+    // protection option, it needs to be determined if the feature has been
+    // activated in the `march` argument. This information is stored within the
+    // CmdArgs variable and can be found using a search.
+    if (isAArch64) {
+      auto isPAuthLR = [](const char *member) {
+        llvm::AArch64::ExtensionInfo pauthlr_extension =
+            llvm::AArch64::getExtensionByID(llvm::AArch64::AEK_PAUTHLR);
+        return (pauthlr_extension.Feature.compare(member) == 0);
+      };
+
+      if (std::any_of(CmdArgs.begin(), CmdArgs.end(), isPAuthLR))
+        EnablePAuthLR = true;
+    }
+    if (!llvm::ARM::parseBranchProtection(A->getValue(), PBP, DiagMsg,
+                                          EnablePAuthLR))
       D.Diag(diag::err_drv_unsupported_option_argument)
           << A->getSpelling() << DiagMsg;
     if (!isAArch64 && PBP.Key == "b_key")
diff --git a/clang/test/Preprocessor/aarch64-target-features.c b/clang/test/Preprocessor/aarch64-target-features.c
index 85762b7fed4d71..31af14f748b765 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -616,6 +616,9 @@
 // ================== Check Armv9.5-A Pointer Authentication Enhancements(PAuth_LR).
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-PAUTH-LR-OFF %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv9.5-a -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-PAUTH-LR-OFF %s
+// RUN: %clang -target arm64-none-linux-gnu -march=armv9.5-a -mbranch-protection=standard -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR-OFF,CHECK-BRANCH-PROTECTION-NO-PC %s
+// RUN: %clang -target arm64-none-linux-gnu -march=armv9.5-a+pauth-lr -mbranch-protection=standard -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR,CHECK-BRANCH-PROTECTION-PC %s
+// RUN: %clang -target arm64-none-linux-gnu -march=armv9.5-a+nopauth-lr -mbranch-protection=standard -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR-OFF,CHECK-BRANCH-PROTECTION-NO-PC %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth -mbranch-protection=none -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-PAUTH-LR-OFF %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth-lr -mbranch-protection=none -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-PAUTH-LR %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth-lr -mbranch-protection=bti -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-PAUTH-LR %s
@@ -636,6 +639,7 @@
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth-lr -mbranch-protection=pac-ret+pc+b-key -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR,CHECK-BRANCH-PROTECTION-PC-BKEY %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth-lr -mbranch-protection=pac-ret+pc+leaf -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR,CHECK-BRANCH-PROTECTION-PC-LEAF %s
 // RUN: %clang -target arm64-none-linux-gnu -march=armv8-a+pauth-lr -mbranch-protection=pac-ret+pc+leaf+b-key -x c -E -dM %s -o - | FileCheck -check-prefixes=CHECK-PAUTH-LR,CHECK-BRANCH-PROTECTION-PC-LEAF-BKEY %s
+// CHECK-BRANCH-PROTECTION-NO-PC:        #define __ARM_FEATURE_PAC_DEFAULT 1
 // CHECK-BRANCH-PROTECTION-PC:           #define __ARM_FEATURE_PAC_DEFAULT 9
 // CHECK-BRANCH-PROTECTION-PC-BKEY:      #define __ARM_FEATURE_PAC_DEFAULT 10
 // CHECK-BRANCH-PROTECTION-PC-LEAF:      #define __ARM_FEATURE_PAC_DEFAULT 13
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 46d79d6c5822b1..e98d31694046c8 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -76,6 +76,11 @@ Changes to the AArch64 Backend
 * Added support for Cortex-A78AE, Cortex-A520AE, Cortex-A720AE,
   Neoverse-N3, Neoverse-V3 and Neoverse-V3AE CPUs.
 
+* `-mbranch-protection=standard` now enables FEAT_PAuth_LR by
+default when the feature is enabled. The new behaviour results 
+in `standard` being equal to `bti+pac-ret+pc` when `+pauth-lr` 
+is passed as part of `-mcpu=`options. 
+
 Changes to the AMDGPU Backend
 -----------------------------
 
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 0d1cfd152151aa..d39fc5667358a0 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -853,6 +853,8 @@ inline constexpr Alias CpuAliases[] = {{"cobalt-100", "neoverse-n2"},
 
 inline constexpr Alias ExtAliases[] = {{"rdma", "rdm"}};
 
+const ExtensionInfo &getExtensionByID(ArchExtKind(ExtID));
+
 bool getExtensionFeatures(
     const AArch64::ExtensionBitset &Extensions,
     std::vector<StringRef> &Features);
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParserCommon.h b/llvm/include/llvm/TargetParser/ARMTargetParserCommon.h
index 8ae553ca80ddc3..f6115718e9f5fd 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParserCommon.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParserCommon.h
@@ -46,7 +46,7 @@ struct ParsedBranchProtection {
 };
 
 bool parseBranchProtection(StringRef Spec, ParsedBranchProtection &PBP,
-                           StringRef &Err);
+                           StringRef &Err, bool EnablePAuthLR = false);
 
 } // namespace ARM
 } // namespace llvm
diff --git a/llvm/lib/TargetParser/AArch64TargetParser.cpp b/llvm/lib/TargetParser/AArch64TargetParser.cpp
index 71099462d5ecff..026214e7e2eac5 100644
--- a/llvm/lib/TargetParser/AArch64TargetParser.cpp
+++ b/llvm/lib/TargetParser/AArch64TargetParser.cpp
@@ -280,3 +280,8 @@ bool AArch64::ExtensionSet::parseModifier(StringRef Modifier) {
   }
   return false;
 }
+
+const AArch64::ExtensionInfo &
+AArch64::getExtensionByID(AArch64::ArchExtKind ExtID) {
+  return lookupExtensionByID(ExtID);
+}
diff --git a/llvm/lib/TargetParser/ARMTargetParserCommon.cpp b/llvm/lib/TargetParser/ARMTargetParserCommon.cpp
index 45d04f9bcbfb6a..d6ce6581bb1a92 100644
--- a/llvm/lib/TargetParser/ARMTargetParserCommon.cpp
+++ b/llvm/lib/TargetParser/ARMTargetParserCommon.cpp
@@ -139,7 +139,7 @@ ARM::EndianKind ARM::parseArchEndian(StringRef Arch) {
 // returned in `PBP`. Returns false in error, with `Err` containing
 // an erroneous part of the spec.
 bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection &PBP,
-                                StringRef &Err) {
+                                StringRef &Err, bool EnablePAuthLR) {
   PBP = {"none", "a_key", false, false, false};
   if (Spec == "none")
     return true; // defaults are ok
@@ -148,6 +148,7 @@ bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection &PBP,
     PBP.Scope = "non-leaf";
     PBP.BranchTargetEnforcement = true;
     PBP.GuardedControlStack = true;
+    PBP.BranchProtectionPAuthLR = EnablePAuthLR;
     return true;
   }
 

@Stylie777 Stylie777 force-pushed the update_pauthlr_behaviour_for_aarch64 branch 2 times, most recently from b37f6e9 to 70692dc Compare May 1, 2024 08:19
Copy link
Contributor

@jthackray jthackray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Stylie777 Stylie777 force-pushed the update_pauthlr_behaviour_for_aarch64 branch from 61d7812 to a8c99dc Compare May 8, 2024 13:08
Currently, an extension cannot be found using the ExtID. To address this,
the function `lookupExtensionByID` has been added to the `ExtensionSet`
Class so it can be targeted from outside the function.

This will allow for the Extensions Information to be searched for and
stored externally to the ExtensionSet Class. This enables being able to
search for if Architecture Features have been enabled by the user in the
command line.
 branch protection when the feature is available

Currently, LLVM implements the `standard` option as `bti+pac-ret`
for ARM and AArch64. Following discussions with the GNU
developemnt team within Arm it was decided to align the
behaviour of `standard` to match across the different compilers.

To ensure the behaviour is aligned. LLVM has been updated to
implement `standard` as `bti+pac-ret+pc` by default when
`+pauth-lr` is passed as part of the `-march` argument.
This was causing a build failure due to using the incorrect syntax
for quoting.
@Stylie777 Stylie777 force-pushed the update_pauthlr_behaviour_for_aarch64 branch from a8c99dc to 3cc7e66 Compare May 9, 2024 07:38
@Stylie777 Stylie777 self-assigned this May 9, 2024
@Stylie777 Stylie777 merged commit 6aac30f into llvm:main May 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants