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.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 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.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 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.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 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.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 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" } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits