Don't touch functions whose internal BBs are targets of interprocedural branches

Summary:
In a test binary, we found 8 cases where code in a function A would jump to the
middle of another function B. In this case, we cannot reorder function B because
this would change instruction offsets and break the program. This is pretty rare
but can happen in code written in assembly.

(cherry picked from FBD2719850)

bolt/BinaryContext.h

@@ -30,6 +30,7 @@
 #include "llvm/Support/TargetRegistry.h"
 #include <functional>
 #include <map>
+#include <set>
 #include <string>
 #include <system_error>
 
@@ -51,6 +52,9 @@ public:
   // [address] -> [name1], [name2], ...
   std::multimap<uint64_t, std::string> GlobalAddresses;
 
+  // Set of addresses we cannot relocate because we have a direct branch to it.
+  std::set<uint64_t> InterproceduralBranchTargets;
+
   std::unique_ptr<MCContext> Ctx;
 
   std::unique_ptr<Triple> TheTriple;

bolt/BinaryFunction.cpp

@@ -357,7 +357,6 @@ bool BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
                          << Twine::utohexstr(AbsoluteInstrAddr)
                          << " in function " << getName() << "\n");
             IsSimple = false;
-            break;
           }
         }
 
@@ -374,6 +373,7 @@ bool BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
               TargetSymbol = LI->second;
             }
           } else {
+            BC.InterproceduralBranchTargets.insert(InstructionTarget);
             if (!IsCall && Size == 2) {
               errs() << "FLO-WARNING: relaxed tail call detected at 0x"
                      << Twine::utohexstr(AbsoluteInstrAddr)
@@ -394,7 +394,6 @@ bool BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
               errs() << "FLO-WARNING: Function \":" << getName()
                      << "\" has call to address zero. Ignoring it.\n";
               IsSimple = false;
-              break;
             }
           }
         }
@@ -419,13 +418,11 @@ bool BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
         // latter case.
         if (MIA->isIndirectBranch(Instruction)) {
           IsSimple = false;
-          break;
         }
         // Indirect call. We only need to fix it if the operand is RIP-relative
         if (MIA->hasRIPOperand(Instruction)) {
           if (!handleRIPOperand(Instruction, AbsoluteInstrAddr, Size)) {
             IsSimple = false;
-            break;
           }
         }
       }
@@ -433,7 +430,6 @@ bool BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
       if (MIA->hasRIPOperand(Instruction)) {
         if (!handleRIPOperand(Instruction, AbsoluteInstrAddr, Size)) {
           IsSimple = false;
-          break;
         }
       }
     }

bolt/RewriteInstance.cpp

@@ -651,6 +651,25 @@ void RewriteInstance::disassembleFunctions() {
     TotalScore += Function.getFunctionScore();
 
   } // Iterate over all functions
+
+  // Mark all functions with internal addresses serving as interprocedural
+  // branch targets as not simple --  pretty rare but can happen in code
+  // written in assembly.
+  // TODO: #9301815
+  for (auto Addr : BC->InterproceduralBranchTargets) {
+    // Check if this address is internal to some function we are reordering
+    auto I = BinaryFunctions.upper_bound(Addr);
+    if (I == BinaryFunctions.begin())
+      continue;
+    BinaryFunction &Func = (--I)->second;
+    uint64_t Offset = Addr - I->first;
+    if (Offset == 0 || Offset >= Func.getSize())
+      continue;
+    errs() << "FLO-WARNING: Function " << Func.getName()
+           << " has internal BBs that are target of a branch located in "
+              "another function. We will not process this function.\n";
+    Func.setSimple(false);
+  }
 }
 
 void RewriteInstance::runOptimizationPasses() {