[llvm] [BOLT][AArch64] Implemented createDummyReturnFunction. (PR #96626)
Paschalis Mpeis via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 9 23:53:25 PDT 2024
https://github.com/paschalis-mpeis updated https://github.com/llvm/llvm-project/pull/96626
>From 5cb9b928724a9e3a92fc629a8d95e96fec21cb1c Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Tue, 25 Jun 2024 12:52:40 +0100
Subject: [PATCH 1/4] [BOLT][AArch64] Instrumentation for static binaries
doesn't work in reloc mode.
On AArch64, when trying to instrument a static binary that has relocation
data BOLT would crash as it misses `createDummyReturnFunction` function.
---
bolt/test/AArch64/dummy-return.test | 6 ++++++
bolt/test/Inputs/main.c | 3 +++
2 files changed, 9 insertions(+)
create mode 100644 bolt/test/AArch64/dummy-return.test
create mode 100644 bolt/test/Inputs/main.c
diff --git a/bolt/test/AArch64/dummy-return.test b/bolt/test/AArch64/dummy-return.test
new file mode 100644
index 0000000000000..ff8422f33bf84
--- /dev/null
+++ b/bolt/test/AArch64/dummy-return.test
@@ -0,0 +1,6 @@
+REQUIRES: system-linux
+
+RUN: %clang %p/../Inputs/main.c -o %t -Wl,-q -static
+RUN: not llvm-bolt -instrument -instrumentation-sleep-time=1 %t -o %t.instr 2>&1 | FileCheck %s
+
+CHECK: not implemented
\ No newline at end of file
diff --git a/bolt/test/Inputs/main.c b/bolt/test/Inputs/main.c
new file mode 100644
index 0000000000000..c05bedd98dba0
--- /dev/null
+++ b/bolt/test/Inputs/main.c
@@ -0,0 +1,3 @@
+// dummy function just for emitting relocations to the linker.
+int foo() { return 0; }
+int main(int argc, char **argv) { return foo() + 1; }
>From 1a1808d28347f6eecf5fb2ceb9d73ac860fe3d56 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Tue, 25 Jun 2024 12:57:37 +0100
Subject: [PATCH 2/4] [BOLT][AArch64] Implemented createDummyReturnFunction.
On AArch64, this method is needed when trying to instrument a static
binary.
Sample commands:
```bash
clang -Wl,-q test.c -static -o out
llvm-bolt -instrument -instrumentation-sleep-time=5 out -o out.instr
```
---
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 6 ++++++
bolt/test/AArch64/dummy-return.test | 8 ++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index 1a2327df1d323..e52b690e44ded 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1644,6 +1644,12 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return Instrs;
}
+ InstructionListType createDummyReturnFunction(MCContext *Ctx) const override {
+ InstructionListType Insts(1);
+ createReturn(Insts[0]);
+ return Insts;
+ }
+
std::vector<MCInst> createSymbolTrampoline(const MCSymbol *TgtSym,
MCContext *Ctx) override {
std::vector<MCInst> Insts;
diff --git a/bolt/test/AArch64/dummy-return.test b/bolt/test/AArch64/dummy-return.test
index ff8422f33bf84..76f12cf9d4f09 100644
--- a/bolt/test/AArch64/dummy-return.test
+++ b/bolt/test/AArch64/dummy-return.test
@@ -1,6 +1,10 @@
+// Tests that AArch64 is able to instrument static binaries in relocation mode.
+
REQUIRES: system-linux
RUN: %clang %p/../Inputs/main.c -o %t -Wl,-q -static
-RUN: not llvm-bolt -instrument -instrumentation-sleep-time=1 %t -o %t.instr 2>&1 | FileCheck %s
+RUN: llvm-bolt -instrument -instrumentation-sleep-time=1 %t -o %t.instr 2>&1 | FileCheck %s
+RUN: llvm-nm -n %t.instr | FileCheck %s -check-prefix=CHECK-SYM
-CHECK: not implemented
\ No newline at end of file
+CHECK: BOLT-INFO: output linked against instrumentation runtime library
+CHECK-SYM: __bolt_fini_trampoline
\ No newline at end of file
>From 77d7f795c3d1d681393bb2704611e1233e121393 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Fri, 5 Jul 2024 09:23:56 +0100
Subject: [PATCH 3/4] Refactored, improved test, and addressed reviewers.
---
bolt/include/bolt/Core/MCPlusBuilder.h | 5 ++++-
bolt/lib/Passes/Instrumentation.cpp | 2 +-
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 2 +-
bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 2 +-
bolt/test/AArch64/dummy-return.test | 9 ++++++---
bolt/test/Inputs/main.c | 2 +-
6 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index ab07f07e49845..e0a5bd986f585 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -2041,7 +2041,10 @@ class MCPlusBuilder {
return InstructionListType();
}
- virtual InstructionListType createDummyReturnFunction(MCContext *Ctx) const {
+ /// Returns a function body that contains only a return instruction. An
+ /// example usage is a workaround for the '__bolt_fini_trampoline' of
+ // Instrumentation.
+ virtual InstructionListType createDummyReturn(MCContext *Ctx) const {
llvm_unreachable("not implemented");
return InstructionListType();
}
diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp
index e824a42d82696..0af8a873790b1 100644
--- a/bolt/lib/Passes/Instrumentation.cpp
+++ b/bolt/lib/Passes/Instrumentation.cpp
@@ -754,7 +754,7 @@ void Instrumentation::createAuxiliaryFunctions(BinaryContext &BC) {
// with unknown symbol in runtime library. E.g. for static PIE
// executable
createSimpleFunction("__bolt_fini_trampoline",
- BC.MIB->createDummyReturnFunction(BC.Ctx.get()));
+ BC.MIB->createDummyReturn(BC.Ctx.get()));
}
}
}
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index e52b690e44ded..ed7d641f40e3d 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1644,7 +1644,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return Instrs;
}
- InstructionListType createDummyReturnFunction(MCContext *Ctx) const override {
+ InstructionListType createDummyReturn(MCContext *Ctx) const override {
InstructionListType Insts(1);
createReturn(Insts[0]);
return Insts;
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index 37136f4a5c551..8945ad17b657d 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -3241,7 +3241,7 @@ class X86MCPlusBuilder : public MCPlusBuilder {
return Insts;
}
- InstructionListType createDummyReturnFunction(MCContext *Ctx) const override {
+ InstructionListType createDummyReturn(MCContext *Ctx) const override {
InstructionListType Insts(1);
createReturn(Insts[0]);
return Insts;
diff --git a/bolt/test/AArch64/dummy-return.test b/bolt/test/AArch64/dummy-return.test
index 76f12cf9d4f09..d7bc1f82d641e 100644
--- a/bolt/test/AArch64/dummy-return.test
+++ b/bolt/test/AArch64/dummy-return.test
@@ -1,10 +1,13 @@
-// Tests that AArch64 is able to instrument static binaries in relocation mode.
+// Tests that AArch64 is able to instrument static binaries in relocation mode,
+// by checking that '__bolt_fini_trampoline' is generated and contains a 'ret'
+// instruction.
REQUIRES: system-linux
RUN: %clang %p/../Inputs/main.c -o %t -Wl,-q -static
RUN: llvm-bolt -instrument -instrumentation-sleep-time=1 %t -o %t.instr 2>&1 | FileCheck %s
-RUN: llvm-nm -n %t.instr | FileCheck %s -check-prefix=CHECK-SYM
+RUN: llvm-objdump --disassemble-symbols=__bolt_fini_trampoline %t.instr -D | FileCheck %s -check-prefix=CHECK-ASM
CHECK: BOLT-INFO: output linked against instrumentation runtime library
-CHECK-SYM: __bolt_fini_trampoline
\ No newline at end of file
+CHECK-ASM: <__bolt_fini_trampoline>:
+CHECK-ASM-NEXT: ret
diff --git a/bolt/test/Inputs/main.c b/bolt/test/Inputs/main.c
index c05bedd98dba0..7bfe92adb92d3 100644
--- a/bolt/test/Inputs/main.c
+++ b/bolt/test/Inputs/main.c
@@ -1,3 +1,3 @@
-// dummy function just for emitting relocations to the linker.
+// dummy function just for emitting relocations to the linker.
int foo() { return 0; }
int main(int argc, char **argv) { return foo() + 1; }
>From a1017dffaf0920bac17e82306894a55a74ff9244 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Wed, 10 Jul 2024 07:44:37 +0100
Subject: [PATCH 4/4] Addressing reviewers (1): Uplifted 'createDummyReturn' to
base class
---
bolt/include/bolt/Core/MCPlusBuilder.h | 5 +----
bolt/lib/Core/MCPlusBuilder.cpp | 6 ++++++
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 6 ------
bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 6 ------
4 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index e0a5bd986f585..b1d8d636a9f43 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -2044,10 +2044,7 @@ class MCPlusBuilder {
/// Returns a function body that contains only a return instruction. An
/// example usage is a workaround for the '__bolt_fini_trampoline' of
// Instrumentation.
- virtual InstructionListType createDummyReturn(MCContext *Ctx) const {
- llvm_unreachable("not implemented");
- return InstructionListType();
- }
+ virtual InstructionListType createDummyReturn(MCContext *Ctx) const;
/// This method takes an indirect call instruction and splits it up into an
/// equivalent set of instructions that use direct calls for target
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 7ff7a2288451c..d9281bf2e991f 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -147,6 +147,12 @@ bool MCPlusBuilder::isTailCall(const MCInst &Inst) const {
return false;
}
+InstructionListType MCPlusBuilder::createDummyReturn(MCContext *Ctx) const {
+ InstructionListType Insts(1);
+ createReturn(Insts[0]);
+ return Insts;
+}
+
std::optional<MCLandingPad> MCPlusBuilder::getEHInfo(const MCInst &Inst) const {
if (!isCall(Inst))
return std::nullopt;
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index ed7d641f40e3d..1a2327df1d323 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1644,12 +1644,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return Instrs;
}
- InstructionListType createDummyReturn(MCContext *Ctx) const override {
- InstructionListType Insts(1);
- createReturn(Insts[0]);
- return Insts;
- }
-
std::vector<MCInst> createSymbolTrampoline(const MCSymbol *TgtSym,
MCContext *Ctx) override {
std::vector<MCInst> Insts;
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index 8945ad17b657d..e46c42533031c 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -3241,12 +3241,6 @@ class X86MCPlusBuilder : public MCPlusBuilder {
return Insts;
}
- InstructionListType createDummyReturn(MCContext *Ctx) const override {
- InstructionListType Insts(1);
- createReturn(Insts[0]);
- return Insts;
- }
-
BlocksVectorTy indirectCallPromotion(
const MCInst &CallInst,
const std::vector<std::pair<MCSymbol *, uint64_t>> &Targets,
More information about the llvm-commits
mailing list