Backport bugfix

This commit is contained in:
Xiong Zhou 2024-06-17 21:50:20 +08:00 committed by rfwang07
parent 3459d3038a
commit 7e0b429d98
3 changed files with 180 additions and 1 deletions

View File

@ -0,0 +1,126 @@
From 868d8c360b3e1e5f291cb3e0dae0777a4529228f Mon Sep 17 00:00:00 2001
From: Denis Revunov <revunov.denis@huawei-partners.com>
Date: Thu, 27 Jul 2023 11:48:08 -0400
Subject: [PATCH] Fix trap value for non-X86
The trap value used by BOLT was assumed to be single-byte instruction.
It made some functions unaligned on AArch64(e.g exceptions-instrumentation test)
and caused emission failures. Fix that by changing fill value to StringRef.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D158191
---
bolt/include/bolt/Core/MCPlusBuilder.h | 9 ++++++---
bolt/lib/Core/BinaryEmitter.cpp | 4 ++--
bolt/lib/Rewrite/RewriteInstance.cpp | 6 ++++--
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 4 ++++
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 4 ++++
bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 2 +-
6 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 56d0228cd..beb06751d 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -636,9 +636,12 @@ public:
return false;
}
- /// If non-zero, this is used to fill the executable space with instructions
- /// that will trap. Defaults to 0.
- virtual unsigned getTrapFillValue() const { return 0; }
+ /// Used to fill the executable space with instructions
+ /// that will trap.
+ virtual StringRef getTrapFillValue() const {
+ llvm_unreachable("not implemented");
+ return StringRef();
+ }
/// Interface and basic functionality of a MCInstMatcher. The idea is to make
/// it easy to match one or more MCInsts against a tree-like pattern and
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index c4129615a..df076c81d 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -376,7 +376,7 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
}
if (opts::MarkFuncs)
- Streamer.emitIntValue(BC.MIB->getTrapFillValue(), 1);
+ Streamer.emitBytes(BC.MIB->getTrapFillValue());
// Emit CFI end
if (Function.hasCFI())
@@ -420,7 +420,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
// case, the call site entries in that LSDA have 0 as offset to the landing
// pad, which the runtime interprets as "no handler". To prevent this,
// insert some padding.
- Streamer.emitIntValue(BC.MIB->getTrapFillValue(), 1);
+ Streamer.emitBytes(BC.MIB->getTrapFillValue());
}
// Track the first emitted instruction with debug info.
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index fe8c134b8..c6ea0b009 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -5273,8 +5273,10 @@ void RewriteInstance::rewriteFile() {
if (!BF.getFileOffset() || !BF.isEmitted())
continue;
OS.seek(BF.getFileOffset());
- for (unsigned I = 0; I < BF.getMaxSize(); ++I)
- OS.write((unsigned char)BC->MIB->getTrapFillValue());
+ StringRef TrapInstr = BC->MIB->getTrapFillValue();
+ unsigned NInstr = BF.getMaxSize() / TrapInstr.size();
+ for (unsigned I = 0; I < NInstr; ++I)
+ OS.write(TrapInstr.data(), TrapInstr.size());
}
OS.seek(SavedPos);
}
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index acf21ba23..cd66b654e 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1142,6 +1142,10 @@ public:
}
}
+ StringRef getTrapFillValue() const override {
+ return StringRef("\0\0\0\0", 4);
+ }
+
bool createReturn(MCInst &Inst) const override {
Inst.setOpcode(AArch64::RET);
Inst.clear();
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index ec5bca852..badc1bde8 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -171,6 +171,10 @@ public:
return true;
}
+ StringRef getTrapFillValue() const override {
+ return StringRef("\0\0\0\0", 4);
+ }
+
bool analyzeBranch(InstructionIterator Begin, InstructionIterator End,
const MCSymbol *&TBB, const MCSymbol *&FBB,
MCInst *&CondBranch,
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index 3ee161d0b..5e3c01a1c 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -397,7 +397,7 @@ public:
}
}
- unsigned getTrapFillValue() const override { return 0xCC; }
+ StringRef getTrapFillValue() const override { return StringRef("\314", 1); }
struct IndJmpMatcherFrag1 : MCInstMatcher {
std::unique_ptr<MCInstMatcher> Base;
--
2.33.0

View File

@ -0,0 +1,44 @@
From e4ae238a42296a84bc819dd1fb61f3c699952f17 Mon Sep 17 00:00:00 2001
From: Denis Revunov <rnovds@gmail.com>
Date: Thu, 17 Aug 2023 18:30:07 +0300
Subject: [PATCH] Add test for emitting trap value
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D158191
---
bolt/test/runtime/mark-funcs.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 bolt/test/runtime/mark-funcs.c
diff --git a/bolt/test/runtime/mark-funcs.c b/bolt/test/runtime/mark-funcs.c
new file mode 100644
index 000000000..a8586ca8b
--- /dev/null
+++ b/bolt/test/runtime/mark-funcs.c
@@ -0,0 +1,22 @@
+#include <stdio.h>
+
+int dummy() {
+ printf("Dummy called\n");
+ return 0;
+}
+
+int main(int argc, char **argv) {
+ if (dummy() != 0)
+ return 1;
+ printf("Main called\n");
+ return 0;
+}
+// Check that emitting trap value works properly and
+// does not break functions
+// REQUIRES: system-linux
+// RUN: %clangxx -Wl,-q %s -o %t.exe
+// RUN: %t.exe | FileCheck %s
+// CHECK: Dummy called
+// CHECK-NEXT: Main called
+// RUN: llvm-bolt %t.exe -o %t.exe.bolt -lite=false --mark-funcs
+// RUN: %t.exe.bolt | FileCheck %s
--
2.33.0

View File

@ -22,7 +22,7 @@
Name: %{pkg_name}
Version: %{bolt_version}
Release: 3
Release: 4
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
@ -30,6 +30,9 @@ URL: https://github.com/llvm/llvm-project/tree/main/bolt
Source0: https://github.com/llvm/llvm-project/releases/download/llvmorg-%{bolt_version}/%{bolt_srcdir}.tar.xz
Source1: https://github.com/llvm/llvm-project/releases/download/llvmorg-%{bolt_version}/%{bolt_srcdir}.tar.xz.sig
Patch1: 0001-Fix-trap-value-for-non-X86.patch
Patch2: 0002-Add-test-for-emitting-trap-value.patch
BuildRequires: gcc
BuildRequires: gcc-c++
BuildRequires: cmake
@ -140,6 +143,12 @@ rm -f %{buildroot}/%{_builddir}/%{bolt_srcdir}/%{_vpath_builddir}/%{_lib}/lib*.a
%doc %{install_docdir}
%changelog
* Tue Jun 18 2024 Xiong Zhou <xiongzhou4@huawei.com> 17.0.6-4
- Type:Backport
- ID:NA
- SUG:NA
- DESC: Backport bugfix.
* Tue Jun 18 2024 Xiong Zhou <xiongzhou4@huawei.com> 17.0.6-3
- Type:Update
- ID:NA