[BOLT][NFC] Simplify handleRelocation

[Amir]

Update

[rafauler]

bolt/lib/Rewrite/RewriteInstance.cpp

2509–2510

I think it's better to have BinaryFunction *BF inside the if clause, so it's clear that the scope of the BF variable does not extend past this if. Unless I'm missing something else.

[Amir]

bolt/lib/Rewrite/RewriteInstance.cpp

2509–2510

Sure. I've made the change because there's no scope difference as there's no code past the if clause (here and below with RogueBF), but let's keep it explicit that the scope is restricted to the if clause.

[Amir]

Revert definitions in if clauses

[rafauler]

LGTM

bolt/lib/Rewrite/RewriteInstance.cpp

void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
 
   // Occasionally we may see a reference past the last byte of the function
   // typically as a result of __builtin_unreachable(). Check it here.
   BinaryFunction *ReferencedBF = BC->getBinaryFunctionContainingAddress(
       Address, /*CheckPastEnd*/ true, /*UseMaxSize*/ IsAArch64);
 
   if (!IsSectionRelocation) {
     if (BinaryFunction *BF =
             BC->getBinaryFunctionContainingAddress(SymbolAddress)) {
       if (BF != ReferencedBF) {
         // It's possible we are referencing a function without referencing any
         // code, e.g. when taking a bitmask action on a function address.
         errs() << "BOLT-WARNING: non-standard function reference (e.g. bitmask)"
                << formatv(" detected against function {0} from ", *BF);
         if (IsFromCode)
           errs() << formatv("function {0}\n", *ContainingBF);
         else
           errs() << formatv("data section at {0:x}\n", Rel.getOffset());
void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
   // Workaround for a member function pointer de-virtualization bug. We check
   // if a non-pc-relative relocation in the code is pointing to (fptr - 1).
   if (IsToCode && ContainingBF && !Relocation::isPCRelative(RType) &&
       (!ReferencedBF || (ReferencedBF->getAddress() != Address))) {
     if (const BinaryFunction *RogueBF =
             BC->getBinaryFunctionAtAddress(Address + 1)) {
       // Do an extra check that the function was referenced previously.
       // It's a linear search, but it should rarely happen.
-      bool Found =
-          llvm::any_of(llvm::make_second_range(ContainingBF->Relocations),
-                       [&](const Relocation &Rel) {
-                         return Rel.Symbol == RogueBF->getSymbol() &&
-                                !Relocation::isPCRelative(Rel.Type);
-                       });
+      auto CheckReloc = [&](const Relocation &Rel) {
+      };
+      bool Found = llvm::any_of(
+          llvm::make_second_range(ContainingBF->Relocations), CheckReloc);
 
       if (Found) {
         errs() << "BOLT-WARNING: detected possible compiler de-virtualization "
                   "bug: -1 addend used with non-pc-relative relocation against "
                << formatv("function {0} in function {1}\n", *RogueBF,
                           *ContainingBF);
         return;
       }
if (BinaryData *BD = BC->getBinaryDataContainingAddress(SymbolAddress)) {
         BinaryData *BD = BC->getBinaryDataByName(ReferencedSymbol->getName());
         BC->markAmbiguousRelocations(*BD, Address);
       }
     }
   }
 
   auto checkMaxDataRelocations = [&]() {
     ++NumDataRelocations;
-    if (opts::MaxDataRelocations &&
+    LLVM_DEBUG(if (opts::MaxDataRelocations &&
                    NumDataRelocations + 1 == opts::MaxDataRelocations) {
-      LLVM_DEBUG({
-        dbgs() << "BOLT-DEBUG: processing ending on data relocation "
-               << NumDataRelocations << ": ";
-      });
       printRelocationInfo(Rel, ReferencedSymbol->getName(), SymbolAddress,
                           Addend, ExtractedValue);
-    }
+    });
 
     return (!opts::MaxDataRelocations ||
             NumDataRelocations < opts::MaxDataRelocations);
   };
 
   if ((ReferencedSection && refersToReorderedSection(ReferencedSection)) ||
       (opts::ForceToDataRelocations && checkMaxDataRelocations()))
     ForceRelocation = true;