llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-lto Author: Dani (DanielKristofKiss) <details> <summary>Changes</summary> If the signing scheme is different that maybe the functions assumes different behaviours and dangerous to inline them without analysing them. This should be a rare case. --- Full diff: https://github.com/llvm/llvm-project/pull/80642.diff 11 Files Affected: - (modified) clang/lib/CodeGen/CodeGenModule.cpp (+4-10) - (modified) llvm/include/llvm/IR/AutoUpgrade.h (+2-1) - (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+1-1) - (modified) llvm/lib/IR/AutoUpgrade.cpp (+68-1) - (modified) llvm/lib/Linker/IRMover.cpp (+4) - (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+15) - (modified) llvm/test/Bitcode/upgrade-arc-runtime-calls.ll (+2-2) - (modified) llvm/test/LTO/AArch64/link-branch-target-enforcement.ll (+1) - (added) llvm/test/LTO/AArch64/link-sign-return-address.ll (+43) - (modified) llvm/test/Linker/link-arm-and-thumb.ll (+4-3) - (added) llvm/test/Transforms/Inline/inline-sign-return-address.ll (+126) ``````````diff diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index c63e4ecc3dcba..36b63d78b06f8 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -1044,17 +1044,14 @@ void CodeGenModule::Release() { llvm::MDString::get(VMContext, "ascii")); } - llvm::Triple::ArchType Arch = Context.getTargetInfo().getTriple().getArch(); - if ( Arch == llvm::Triple::arm - || Arch == llvm::Triple::armeb - || Arch == llvm::Triple::thumb - || Arch == llvm::Triple::thumbeb) { + llvm::Triple T = Context.getTargetInfo().getTriple(); + if (T.isARM() || T.isThumb()) { // The minimum width of an enum in bytes uint64_t EnumWidth = Context.getLangOpts().ShortEnums ? 1 : 4; getModule().addModuleFlag(llvm::Module::Error, "min_enum_size", EnumWidth); } - if (Arch == llvm::Triple::riscv32 || Arch == llvm::Triple::riscv64) { + if (T.isRISCV()) { StringRef ABIStr = Target.getABI(); llvm::LLVMContext &Ctx = TheModule.getContext(); getModule().addModuleFlag(llvm::Module::Error, "target-abi", @@ -1127,10 +1124,7 @@ void CodeGenModule::Release() { getModule().addModuleFlag(llvm::Module::Override, "tag-stack-memory-buildattr", 1); - if (Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb || - Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb || - Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_32 || - Arch == llvm::Triple::aarch64_be) { + if (T.isARM() || T.isThumb() || T.isAArch64()) { if (LangOpts.BranchTargetEnforcement) getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement", 1); diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h index 152f781ffa9b3..c0d96efc54752 100644 --- a/llvm/include/llvm/IR/AutoUpgrade.h +++ b/llvm/include/llvm/IR/AutoUpgrade.h @@ -67,7 +67,8 @@ namespace llvm { void UpgradeSectionAttributes(Module &M); /// Correct any IR that is relying on old function attribute behavior. - void UpgradeFunctionAttributes(Function &F); + void UpgradeFunctionAttributes(Function &F, + bool ModuleMetadataIsMaterialized = false); /// If the given TBAA tag uses the scalar TBAA format, create a new node /// corresponding to the upgrade to the struct-path aware TBAA format. diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 5b233fb365fe2..6b335dd9f1f89 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -6708,7 +6708,7 @@ Error BitcodeReader::materialize(GlobalValue *GV) { } // Look for functions that rely on old function attribute behavior. - UpgradeFunctionAttributes(*F); + UpgradeFunctionAttributes(*F, true); // If we've materialized a function set up in "new" debug-info mode, the // contents just loaded will still be in dbg.value mode. Switch to the new diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp index 19d80eb9aec0b..e25ac46450cec 100644 --- a/llvm/lib/IR/AutoUpgrade.cpp +++ b/llvm/lib/IR/AutoUpgrade.cpp @@ -5155,7 +5155,39 @@ struct StrictFPUpgradeVisitor : public InstVisitor<StrictFPUpgradeVisitor> { }; } // namespace -void llvm::UpgradeFunctionAttributes(Function &F) { +static void +CopyModuleAttributeToFunction(Function &F, StringRef FnAttrName, + StringRef ModAttrName, + std::pair<StringRef, StringRef> Values) { + Module *M = F.getParent(); + if (!M) + return; + if (F.hasFnAttribute(FnAttrName)) + return; + if (const auto *MAttr = mdconst::extract_or_null<ConstantInt>( + M->getModuleFlag(ModAttrName))) { + if (MAttr->getZExtValue()) { + F.addFnAttr(FnAttrName, Values.first); + return; + } + } + F.addFnAttr(FnAttrName, Values.second); +} + +static void CopyModuleAttributeToFunction(Function &F, StringRef AttrName) { + CopyModuleAttributeToFunction( + F, AttrName, AttrName, + std::make_pair<StringRef, StringRef>("true", "false")); +} + +static void +CopyModuleAttributeToFunction(Function &F, StringRef AttrName, + std::pair<StringRef, StringRef> Values) { + CopyModuleAttributeToFunction(F, AttrName, AttrName, Values); +} + +void llvm::UpgradeFunctionAttributes(Function &F, + bool ModuleMetadataIsMaterialized) { // If a function definition doesn't have the strictfp attribute, // convert any callsite strictfp attributes to nobuiltin. if (!F.isDeclaration() && !F.hasFnAttribute(Attribute::StrictFP)) { @@ -5167,6 +5199,41 @@ void llvm::UpgradeFunctionAttributes(Function &F) { F.removeRetAttrs(AttributeFuncs::typeIncompatible(F.getReturnType())); for (auto &Arg : F.args()) Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType())); + + if (!ModuleMetadataIsMaterialized) + return; + if (F.isDeclaration()) + return; + Module *M = F.getParent(); + if (!M) + return; + + Triple T(M->getTargetTriple()); + // Convert module level attributes to function level attributes because + // after merging modules the attributes might change and would have different + // effect on the functions as the original module would have. + if (T.isThumb() || T.isARM() || T.isAArch64()) { + if (!F.hasFnAttribute("sign-return-address")) { + StringRef SignType = "none"; + if (const auto *Sign = mdconst::extract_or_null<ConstantInt>( + M->getModuleFlag("sign-return-address"))) + if (Sign->getZExtValue()) + SignType = "non-leaf"; + + if (const auto *SignAll = mdconst::extract_or_null<ConstantInt>( + M->getModuleFlag("sign-return-address-all"))) + if (SignAll->getZExtValue()) + SignType = "all"; + + F.addFnAttr("sign-return-address", SignType); + } + CopyModuleAttributeToFunction(F, "branch-target-enforcement"); + CopyModuleAttributeToFunction(F, "branch-protection-pauth-lr"); + CopyModuleAttributeToFunction(F, "guarded-control-stack"); + CopyModuleAttributeToFunction( + F, "sign-return-address-key", + std::make_pair<StringRef, StringRef>("b_key", "a_key")); + } } static bool isOldLoopArgument(Metadata *MD) { diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp index 8cc0f7fb90991..47d5a39c9f746 100644 --- a/llvm/lib/Linker/IRMover.cpp +++ b/llvm/lib/Linker/IRMover.cpp @@ -1606,6 +1606,10 @@ Error IRLinker::run() { // Loop over all of the linked values to compute type mappings. computeTypeMapping(); + // Update function attributes before copy them to destation module. + for (Function &F : SrcM->getFunctionList()) + UpgradeFunctionAttributes(F, true); + std::reverse(Worklist.begin(), Worklist.end()); while (!Worklist.empty()) { GlobalValue *GV = Worklist.back(); diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index d4d4bf5ebdf36..639c77716463e 100644 --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -2110,6 +2110,21 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI, return InlineResult::failure("incompatible strictfp attributes"); } + // Do not inline function with a different signing scheme. + if (CalledFunc->getFnAttribute("sign-return-address") != + Caller->getFnAttribute("sign-return-address")) { + return InlineResult::failure("incompatible sign return address attributes"); + } + if (CalledFunc->getFnAttribute("sign-return-address-key") != + Caller->getFnAttribute("sign-return-address-key")) { + return InlineResult::failure("incompatible signing keys attributes"); + } + if (CalledFunc->getFnAttribute("branch-protection-pauth-lr") != + Caller->getFnAttribute("branch-protection-pauth-lr")) { + return InlineResult::failure( + "incompatible sign return address modifier attributes"); + } + // GC poses two hazards to inlining, which only occur when the callee has GC: // 1. If the caller has no GC, then the callee's GC must be propagated to the // caller. diff --git a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll index 19f25f98953fa..d2edec18d55e5 100644 --- a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll +++ b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll @@ -55,7 +55,7 @@ unwindBlock: // Check that auto-upgrader converts function calls to intrinsic calls. Note that // the auto-upgrader doesn't touch invoke instructions. -// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality +// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality // ARC: %[[V0:.*]] = tail call ptr @llvm.objc.autorelease(ptr %[[A]]) // ARC-NEXT: tail call void @llvm.objc.autoreleasePoolPop(ptr %[[A]]) // ARC-NEXT: %[[V1:.*]] = tail call ptr @llvm.objc.autoreleasePoolPush() @@ -88,7 +88,7 @@ unwindBlock: // ARC-NEXT: tail call void @llvm.objc.arc.annotation.bottomup.bbend(ptr %[[B]], ptr %[[C]]) // ARC-NEXT: invoke void @objc_autoreleasePoolPop(ptr %[[A]]) -// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality +// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality // NOUPGRADE: %[[V0:.*]] = tail call ptr @objc_autorelease(ptr %[[A]]) // NOUPGRADE-NEXT: tail call void @objc_autoreleasePoolPop(ptr %[[A]]) // NOUPGRADE-NEXT: %[[V1:.*]] = tail call ptr @objc_autoreleasePoolPush() diff --git a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll index ccf8cf67ede6d..74d9c86881d52 100644 --- a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll +++ b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll @@ -32,6 +32,7 @@ entry: ; CHECK-DUMP: <main>: ; CHECK-DUMP: bl 0x8 <main+0x8> ; CHECK-DUMP: <foo>: +; CHECK-DUMP: paciasp ; `main` doesn't support BTI while `foo` does, so in the binary ; we should see only PAC which is supported by both. diff --git a/llvm/test/LTO/AArch64/link-sign-return-address.ll b/llvm/test/LTO/AArch64/link-sign-return-address.ll new file mode 100644 index 0000000000000..b2e70b2b04e08 --- /dev/null +++ b/llvm/test/LTO/AArch64/link-sign-return-address.ll @@ -0,0 +1,43 @@ +; Testcase to check that module with different branch-target-enforcement can +; be mixed. +; +; RUN: llvm-as %s -o %t1.bc +; RUN: llvm-as %p/Inputs/foo.ll -o %t2.bc +; RUN: llvm-lto -exported-symbol main \ +; RUN: -exported-symbol foo \ +; RUN: -filetype=obj \ +; RUN: %t2.bc %t1.bc \ +; RUN: -o %t1.exe 2>&1 +; RUN: llvm-objdump -d %t1.exe | FileCheck --check-prefix=CHECK-DUMP %s +; RUN: llvm-readelf -n %t1.exe | FileCheck --allow-empty --check-prefix=CHECK-PROP %s + +target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128" +target triple = "aarch64-unknown-linux-gnu" + +declare i32 @foo(); + +define i32 @main() { +entry: + %add = call i32 @foo() + ret i32 %add +} + +!llvm.module.flags = !{!0, !1, !2, !3 } +!0 = !{i32 8, !"branch-target-enforcement", i32 0} +!1 = !{i32 8, !"sign-return-address", i32 0} +!2 = !{i32 8, !"sign-return-address-all", i32 0} +!3 = !{i32 8, !"sign-return-address-with-bkey", i32 0} + +; CHECK-DUMP: <foo>: +; CHECK-DUMP: paciasp +; CHECK-DUMP: mov w0, #0x2a +; CHECK-DUMP: autiasp +; CHECK-DUMP: ret +; CHECK-DUMP: <main>: +; CHECK-DUMP-NOT: paciasp +; CHECK-DUMP: str x30, +; CHECK-DUMP: bl 0x14 <main+0x4> + +; `main` doesn't support PAC sign-return-address while `foo` does, so in the binary +; we should not see anything. +; CHECK-PROP-NOT: Proper ties: aarch64 feature: PAC \ No newline at end of file diff --git a/llvm/test/Linker/link-arm-and-thumb.ll b/llvm/test/Linker/link-arm-and-thumb.ll index a90f2128e4430..37bd8c37f8b5e 100644 --- a/llvm/test/Linker/link-arm-and-thumb.ll +++ b/llvm/test/Linker/link-arm-and-thumb.ll @@ -13,11 +13,12 @@ entry: ret i32 %add } -; CHECK: define i32 @main() { +; CHECK: define i32 @main() [[MAIN_ATTRS:#[0-9]+]] ; CHECK: define i32 @foo(i32 %a, i32 %b) [[ARM_ATTRS:#[0-9]+]] ; CHECK: define i32 @bar(i32 %a, i32 %b) [[THUMB_ATTRS:#[0-9]+]] -; CHECK: attributes [[ARM_ATTRS]] = { "target-features"="-thumb-mode" } -; CHECK: attributes [[THUMB_ATTRS]] = { "target-features"="+thumb-mode" } +; CHECK: attributes [[MAIN_ATTRS]] = { {{.*}} } +; CHECK: attributes [[ARM_ATTRS]] = { {{.*}} "target-features"="-thumb-mode" } +; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}} "target-features"="+thumb-mode" } ; STDERR-NOT: warning: Linking two modules of different target triples: diff --git a/llvm/test/Transforms/Inline/inline-sign-return-address.ll b/llvm/test/Transforms/Inline/inline-sign-return-address.ll new file mode 100644 index 0000000000000..d83460e1e7684 --- /dev/null +++ b/llvm/test/Transforms/Inline/inline-sign-return-address.ll @@ -0,0 +1,126 @@ +; Check the inliner doesn't inline a function with different sign return address schemes. +; RUN: opt < %s -passes=inline -S | FileCheck %s + +declare void @init(ptr) + +define internal i32 @foo_all() #0 { + ret i32 43 +} + +define internal i32 @foo_nonleaf() #1 { + ret i32 44 +} + +define internal i32 @foo_none() #2 { + ret i32 42 +} + +define internal i32 @foo_lr() #3 { + ret i32 45 +} + +define internal i32 @foo_bkey() #4 { + ret i32 46 +} + +define dso_local i32 @bar_all() #0 { +; CHECK-LABEL: bar_all +; CHECK-NOT: call i32 @foo_all() +; CHECK: call i32 @foo_nonleaf() +; CHECK: call i32 @foo_none() +; CHECK: call i32 @foo_lr() +; CHECK: call i32 @foo_bkey() + %1 = call i32 @foo_all() + %2 = call i32 @foo_nonleaf() + %3 = call i32 @foo_none() + %4 = call i32 @foo_lr() + %5 = call i32 @foo_bkey() + %6 = add nsw i32 %1, %2 + %7 = add nsw i32 %6, %3 + %8 = add nsw i32 %7, %4 + %9 = add nsw i32 %8, %5 + ret i32 %9 +} + +define dso_local i32 @bar_nonleaf() #1 { +; CHECK-LABEL: bar_nonleaf +; CHECK: call i32 @foo_all() +; CHECK-NOT: call i32 @foo_nonleaf() +; CHECK: call i32 @foo_none() +; CHECK: call i32 @foo_lr() +; CHECK: call i32 @foo_bkey() + %1 = call i32 @foo_all() + %2 = call i32 @foo_nonleaf() + %3 = call i32 @foo_none() + %4 = call i32 @foo_lr() + %5 = call i32 @foo_bkey() + %6 = add nsw i32 %1, %2 + %7 = add nsw i32 %6, %3 + %8 = add nsw i32 %7, %4 + %9 = add nsw i32 %8, %5 + ret i32 %9 +} + +define dso_local i32 @bar_none() #2 { +; CHECK-LABEL: bar_none +; CHECK: call i32 @foo_all() +; CHECK: call i32 @foo_nonleaf() +; CHECK-NOT: call i32 @foo_none() +; CHECK: call i32 @foo_lr() +; CHECK: call i32 @foo_bkey() + %1 = call i32 @foo_all() + %2 = call i32 @foo_nonleaf() + %3 = call i32 @foo_none() + %4 = call i32 @foo_lr() + %5 = call i32 @foo_bkey() + %6 = add nsw i32 %1, %2 + %7 = add nsw i32 %6, %3 + %8 = add nsw i32 %7, %4 + %9 = add nsw i32 %8, %5 + ret i32 %9 +} + +define dso_local i32 @bar_lr() #3 { +; CHECK-LABEL: bar_lr +; CHECK: call i32 @foo_all() +; CHECK: call i32 @foo_nonleaf() +; CHECK: call i32 @foo_none() +; CHECK-NOT: call i32 @foo_lr() +; CHECK: call i32 @foo_bkey() + %1 = call i32 @foo_all() + %2 = call i32 @foo_nonleaf() + %3 = call i32 @foo_none() + %4 = call i32 @foo_lr() + %5 = call i32 @foo_bkey() + %6 = add nsw i32 %1, %2 + %7 = add nsw i32 %6, %3 + %8 = add nsw i32 %7, %4 + %9 = add nsw i32 %8, %5 + ret i32 %9 +} + +define dso_local i32 @bar_bkey() #4 { +; CHECK-LABEL: bar_bkey +; CHECK: call i32 @foo_all() +; CHECK: call i32 @foo_nonleaf() +; CHECK: call i32 @foo_none() +; CHECK: call i32 @foo_lr() +; CHECK-NOT: call i32 @foo_bkey() + %1 = call i32 @foo_all() + %2 = call i32 @foo_nonleaf() + %3 = call i32 @foo_none() + %4 = call i32 @foo_lr() + %5 = call i32 @foo_bkey() + %6 = add nsw i32 %1, %2 + %7 = add nsw i32 %6, %3 + %8 = add nsw i32 %7, %4 + %9 = add nsw i32 %8, %5 + ret i32 %9 +} + + +attributes #0 = { "branch-protection-pauth-lr"="false" "sign-return-address"="all" } +attributes #1 = { "branch-protection-pauth-lr"="false" "sign-return-address"="non-leaf" } +attributes #2 = { "branch-protection-pauth-lr"="false" "sign-return-address"="none" } +attributes #3 = { "branch-protection-pauth-lr"="true" "sign-return-address"="non-leaf" } +attributes #4 = { "branch-protection-pauth-lr"="true" "sign-return-address"="non-leaf" "sign-return-address-key"="b_key" } \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/80642 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
