[clang] e3a97df - [Verifier] Check function attributes related to branch protection (NFC) (#70565)

[via cfe-commits]

Author: Momchil Velikov
Date: 2023-12-04T16:16:55Z
New Revision: e3a97dffee93f03e12b5911869f9a443c39821bc

URL: https://github.com/llvm/llvm-project/commit/e3a97dffee93f03e12b5911869f9a443c39821bc
DIFF: https://github.com/llvm/llvm-project/commit/e3a97dffee93f03e12b5911869f9a443c39821bc.diff

LOG: [Verifier] Check function attributes related to branch protection (NFC) (#70565)

Added: 
    llvm/test/Verifier/branch-prot-attrs.ll

Modified: 
    clang/lib/Frontend/CompilerInvocation.cpp
    llvm/lib/IR/Verifier.cpp
    llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
    llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index a6188cb45e178..be0d4963a2092 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4159,10 +4159,10 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
     if (Arg *A = Args.getLastArg(OPT_msign_return_address_key_EQ)) {
       StringRef SignKey = A->getValue();
       if (!SignScope.empty() && !SignKey.empty()) {
-        if (SignKey.equals_insensitive("a_key"))
+        if (SignKey == "a_key")
           Opts.setSignReturnAddressKey(
               LangOptions::SignReturnAddressKeyKind::AKey);
-        else if (SignKey.equals_insensitive("b_key"))
+        else if (SignKey == "b_key")
           Opts.setSignReturnAddressKey(
               LangOptions::SignReturnAddressKeyKind::BKey);
         else

diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5f466581ea980..f137f0468c3c5 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2241,6 +2241,26 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
   checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-prefix", V);
   checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-entry", V);
   checkUnsignedBaseTenFuncAttr(Attrs, "warn-stack-size", V);
+
+  if (auto A = Attrs.getFnAttr("sign-return-address"); A.isValid()) {
+    StringRef S = A.getValueAsString();
+    if (S != "none" && S != "all" && S != "non-leaf")
+      CheckFailed("invalid value for 'sign-return-address' attribute: " + S, V);
+  }
+
+  if (auto A = Attrs.getFnAttr("sign-return-address-key"); A.isValid()) {
+    StringRef S = A.getValueAsString();
+    if (S != "a_key" && S != "b_key")
+      CheckFailed("invalid value for 'sign-return-address-key' attribute: " + S,
+                  V);
+  }
+
+  if (auto A = Attrs.getFnAttr("branch-target-enforcement"); A.isValid()) {
+    StringRef S = A.getValueAsString();
+    if (S != "true" && S != "false")
+      CheckFailed(
+          "invalid value for 'branch-target-enforcement' attribute: " + S, V);
+  }
 }
 
 void Verifier::verifyFunctionMetadata(

diff  --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
index f8b73ba6dda3c..9da59ef2a8062 100644
--- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
@@ -78,8 +78,8 @@ static bool ShouldSignWithBKey(const Function &F, const AArch64Subtarget &STI) {
 
   const StringRef Key =
       F.getFnAttribute("sign-return-address-key").getValueAsString();
-  assert(Key.equals_insensitive("a_key") || Key.equals_insensitive("b_key"));
-  return Key.equals_insensitive("b_key");
+  assert(Key == "a_key" || Key == "b_key");
+  return Key == "b_key";
 }
 
 AArch64FunctionInfo::AArch64FunctionInfo(const Function &F,
@@ -100,9 +100,8 @@ AArch64FunctionInfo::AArch64FunctionInfo(const Function &F,
   } else {
     const StringRef BTIEnable =
         F.getFnAttribute("branch-target-enforcement").getValueAsString();
-    assert(BTIEnable.equals_insensitive("true") ||
-           BTIEnable.equals_insensitive("false"));
-    BranchTargetEnforcement = BTIEnable.equals_insensitive("true");
+    assert(BTIEnable == "true" || BTIEnable == "false");
+    BranchTargetEnforcement = BTIEnable == "true";
   }
 
   // The default stack probe size is 4096 if the function has no

diff  --git a/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp b/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp
index aa9d8b54d9636..ec2d8f59ee9b5 100644
--- a/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp
@@ -27,9 +27,8 @@ static bool GetBranchTargetEnforcement(const Function &F,
 
   const StringRef BTIEnable =
       F.getFnAttribute("branch-target-enforcement").getValueAsString();
-  assert(BTIEnable.equals_insensitive("true") ||
-         BTIEnable.equals_insensitive("false"));
-  return BTIEnable.equals_insensitive("true");
+  assert(BTIEnable == "true" || BTIEnable == "false");
+  return BTIEnable == "true";
 }
 
 // The pair returns values for the ARMFunctionInfo members

diff  --git a/llvm/test/Verifier/branch-prot-attrs.ll b/llvm/test/Verifier/branch-prot-attrs.ll
new file mode 100644
index 0000000000000..aa0fad6b258ec
--- /dev/null
+++ b/llvm/test/Verifier/branch-prot-attrs.ll
@@ -0,0 +1,25 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @f() #0 {
+  ret void
+}
+
+define void @g() #1 {
+  ret void
+}
+
+attributes #0 = {
+; CHECK:  invalid value for 'sign-return-address' attribute: non-loaf
+  "sign-return-address"="non-loaf"
+; CHECK: invalid value for 'sign-return-address-key' attribute: bad-mkey
+  "sign-return-address-key"="bad-mkey"
+; CHECK:   invalid value for 'branch-target-enforcement' attribute: yes-please
+  "branch-target-enforcement"="yes-please" }
+
+attributes #1 = {
+; CHECK:  invalid value for 'sign-return-address' attribute: All
+  "sign-return-address"="All"
+; CHECK: invalid value for 'sign-return-address-key' attribute: B_Key
+  "sign-return-address-key"="B_Key"
+; CHECK:   invalid value for 'branch-target-enforcement' attribute: True
+  "branch-target-enforcement"="True" }