[BOLT] Add isParentOf and isParentOrChildOf BF checks

[Amir]

Summary

Add helper methods and simplify cases where we want to check if two functions
are parent-child of each other (function-fragment relationship).

[Amir]

Remove doesBelongToFunction lambda

[rafauler]

bolt/include/bolt/Core/BinaryFunction.h

1766

sibling?

[Amir]

bolt/include/bolt/Core/BinaryFunction.h

1766

Yes, it's a bit confusing term, but it's useful in context of stripped binaries where we can't tell which function is a parent and which is a child, hence sibling (as we treat them as equal). For parent-child relationship, I don't know of a good unambiguous term: "isRelativeOf"? Kins? isParentOrChildOf? I would go with sibling unless you strongly disagree.

[rafauler]

bolt/include/bolt/Core/BinaryFunction.h

1766

I think "sibling" introduces more confusion than helps, but you can ask what's Maksim's take on this if you would like to keep sibling. After you explained, I get why you chose this name, but for someone reading this with no context, it's not clear the intent is to treat them as equals, someone might read this as if you were trying to say these two fragments have a common parent. For the lack of a better word, I would go with "isParentOrChildOf()"

[Amir]

s/isSiblingOf/isParentOrChildOf

[Amir]

Rebase

[Amir]

clang-format

bolt/include/bolt/Core/BinaryFunction.h

   /// Return true if the function is a secondary fragment of another function.
   bool isFragment() const { return IsFragment; }
 
   /// Returns if this function is a child of \p Other function.
   bool isChildOf(const BinaryFunction &Other) const {
     return ParentFragments.contains(&Other);
   }
 
+  /// Returns if this function is a parent of \p Other function.
+  bool isParentOf(const BinaryFunction &Other) const {
+    return llvm::is_contained(Fragments, &Other);
+  }
+
+  /// Returns if this function is a parent or child of \p Other function.
+  bool isParentOrChildOf(const BinaryFunction &Other) const {
+    return isChildOf(Other) || isParentOf(Other);
+  }
+
   /// Set the profile data for the number of times the function was called.
   BinaryFunction &setExecutionCount(uint64_t Count) {
     ExecutionCount = Count;
     return *this;
   }
 
   /// Adjust execution count for the function by a given \p Count. The value
   /// \p Count will be subtracted from the current function count.

bolt/lib/Core/BinaryContext.cpp

bool BinaryContext::analyzeJumpTable(const uint64_t Address,
   // Number of targets other than __builtin_unreachable.
   uint64_t NumRealEntries = 0;
 
   auto addEntryAddress = [&](uint64_t EntryAddress) {
     if (EntriesAsAddress)
       EntriesAsAddress->emplace_back(EntryAddress);
   };
 
-  auto doesBelongToFunction = [&](const uint64_t Addr,
-                                  const BinaryFunction *TargetBF) -> bool {
-    if (BF.containsAddress(Addr))
-      return true;
-    // Nothing to do if we failed to identify the containing function.
-    if (!TargetBF)
-      return false;
-    // Check if BF is a fragment of TargetBF or vice versa.
-    return BF.isChildOf(*TargetBF) || TargetBF->isChildOf(BF);
-  };
-
   ErrorOr<const BinarySection &> Section = getSectionForAddress(Address);
   if (!Section)
     return false;
 
   // The upper bound is defined by containing object, section limits, and
   // the next jump table in memory.
   uint64_t UpperBound = Section->getEndAddress();
   const BinaryData *JumpTableBD = getBinaryDataAtAddress(Address);
if (Value == BF.getAddress() + BF.getSize()) {
       HasUnreachable = true;
       LLVM_DEBUG(dbgs() << formatv("OK: {0:x} __builtin_unreachable\n", Value));
       continue;
     }
 
     // Function or one of its fragments.
     const BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Value);
 
+    bool DoesBelongToFunction = BF.containsAddress(Value) ||
+                                (TargetBF && TargetBF->isParentOrChildOf(BF));
+
     // We assume that a jump table cannot have function start as an entry.
-    if (!doesBelongToFunction(Value, TargetBF) || Value == BF.getAddress()) {
+    if (!DoesBelongToFunction || Value == BF.getAddress()) {
       LLVM_DEBUG({
         if (!BF.containsAddress(Value)) {
           dbgs() << "FAIL: function doesn't contain this address\n";
           if (TargetBF) {
             dbgs() << "  ! function containing this address: "
                    << TargetBF->getPrintName() << '\n';
             if (TargetBF->isFragment()) {
               dbgs() << "  ! is a fragment";
BinaryContext::getOrCreateJumpTable(BinaryFunction &Function, uint64_t Address,
   // Two fragments of same function access same jump table
   if (JumpTable *JT = getJumpTableContainingAddress(Address)) {
     assert(JT->Type == Type && "jump table types have to match");
     assert(Address == JT->getAddress() && "unexpected non-empty jump table");
 
     // Prevent associating a jump table to a specific fragment twice.
     // This simple check arises from the assumption: no more than 2 fragments.
     if (JT->Parents.size() == 1 && JT->Parents[0] != &Function) {
-      assert((JT->Parents[0]->isChildOf(Function) ||
-              Function.isChildOf(*JT->Parents[0])) &&
+      assert(JT->Parents[0]->isParentOrChildOf(Function) &&
              "cannot re-use jump table of a different function");
       // Duplicate the entry for the parent function for easy access
       JT->Parents.push_back(&Function);
       if (opts::Verbosity > 2) {
         outs() << "BOLT-INFO: Multiple fragments access same jump table: "
                << JT->Parents[0]->getPrintName() << "; "
                << Function.getPrintName() << "\n";
         JT->print(outs());

bolt/lib/Core/Exceptions.cpp

while (CallSitePtr < CallSiteTableEnd) {
     // Support landing pad to builtin_unreachable
     if (LPAddress < Address || LPAddress > Address + getSize()) {
       BinaryFunction *Fragment =
           BC.getBinaryFunctionContainingAddress(LPAddress);
       assert(Fragment != nullptr &&
              "BOLT-ERROR: cannot find landing pad fragment");
       BC.addInterproceduralReference(this, Fragment->getAddress());
       BC.processInterproceduralReferences();
-      assert((isChildOf(*Fragment) || Fragment->isChildOf(*this)) &&
-             "BOLT-ERROR: cannot have landing pads in different "
-             "functions");
+      assert(isParentOrChildOf(*Fragment) &&
+             "BOLT-ERROR: cannot have landing pads in different functions");
       setHasIndirectTargetToSplitFragment(true);
       BC.addFragmentsToSkip(this);
       return;
     }
 
     if (opts::PrintExceptions) {
       outs() << "Call Site: [0x" << Twine::utohexstr(RangeBase + Start)
              << ", 0x" << Twine::utohexstr(RangeBase + Start + Length)