[BOLT] Fix NOP instruction emission on x86 (#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.

Author: maksfb

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; }

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);
  }

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;

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);
  }

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>"),

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,@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