[PATCH] D133978: [BOLT] Fix empty function emission in non-relocation mode

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb created this revision.
maksfb added reviewers: yota9, Amir, ayermolo, rafauler.
Herald added a subscriber: treapster.
Herald added a project: All.
maksfb requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In non-relocation mode, every function is emitted in its own section. If
a function is empty, RuntimeDyld will still allocate 1-byte section
for the function and initialize it with zero. As a result, we will
overwrite the first byte of the original function contents with zero.
Such scenario can happen when the input function had only NOP
instructions which BOLT removes by default. Even though such functions
likely cause undefined behavior, it's better to preserve their contents.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133978

Files:
  bolt/include/bolt/Core/BinaryFunction.h
  bolt/lib/Core/BinaryEmitter.cpp
  bolt/test/X86/nop-function.s


Index: bolt/test/X86/nop-function.s
===================================================================
--- /dev/null
+++ bolt/test/X86/nop-function.s
@@ -0,0 +1,30 @@
+## Check that BOLT preserves nop instruction if it's the only instruction
+## in a function.
+
+# 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 --relocs=0
+# RUN: llvm-objdump -d %t.bolt.exe | FileCheck %s
+
+  .text
+  .globl nop_function
+  .type nop_function, at function
+nop_function:
+  .cfi_startproc
+  nop
+# CHECK: <nop_function>:
+# CHECK-NEXT: nop
+
+  .size nop_function, .-nop_function
+  .cfi_endproc
+
+
+  .globl _start
+  .type _start, at function
+_start:
+  .cfi_startproc
+  call nop_function
+  .size _start, .-_start
+  .cfi_endproc
Index: bolt/lib/Core/BinaryEmitter.cpp
===================================================================
--- bolt/lib/Core/BinaryEmitter.cpp
+++ bolt/lib/Core/BinaryEmitter.cpp
@@ -287,6 +287,11 @@
   if (Function.getState() == BinaryFunction::State::Empty)
     return false;
 
+  // Avoid emitting function without instructions when overwriting the original
+  // function in-place. Otherwise, emit the empty function to define the symbol.
+  if (!BC.HasRelocations && !Function.hasNonPseudoInstructions())
+    return false;
+
   MCSection *Section =
       BC.getCodeSection(Function.getCodeSectionName(FF.getFragmentNum()));
   Streamer.switchSection(Section);
Index: bolt/include/bolt/Core/BinaryFunction.h
===================================================================
--- bolt/include/bolt/Core/BinaryFunction.h
+++ bolt/include/bolt/Core/BinaryFunction.h
@@ -1074,6 +1074,14 @@
     return N;
   }
 
+  /// Return true if function has instructions to emit.
+  bool hasNonPseudoInstructions() const {
+    for (const BinaryBasicBlock &BB : blocks())
+      if (BB.getNumNonPseudos() > 0)
+        return true;
+    return false;
+  }
+
   /// Return MC symbol associated with the function.
   /// All references to the function should use this symbol.
   MCSymbol *getSymbol(const FragmentNum Fragment = FragmentNum::main()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133978.460509.patch
Type: text/x-patch
Size: 2183 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220915/0bb5aaf3/attachment.bin>