[PATCH] D130071: [BOLT] Adapted policy checks for stripped binaries

[Huan Nguyen via Phabricator via llvm-commits]

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

Stripped binaries do not have relocation and symbol name.
Relax sibling and entry point policy checks for stripped binaries.

Test Plan:

  ninja check-bolt


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130071

Files:
  bolt/lib/Core/BinaryContext.cpp
  bolt/lib/Core/BinaryFunction.cpp


Index: bolt/lib/Core/BinaryFunction.cpp
===================================================================
--- bolt/lib/Core/BinaryFunction.cpp
+++ bolt/lib/Core/BinaryFunction.cpp
@@ -1602,10 +1602,11 @@
     if (!getSecondaryEntryPointSymbol(Label))
       continue;
 
-    // In non-relocation mode there's potentially an external undetectable
-    // reference to the entry point and hence we cannot move this entry
-    // point. Optimizing without moving could be difficult.
-    if (!BC.HasRelocations)
+    // When strip -s, target binaries don't have .rela.text
+    // Without .rela.text, BOLT cannot reliably move code
+    // --> Due to potentially external undetectable to the code
+    // --> We relax this conservative approach for stripped binaries
+    if (!BC.IsStripped && !BC.HasRelocations)
       setSimple(false);
 
     const uint32_t Offset = KV.first;
Index: bolt/lib/Core/BinaryContext.cpp
===================================================================
--- bolt/lib/Core/BinaryContext.cpp
+++ bolt/lib/Core/BinaryContext.cpp
@@ -1119,14 +1119,19 @@
 
 bool BinaryContext::registerFragment(BinaryFunction &TargetFunction,
                                      BinaryFunction &Function) const {
-  if (!isPotentialFragmentByName(TargetFunction, Function))
+  // Only use symbol name matching for non-stripped binaries
+  if (!IsStripped && !isPotentialFragmentByName(TargetFunction, Function))
     return false;
   assert(TargetFunction.isFragment() && "TargetFunction must be a fragment");
   if (TargetFunction.isParentFragment(&Function))
     return true;
   TargetFunction.addParentFragment(Function);
   Function.addFragment(TargetFunction);
-  if (!HasRelocations) {
+  // When strip -s, target binaries don't have .rela.text
+  // Without .rela.text, BOLT cannot reliably move code
+  // --> BOLT could misidentify some other accesses to the code
+  // --> We relax this conservative approach for stripped binaries
+  if (!IsStripped && !HasRelocations) {
     TargetFunction.setSimple(false);
     Function.setSimple(false);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130071.445755.patch
Type: text/x-patch
Size: 2071 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220719/ed7362d5/attachment.bin>