Backport bugfix

This commit is contained in:
rfwang07 2024-06-21 11:37:58 +08:00
parent 04e2773dec
commit 2529d15deb
3 changed files with 309 additions and 1 deletions

View File

@ -0,0 +1,170 @@
From a28084a4adff2340dd02c2c0c42f4997f76b3ffa Mon Sep 17 00:00:00 2001
From: rfwang07 <wangrufeng5@huawei.com>
Date: Fri, 21 Jun 2024 11:16:44 +0800
Subject: [PATCH] [Bolt] Solving pie support issue
---
bolt/lib/Core/BinaryContext.cpp | 25 +++++++++++++++++++----
bolt/test/perf2bolt/Inputs/perf_test.c | 26 ++++++++++++++++++++++++
bolt/test/perf2bolt/Inputs/perf_test.lds | 13 ++++++++++++
bolt/test/perf2bolt/lit.local.cfg | 4 ++++
bolt/test/perf2bolt/perf_test.test | 17 ++++++++++++++++
bolt/unittests/Core/BinaryContext.cpp | 21 +++++++++++++++++++
6 files changed, 102 insertions(+), 4 deletions(-)
create mode 100644 bolt/test/perf2bolt/Inputs/perf_test.c
create mode 100644 bolt/test/perf2bolt/Inputs/perf_test.lds
create mode 100644 bolt/test/perf2bolt/lit.local.cfg
create mode 100644 bolt/test/perf2bolt/perf_test.test
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 2d2b35ee2..ab9f0b844 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1880,10 +1880,27 @@ BinaryContext::getBaseAddressForMapping(uint64_t MMapAddress,
// Find a segment with a matching file offset.
for (auto &KV : SegmentMapInfo) {
const SegmentInfo &SegInfo = KV.second;
- if (alignDown(SegInfo.FileOffset, SegInfo.Alignment) == FileOffset) {
- // Use segment's aligned memory offset to calculate the base address.
- const uint64_t MemOffset = alignDown(SegInfo.Address, SegInfo.Alignment);
- return MMapAddress - MemOffset;
+ // FileOffset is got from perf event,
+ // and it is equal to alignDown(SegInfo.FileOffset, pagesize).
+ // If the pagesize is not equal to SegInfo.Alignment.
+ // FileOffset and SegInfo.FileOffset should be aligned first,
+ // and then judge whether they are equal.
+ if (alignDown(SegInfo.FileOffset, SegInfo.Alignment) ==
+ alignDown(FileOffset, SegInfo.Alignment)) {
+ // The function's offset from base address in VAS is aligned by pagesize
+ // instead of SegInfo.Alignment. Pagesize can't be got from perf events.
+ // However, The ELF document says that SegInfo.FileOffset should equal
+ // to SegInfo.Address, modulo the pagesize.
+ // Reference: https://refspecs.linuxfoundation.org/elf/elf.pdf
+
+ // So alignDown(SegInfo.Address, pagesize) can be calculated by:
+ // alignDown(SegInfo.Address, pagesize)
+ // = SegInfo.Address - (SegInfo.Address % pagesize)
+ // = SegInfo.Address - (SegInfo.FileOffset % pagesize)
+ // = SegInfo.Address - SegInfo.FileOffset +
+ // alignDown(SegInfo.FileOffset, pagesize)
+ // = SegInfo.Address - SegInfo.FileOffset + FileOffset
+ return MMapAddress - (SegInfo.Address - SegInfo.FileOffset + FileOffset);
}
}
diff --git a/bolt/test/perf2bolt/Inputs/perf_test.c b/bolt/test/perf2bolt/Inputs/perf_test.c
new file mode 100644
index 000000000..ff5ecf7a8
--- /dev/null
+++ b/bolt/test/perf2bolt/Inputs/perf_test.c
@@ -0,0 +1,26 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int add(int a, int b) { return a + b; }
+int minus(int a, int b) { return a - b; }
+int multiple(int a, int b) { return a * b; }
+int divide(int a, int b) {
+ if (b == 0)
+ return 0;
+ return a / b;
+}
+
+int main() {
+ int a = 16;
+ int b = 8;
+
+ for (int i = 1; i < 100000; i++) {
+ add(a, b);
+ minus(a, b);
+ multiple(a, b);
+ divide(a, b);
+ }
+
+ return 0;
+}
diff --git a/bolt/test/perf2bolt/Inputs/perf_test.lds b/bolt/test/perf2bolt/Inputs/perf_test.lds
new file mode 100644
index 000000000..9cb4ebbf1
--- /dev/null
+++ b/bolt/test/perf2bolt/Inputs/perf_test.lds
@@ -0,0 +1,13 @@
+SECTIONS {
+ . = SIZEOF_HEADERS;
+ .interp : { *(.interp) }
+ .note.gnu.build-id : { *(.note.gnu.build-id) }
+ . = 0x212e8;
+ .dynsym : { *(.dynsym) }
+ . = 0x31860;
+ .text : { *(.text*) }
+ . = 0x41c20;
+ .fini_array : { *(.fini_array) }
+ . = 0x54e18;
+ .data : { *(.data) }
+}
diff --git a/bolt/test/perf2bolt/lit.local.cfg b/bolt/test/perf2bolt/lit.local.cfg
new file mode 100644
index 000000000..87a96ec34
--- /dev/null
+++ b/bolt/test/perf2bolt/lit.local.cfg
@@ -0,0 +1,4 @@
+import shutil
+
+if shutil.which("perf") != None:
+ config.available_features.add("perf")
diff --git a/bolt/test/perf2bolt/perf_test.test b/bolt/test/perf2bolt/perf_test.test
new file mode 100644
index 000000000..fe6e015ab
--- /dev/null
+++ b/bolt/test/perf2bolt/perf_test.test
@@ -0,0 +1,17 @@
+# Check perf2bolt binary function which was compiled with pie
+
+REQUIRES: system-linux, perf
+
+RUN: %clang %S/Inputs/perf_test.c -fuse-ld=lld -Wl,--script=%S/Inputs/perf_test.lds -o %t
+RUN: perf record -e cycles:u -o %t2 -- %t
+RUN: perf2bolt %t -p=%t2 -o %t3 -nl -ignore-build-id 2>&1 | FileCheck %s
+
+CHECK-NOT: PERF2BOLT-ERROR
+CHECK-NOT: !! WARNING !! This high mismatch ratio indicates the input binary is probably not the same binary used during profiling collection.
+
+RUN: %clang %S/Inputs/perf_test.c -no-pie -fuse-ld=lld -o %t4
+RUN: perf record -e cycles:u -o %t5 -- %t4
+RUN: perf2bolt %t4 -p=%t5 -o %t6 -nl -ignore-build-id 2>&1 | FileCheck %s --check-prefix=CHECK-NO-PIE
+
+CHECK-NO-PIE-NOT: PERF2BOLT-ERROR
+CHECK-NO-PIE-NOT: !! WARNING !! This high mismatch ratio indicates the input binary is probably not the same binary used during profiling collection.
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index bac264141..5a80cb4a2 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -83,3 +83,24 @@ TEST_P(BinaryContextTester, BaseAddress) {
BaseAddress = BC->getBaseAddressForMapping(0x7f13f5556000, 0x137a000);
ASSERT_FALSE(BaseAddress.has_value());
}
+
+TEST_P(BinaryContextTester, BaseAddress2) {
+ // Check that base address calculation is correct for a binary if the
+ // alignment in ELF file are different from pagesize.
+ // The segment layout is as follows:
+ BC->SegmentMapInfo[0] = SegmentInfo{0, 0x2177c, 0, 0x2177c, 0x10000};
+ BC->SegmentMapInfo[0x31860] =
+ SegmentInfo{0x31860, 0x370, 0x21860, 0x370, 0x10000};
+ BC->SegmentMapInfo[0x41c20] =
+ SegmentInfo{0x41c20, 0x1f8, 0x21c20, 0x1f8, 0x10000};
+ BC->SegmentMapInfo[0x54e18] =
+ SegmentInfo{0x54e18, 0x51, 0x24e18, 0x51, 0x10000};
+
+ std::optional<uint64_t> BaseAddress =
+ BC->getBaseAddressForMapping(0xaaaaea444000, 0x21000);
+ ASSERT_TRUE(BaseAddress.has_value());
+ ASSERT_EQ(*BaseAddress, 0xaaaaea413000ULL);
+
+ BaseAddress = BC->getBaseAddressForMapping(0xaaaaea444000, 0x11000);
+ ASSERT_FALSE(BaseAddress.has_value());
+}
--
2.39.2 (Apple Git-143)

View File

@ -0,0 +1,130 @@
From 28e7e71251dc4b79c29aa0d4904cb424f9081455 Mon Sep 17 00:00:00 2001
From: rfwang07 <wangrufeng5@huawei.com>
Date: Fri, 21 Jun 2024 11:23:42 +0800
Subject: [PATCH] [BOLT][AArch64] Don't change layout in PatchEntries
---
bolt/lib/Passes/PatchEntries.cpp | 11 ++++++++
bolt/test/AArch64/patch-entries.s | 36 ++++++++++++++++++++++++
bolt/unittests/Core/BinaryContext.cpp | 40 +++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 bolt/test/AArch64/patch-entries.s
diff --git a/bolt/lib/Passes/PatchEntries.cpp b/bolt/lib/Passes/PatchEntries.cpp
index 02a044d8b..ee7512d89 100644
--- a/bolt/lib/Passes/PatchEntries.cpp
+++ b/bolt/lib/Passes/PatchEntries.cpp
@@ -98,6 +98,17 @@ void PatchEntries::runOnFunctions(BinaryContext &BC) {
});
if (!Success) {
+ // We can't change output layout for AArch64 due to LongJmp pass
+ if (BC.isAArch64()) {
+ if (opts::ForcePatch) {
+ errs() << "BOLT-ERROR: unable to patch entries in " << Function
+ << "\n";
+ exit(1);
+ }
+
+ continue;
+ }
+
// If the original function entries cannot be patched, then we cannot
// safely emit new function body.
errs() << "BOLT-WARNING: failed to patch entries in " << Function
diff --git a/bolt/test/AArch64/patch-entries.s b/bolt/test/AArch64/patch-entries.s
new file mode 100644
index 000000000..cf6f72a0b
--- /dev/null
+++ b/bolt/test/AArch64/patch-entries.s
@@ -0,0 +1,36 @@
+# This test checks patch entries functionality
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
+# RUN: %s -o %t.o
+# RUN: %clang %cflags -pie %t.o -o %t.exe -nostdlib -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.bolt --use-old-text=0 --lite=0 --skip-funcs=_start
+# RUN: llvm-objdump -dz %t.bolt | FileCheck %s
+
+# CHECK: <pathedEntries.org.0>:
+# CHECK-NEXT: adrp x16, 0x[[#%x,ADRP:]]
+# CHECK-NEXT: add x16, x16, #0x[[#%x,ADD:]]
+# CHECK-NEXT: br x16
+
+# CHECK: [[#ADRP + ADD]] <pathedEntries>:
+# CHECK-NEXT: [[#ADRP + ADD]]: {{.*}} ret
+
+.text
+.balign 4
+.global pathedEntries
+.type pathedEntries, %function
+pathedEntries:
+ .rept 32
+ nop
+ .endr
+ ret
+.size pathedEntries, .-pathedEntries
+
+.global _start
+.type _start, %function
+_start:
+ bl pathedEntries
+ .inst 0xdeadbeef
+ ret
+.size _start, .-_start
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 5a80cb4a2..7ac1c1435 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -62,6 +62,46 @@ INSTANTIATE_TEST_SUITE_P(X86, BinaryContextTester,
INSTANTIATE_TEST_SUITE_P(AArch64, BinaryContextTester,
::testing::Values(Triple::aarch64));
+TEST_P(BinaryContextTester, FlushPendingRelocCALL26) {
+ if (GetParam() != Triple::aarch64)
+ GTEST_SKIP();
+
+ // This test checks that encodeValueAArch64 used by flushPendingRelocations
+ // returns correctly encoded values for CALL26 relocation for both backward
+ // and forward branches.
+ //
+ // The offsets layout is:
+ // 4: func1
+ // 8: bl func1
+ // 12: bl func2
+ // 16: func2
+
+ char Data[20] = {};
+ BinarySection &BS = BC->registerOrUpdateSection(
+ ".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC,
+ (uint8_t *)Data, sizeof(Data), 4);
+ MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1");
+ ASSERT_TRUE(RelSymbol1);
+ BS.addRelocation(8, RelSymbol1, ELF::R_AARCH64_CALL26, 0, 0, true);
+ MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2");
+ ASSERT_TRUE(RelSymbol2);
+ BS.addRelocation(12, RelSymbol2, ELF::R_AARCH64_CALL26, 0, 0, true);
+
+ std::error_code EC;
+ SmallVector<char> Vect(sizeof(Data));
+ raw_svector_ostream OS(Vect);
+
+ BS.flushPendingRelocations(OS, [&](const MCSymbol *S) {
+ return S == RelSymbol1 ? 4 : S == RelSymbol2 ? 16 : 0;
+ });
+
+ const uint8_t Func1Call[4] = {255, 255, 255, 151};
+ const uint8_t Func2Call[4] = {1, 0, 0, 148};
+
+ EXPECT_FALSE(memcmp(Func1Call, &Vect[8], 4)) << "Wrong backward call value\n";
+ EXPECT_FALSE(memcmp(Func2Call, &Vect[12], 4)) << "Wrong forward call value\n";
+}
+
#endif
TEST_P(BinaryContextTester, BaseAddress) {
--
2.39.2 (Apple Git-143)

View File

@ -22,7 +22,7 @@
Name: %{pkg_name}
Version: %{bolt_version}
Release: 5
Release: 6
Summary: BOLT is a post-link optimizer developed to speed up large applications
License: Apache 2.0
URL: https://github.com/llvm/llvm-project/tree/main/bolt
@ -33,6 +33,8 @@ Source1: https://github.com/llvm/llvm-project/releases/download/llvmorg-%
Patch1: 0001-Fix-trap-value-for-non-X86.patch
Patch2: 0002-Add-test-for-emitting-trap-value.patch
Patch3: 0003-AArch64-Add-AArch64-support-for-inline.patch
Patch4: 0004-Bolt-Solving-pie-support-issue.patch
Patch5: 0005-BOLT-AArch64-Don-t-change-layout-in-PatchEntries.patch
BuildRequires: gcc
BuildRequires: gcc-c++
@ -144,6 +146,12 @@ rm -f %{buildroot}/%{_builddir}/%{bolt_srcdir}/%{_vpath_builddir}/%{_lib}/lib*.a
%doc %{install_docdir}
%changelog
* Fri Jun 21 2024 rfwang07 <wangrufeng5@huawei.com> 17.0.6-6
- Type:Backport
- ID:NA
- SUG:NA
- DESC: Backport bugfix.
* Tue Jun 18 2024 Xiong Zhou <xiongzhou4@huawei.com> 17.0.6-5
- Type:Feature
- ID:NA