[llvm] [BOLT] Fix NOP instruction emission on x86 (PR #72186)
Maksim Panchenko via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 13 17:31:10 PST 2023
https://github.com/maksfb created https://github.com/llvm/llvm-project/pull/72186
Use MCAsmBackend::writeNopData() interface to emit NOP instructions on x86. There are multiple forms of NOP instruction on x86 with different sizes. Currently, LLVM's assembly/disassembly does not support all forms correctly which can lead to a breakage of input code semantics, e.g. if the program relies on NOP instructions for reserving a patch space.
Add "--keep-nops" option to preserve NOP instructions.
>From 227609fe376df081a3fe62f0bfd06fb2b1c782e4 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Mon, 13 Nov 2023 12:31:49 -0800
Subject: [PATCH] [BOLT] Fix NOP instruction emission on x86
Use MCAsmBackend::writeNopData() interface to emit NOP instructions on
x86. There are multiple forms of NOP instruction on x86 with different
sizes. Currently, LLVM's assembly/disassembly does not support all forms
correctly which can lead to a breakage of input code semantics, e.g. if
the program relies on NOP instructions for reserving a patch space.
Add "--keep-nops" option to preserve NOP instructions.
---
bolt/include/bolt/Core/BinaryFunction.h | 2 +-
bolt/lib/Core/BinaryEmitter.cpp | 12 +++++
bolt/lib/Core/BinaryFunction.cpp | 5 ++
bolt/lib/Passes/BinaryPasses.cpp | 3 ++
bolt/lib/Utils/CommandLineOpts.cpp | 5 ++
bolt/test/X86/keep-nops.s | 69 +++++++++++++++++++++++++
6 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 bolt/test/X86/keep-nops.s
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 63c0b118db65778..b1adf2da0da3e15 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -1296,7 +1296,7 @@ class BinaryFunction {
/// Return true if the function body is non-contiguous.
bool isSplit() const { return isSimple() && getLayout().isSplit(); }
- bool shouldPreserveNops() const { return PreserveNops; }
+ bool shouldPreserveNops() const;
/// Return true if the function has exception handling tables.
bool hasEHRanges() const { return HasEHRanges; }
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 67d139d7eb5a6a7..fb1bf530c1974aa 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -507,6 +507,18 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
Streamer.emitLabel(InstrLabel);
}
+ // Emit sized NOPs via MCAsmBackend::writeNopData() interface on x86.
+ // This is a workaround for invalid NOPs handling by asm/disasm layer.
+ if (BC.MIB->isNoop(Instr) && BC.isX86()) {
+ if (std::optional<uint32_t> Size = BC.MIB->getSize(Instr)) {
+ SmallString<15> Code;
+ raw_svector_ostream VecOS(Code);
+ BC.MAB->writeNopData(VecOS, *Size, BC.STI.get());
+ Streamer.emitBytes(Code);
+ continue;
+ }
+ }
+
Streamer.emitInstruction(Instr, *BC.STI);
LastIsPrefix = BC.MIB->isPrefix(Instr);
}
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 9bbdd0deaff2d3a..1f4a7cc35247425 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -58,6 +58,7 @@ extern cl::OptionCategory BoltRelocCategory;
extern cl::opt<bool> EnableBAT;
extern cl::opt<bool> Instrument;
+extern cl::opt<bool> KeepNops;
extern cl::opt<bool> StrictMode;
extern cl::opt<bool> UpdateDebugSections;
extern cl::opt<unsigned> Verbosity;
@@ -4366,6 +4367,10 @@ MCInst *BinaryFunction::getInstructionAtOffset(uint64_t Offset) {
}
}
+bool BinaryFunction::shouldPreserveNops() const {
+ return PreserveNops || opts::KeepNops;
+}
+
void BinaryFunction::printLoopInfo(raw_ostream &OS) const {
if (!opts::shouldPrint(*this))
return;
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 83c7138e5fe5ce5..4e1343e2c30be56 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -608,12 +608,15 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) {
std::optional<uint32_t> Offset = BF->requiresAddressTranslation()
? BC.MIB->getOffset(*II)
: std::nullopt;
+ std::optional<uint32_t> Size = BC.MIB->getSize(*II);
MCSymbol *Label = BC.MIB->getLabel(*II);
BC.MIB->stripAnnotations(*II);
if (Offset)
BC.MIB->setOffset(*II, *Offset);
+ if (Size)
+ BC.MIB->setSize(*II, *Size);
if (Label)
BC.MIB->setLabel(*II, Label);
}
diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp
index a1df5de26234029..0c0e83c4ec9703b 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -129,6 +129,11 @@ cl::opt<bool>
cl::desc("instrument code to generate accurate profile data"),
cl::cat(BoltOptCategory));
+cl::opt<bool>
+ KeepNops("keep-nops",
+ cl::desc("keep no-op instructions. By default they are removed."),
+ cl::Hidden, cl::cat(BoltOptCategory));
+
cl::opt<std::string>
OutputFilename("o",
cl::desc("<output file>"),
diff --git a/bolt/test/X86/keep-nops.s b/bolt/test/X86/keep-nops.s
new file mode 100644
index 000000000000000..37da2ff07b9b798
--- /dev/null
+++ b/bolt/test/X86/keep-nops.s
@@ -0,0 +1,69 @@
+## Check that BOLT preserves NOP instructions of different sizes correctly.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t.exe -q
+# RUN: llvm-bolt %t.exe -o %t.bolt.exe --keep-nops --relocs --print-finalized \
+# RUN: |& FileCheck --check-prefix=CHECK-BOLT %s
+# RUN: llvm-objdump -d %t.bolt.exe | FileCheck %s
+
+ .text
+ .globl _start
+ .type _start, at function
+_start:
+ .cfi_startproc
+ .nops 1
+ .nops 2
+ .nops 3
+ .nops 4
+ .nops 5
+ .nops 6
+ .nops 7
+ .nops 8
+ .nops 9
+ .nops 10
+ .nops 11
+ .nops 12
+ .nops 13
+ .nops 14
+ .nops 15
+
+# CHECK: <_start>:
+# CHECK-NEXT: 90
+# CHECK-NEXT: 66 90
+# CHECK-NEXT: 0f 1f 00
+# CHECK-NEXT: 0f 1f 40 00
+# CHECK-NEXT: 0f 1f 44 00 00
+# CHECK-NEXT: 66 0f 1f 44 00 00
+# CHECK-NEXT: 0f 1f 80 00 00 00 00
+# CHECK-NEXT: 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 2e 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 66 2e 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 66 66 2e 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
+# CHECK-NEXT: 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
+
+# CHECK-BOLT: Size: 1
+# CHECK-BOLT-NEXT: Size: 2
+# CHECK-BOLT-NEXT: Size: 3
+# CHECK-BOLT-NEXT: Size: 4
+# CHECK-BOLT-NEXT: Size: 5
+# CHECK-BOLT-NEXT: Size: 6
+# CHECK-BOLT-NEXT: Size: 7
+# CHECK-BOLT-NEXT: Size: 8
+# CHECK-BOLT-NEXT: Size: 9
+# CHECK-BOLT-NEXT: Size: 10
+# CHECK-BOLT-NEXT: Size: 11
+# CHECK-BOLT-NEXT: Size: 12
+# CHECK-BOLT-NEXT: Size: 13
+# CHECK-BOLT-NEXT: Size: 14
+# CHECK-BOLT-NEXT: Size: 15
+
+# Needed for relocation mode.
+ .reloc 0, R_X86_64_NONE
+
+ .size _start, .-_start
+ .cfi_endproc
More information about the llvm-commits
mailing list