https://github.com/momchil-velikov updated https://github.com/llvm/llvm-project/pull/70565
>From eb47903ad47f4a9e833948424a1c88bc2c72090e Mon Sep 17 00:00:00 2001 From: Momchil Velikov <momchil.veli...@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 d3db7a16dc0d607..a65c5abb823011f 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2244,6 +2244,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 e518faffb9ec822c6809a6b8e731425c3212a7eb Mon Sep 17 00:00:00 2001 From: Momchil Velikov <momchil.veli...@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 a65c5abb823011f..453a51b859d1ebe 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2245,22 +2245,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 a3aeeb8ad28de3c102180d82d86a0d3aed760d00 Mon Sep 17 00:00:00 2001 From: Momchil Velikov <momchil.veli...@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 ccc214037354b96dc948d1b52f10ea203c19f1b0 Mon Sep 17 00:00:00 2001 From: Momchil Velikov <momchil.veli...@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 453a51b859d1ebe..3bfc9d2c623319d 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2253,14 +2253,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" } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits