Skip to content

Commit

Permalink
[llvm][AArch64] Autoupgrade function attributes from Module attribute…
Browse files Browse the repository at this point in the history
…s. (#80640)

`sign-return-address` and similar module attributes should be propagated
to the function level before modules got merged because module flags may
contradict and this information is not recoverable.
Generated code will match with the normal linking flow.
  • Loading branch information
DanielKristofKiss authored Feb 23, 2024
1 parent 354401f commit 531e8c2
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 8 deletions.
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/AutoUpgrade.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6706,7 +6706,7 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
}

// Look for functions that rely on old function attribute behavior.
UpgradeFunctionAttributes(*F);
UpgradeFunctionAttributes(*F, true);

// Bring in any functions that this function forward-referenced via
// blockaddresses.
Expand Down
72 changes: 71 additions & 1 deletion llvm/lib/IR/AutoUpgrade.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5155,7 +5155,46 @@ struct StrictFPUpgradeVisitor : public InstVisitor<StrictFPUpgradeVisitor> {
};
} // namespace

void llvm::UpgradeFunctionAttributes(Function &F) {
// Check if the module attribute is present and not zero.
static bool isModuleAttributeSet(const Module *M, const StringRef &ModAttr) {
const auto *Attr =
mdconst::extract_or_null<ConstantInt>(M->getModuleFlag(ModAttr));
return Attr && Attr->getZExtValue();
}

// Copy an attribute from module to the function if exists.
// First value of the pair is used when the module attribute is not zero
// the second otherwise.
static void
CopyModuleAttributeToFunction(Function &F, StringRef FnAttrName,
StringRef ModAttrName,
std::pair<StringRef, StringRef> Values) {
if (F.hasFnAttribute(FnAttrName))
return;
F.addFnAttr(FnAttrName, isModuleAttributeSet(F.getParent(), ModAttrName)
? Values.first
: Values.second);
}

// Copy a boolean attribute from module to the function if exists.
// Module attribute treated false if zero otherwise true.
static void CopyModuleAttributeToFunction(Function &F, StringRef AttrName) {
CopyModuleAttributeToFunction(
F, AttrName, AttrName,
std::make_pair<StringRef, StringRef>("true", "false"));
}

// Copy an attribute from module to the function if exists.
// First value of the pair is used when the module attribute is not zero
// the second otherwise.
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)) {
Expand All @@ -5167,6 +5206,37 @@ 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 (isModuleAttributeSet(M, "sign-return-address"))
SignType = "non-leaf";

if (isModuleAttributeSet(M, "sign-return-address-all"))
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) {
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Linker/IRMover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,10 @@ Error IRLinker::run() {
// Loop over all of the linked values to compute type mappings.
computeTypeMapping();

// Update function attributes before copying 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();
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/LTO/AArch64/link-sign-return-address.ll
Original file line number Diff line number Diff line change
@@ -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: Properties: aarch64 feature: PAC
7 changes: 4 additions & 3 deletions llvm/test/Linker/link-arm-and-thumb.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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:

0 comments on commit 531e8c2

Please sign in to comment.