[llvm] [clang] [llvm][AArch64] Do not inline a function with different signing scheme. (PR #80642)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 00:02:04 PST 2024
https://github.com/DanielKristofKiss created https://github.com/llvm/llvm-project/pull/80642
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.
>From 2215b0400daecb3eb10040efb07a9669ef6f97ca Mon Sep 17 00:00:00 2001
From: Daniel Kiss <daniel.kiss at arm.com>
Date: Wed, 31 Jan 2024 17:54:24 +0100
Subject: [PATCH 1/3] [Clang] Replace Arch with Triplet.
---
clang/lib/CodeGen/CodeGenModule.cpp | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
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);
>From 3f4eb61edf02475c4d9a91f8e918b848add000f7 Mon Sep 17 00:00:00 2001
From: Daniel Kiss <daniel.kiss at arm.com>
Date: Mon, 29 Jan 2024 16:27:50 +0100
Subject: [PATCH 2/3] [llvm][AArch64] Autoupgrade function attributes from
Module attributes.
sign-return-address and similar module attributes should be propagated to
the function level before got merged because module flags may contradict and
this information is not recoverable.
Generated code will match with the normal linking flow.
---
llvm/include/llvm/IR/AutoUpgrade.h | 3 +-
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 2 +-
llvm/lib/IR/AutoUpgrade.cpp | 69 ++++++++++++++++++-
llvm/lib/Linker/IRMover.cpp | 4 ++
.../test/Bitcode/upgrade-arc-runtime-calls.ll | 4 +-
.../AArch64/link-branch-target-enforcement.ll | 1 +
.../LTO/AArch64/link-sign-return-address.ll | 43 ++++++++++++
llvm/test/Linker/link-arm-and-thumb.ll | 7 +-
8 files changed, 125 insertions(+), 8 deletions(-)
create mode 100644 llvm/test/LTO/AArch64/link-sign-return-address.ll
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/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:
>From 08201a9a5f5b9d55b504259b99f4b4601815dad5 Mon Sep 17 00:00:00 2001
From: Daniel Kiss <daniel.kiss at arm.com>
Date: Wed, 24 Jan 2024 16:05:07 +0100
Subject: [PATCH 3/3] [llvm][AArch64] Do not inline a function with different
signing scheme.
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.
---
llvm/lib/Transforms/Utils/InlineFunction.cpp | 15 +++
.../Inline/inline-sign-return-address.ll | 126 ++++++++++++++++++
2 files changed, 141 insertions(+)
create mode 100644 llvm/test/Transforms/Inline/inline-sign-return-address.ll
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/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
More information about the llvm-commits
mailing list