[llvm] [clang] [Verifier] Check function attributes related to branch protection (NFC) (PR #70565)
Momchil Velikov via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 4 06:52:42 PST 2023
https://github.com/momchil-velikov updated https://github.com/llvm/llvm-project/pull/70565
>From 66a84fffb5d1b5c34eea9ecdb83a88afb0b627ff Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Sat, 28 Oct 2023 15:01:36 +0100
Subject: [PATCH 1/4] [Verifier] Check function attributes related to branch
protection (NFC)
---
llvm/lib/IR/Verifier.cpp | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5f466581ea980..ab9cc89693bd4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2241,6 +2241,27 @@ 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 (Attrs.hasFnAttr("sign-return-adress")) {
+ StringRef S = Attrs.getFnAttr("sign-return-adress").getValueAsString();
+ if (S != "none" && S != "all" && S != "non-leaf")
+ CheckFailed("invalid value for 'sign-return-adress' attribute: " + S, V);
+ }
+
+ if (Attrs.hasFnAttr("sign-return-adress-key")) {
+ StringRef S = Attrs.getFnAttr("sign-return-adress-key").getValueAsString();
+ if (!S.equals_insensitive("a_key") && !S.equals_insensitive("b_key"))
+ CheckFailed("invalid value for 'sign-return-adress-key' attribute: " + S,
+ V);
+ }
+
+ if (Attrs.hasFnAttr("branch-target-enforcement")) {
+ StringRef S =
+ Attrs.getFnAttr("branch-target-enforcement").getValueAsString();
+ if (!S.equals_insensitive("true") && !S.equals_insensitive("false"))
+ CheckFailed(
+ "invalid value for 'branch-target-enforcement' attribute: " + S, V);
+ }
}
void Verifier::verifyFunctionMetadata(
>From 224db85a1dad0416ab403ab526eae413bd0f03fd Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Mon, 30 Oct 2023 09:27:08 +0000
Subject: [PATCH 2/4] Fix typos, do attribute lookup once
---
llvm/lib/IR/Verifier.cpp | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index ab9cc89693bd4..c01e52d2671cb 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2242,22 +2242,21 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-entry", V);
checkUnsignedBaseTenFuncAttr(Attrs, "warn-stack-size", V);
- if (Attrs.hasFnAttr("sign-return-adress")) {
- StringRef S = Attrs.getFnAttr("sign-return-adress").getValueAsString();
+ 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-adress' attribute: " + S, V);
+ CheckFailed("invalid value for 'sign-return-address' attribute: " + S, V);
}
- if (Attrs.hasFnAttr("sign-return-adress-key")) {
- StringRef S = Attrs.getFnAttr("sign-return-adress-key").getValueAsString();
+ if (auto A = Attrs.getFnAttr("sign-return-address-key"); A.isValid()) {
+ StringRef S = A.getValueAsString();
if (!S.equals_insensitive("a_key") && !S.equals_insensitive("b_key"))
- CheckFailed("invalid value for 'sign-return-adress-key' attribute: " + S,
+ CheckFailed("invalid value for 'sign-return-address-key' attribute: " + S,
V);
}
- if (Attrs.hasFnAttr("branch-target-enforcement")) {
- StringRef S =
- Attrs.getFnAttr("branch-target-enforcement").getValueAsString();
+ if (auto A = Attrs.getFnAttr("branch-target-enforcement"); A.isValid()) {
+ StringRef S = A.getValueAsString();
if (!S.equals_insensitive("true") && !S.equals_insensitive("false"))
CheckFailed(
"invalid value for 'branch-target-enforcement' attribute: " + S, V);
>From 24b5f411e79f1173b8413b8a6ab113169fc9c0b6 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Mon, 30 Oct 2023 10:34:26 +0000
Subject: [PATCH 3/4] Add a test
---
llvm/test/Verifier/branch-prot-attrs.ll | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 llvm/test/Verifier/branch-prot-attrs.ll
diff --git a/llvm/test/Verifier/branch-prot-attrs.ll b/llvm/test/Verifier/branch-prot-attrs.ll
new file mode 100644
index 0000000000000..123a8207cfe6b
--- /dev/null
+++ b/llvm/test/Verifier/branch-prot-attrs.ll
@@ -0,0 +1,13 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+define void @f() #0 {
+ ret void
+}
+
+attributes #0 = {
+; CHECK: invalid value for 'sign-return-address' attribute: loaf
+ "sign-return-address"="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" }
>From d4d34e3f4c60f40cbc1d64d9f6458aac97b5e323 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Sat, 4 Nov 2023 18:45:59 +0000
Subject: [PATCH 4/4] Recognise only lower case attributes
---
clang/lib/Frontend/CompilerInvocation.cpp | 4 ++--
llvm/lib/IR/Verifier.cpp | 4 ++--
.../AArch64/AArch64MachineFunctionInfo.cpp | 9 ++++-----
llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp | 5 ++---
llvm/test/Verifier/branch-prot-attrs.ll | 16 ++++++++++++++--
5 files changed, 24 insertions(+), 14 deletions(-)
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 c01e52d2671cb..f137f0468c3c5 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2250,14 +2250,14 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
if (auto A = Attrs.getFnAttr("sign-return-address-key"); A.isValid()) {
StringRef S = A.getValueAsString();
- if (!S.equals_insensitive("a_key") && !S.equals_insensitive("b_key"))
+ 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.equals_insensitive("true") && !S.equals_insensitive("false"))
+ if (S != "true" && S != "false")
CheckFailed(
"invalid value for 'branch-target-enforcement' attribute: " + S, V);
}
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
index 123a8207cfe6b..aa0fad6b258ec 100644
--- a/llvm/test/Verifier/branch-prot-attrs.ll
+++ b/llvm/test/Verifier/branch-prot-attrs.ll
@@ -4,10 +4,22 @@ define void @f() #0 {
ret void
}
+define void @g() #1 {
+ ret void
+}
+
attributes #0 = {
-; CHECK: invalid value for 'sign-return-address' attribute: loaf
- "sign-return-address"="loaf"
+; 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" }
More information about the cfe-commits
mailing list