[llvm] [clang] [Verifier] Check function attributes related to branch protection (NFC) (PR #70565)
Momchil Velikov via cfe-commits
cfe-commits at lists.llvm.org
Sat Nov 11 09:19:58 PST 2023
https://github.com/momchil-velikov updated https://github.com/llvm/llvm-project/pull/70565
>From 0fb3e4e96d9377e65d1c794fe0b648ff835748b9 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 b1d1075285c2210..58f7f674a3405a3 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2248,6 +2248,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 a8d2edfc0bdb59f319c0ab11aa0b182750dcdfca 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 58f7f674a3405a3..7a259833808aafd 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2249,22 +2249,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 74ad1067e56329bc3aa7c0315f6fefed71ec294c 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 000000000000000..123a8207cfe6b4f
--- /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 aa0fef6e712d0767bce8b89359e6e36d8dfa1eca 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 637c6a35af6532b..383525d4c29c4af 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4121,10 +4121,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 7a259833808aafd..3d89db1c77815a1 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2257,14 +2257,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 7bb5041b8ba9481..e21982b5987f98e 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,
@@ -102,9 +102,8 @@ AArch64FunctionInfo::AArch64FunctionInfo(const Function &F,
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";
}
MachineFunctionInfo *AArch64FunctionInfo::clone(
diff --git a/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp b/llvm/lib/Target/ARM/ARMMachineFunctionInfo.cpp
index aa9d8b54d963607..ec2d8f59ee9b507 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 123a8207cfe6b4f..aa0fad6b258ec2b 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