diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -358,16 +358,10 @@ /// Name for the corresponding cold code section. std::string ColdCodeSectionName; - /// Parent function fragment for split function fragments. - SmallPtrSet ParentFragments; - /// Indicate if the function body was folded into another function. /// Used by ICF optimization. BinaryFunction *FoldedIntoFunction{nullptr}; - /// All fragments for a parent function. - SmallPtrSet Fragments; - /// The profile data for the number of times the function was executed. uint64_t ExecutionCount{COUNT_NO_PROFILE}; @@ -844,6 +838,12 @@ return iterator_range(cie_begin(), cie_end()); } + /// Parent function fragment for split function fragments. + SmallPtrSet ParentFragments; + + /// All fragments for a parent function. + SmallPtrSet Fragments; + /// Iterate over all jump tables associated with this function. iterator_range::const_iterator> jumpTables() const { diff --git a/bolt/lib/Core/BinaryBasicBlock.cpp b/bolt/lib/Core/BinaryBasicBlock.cpp --- a/bolt/lib/Core/BinaryBasicBlock.cpp +++ b/bolt/lib/Core/BinaryBasicBlock.cpp @@ -101,8 +101,17 @@ // must be one of the function end labels. if (Valid) { for (const MCSymbol *Sym : UniqueSyms) { - Valid &= (Sym == Function->getFunctionEndLabel() || - Sym == Function->getFunctionColdEndLabel()); + std::unordered_set Siblings; + Siblings.insert(Function->ParentFragments.begin(), + Function->ParentFragments.end()); + Siblings.insert(Function->Fragments.begin(), Function->Fragments.end()); + Siblings.insert(Function); + + Valid = false; + for (BinaryFunction *BF : Siblings) + Valid |= (Sym == BF->getFunctionEndLabel() || + Sym == BF->getFunctionColdEndLabel()); + if (!Valid) { errs() << "BOLT-WARNING: Jump table contains illegal entry: " << Sym->getName() << "\n"; diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -503,6 +503,20 @@ BinaryFunction *&ValidExternalTargetBF) { BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Address); + BinaryFunction *PredBF = getBinaryFunctionContainingAddress(Address - 1); + + // Property 0: Entry target builtin_unreachable of sibling fragments + if (!IsStripped && PredBF != nullptr && PredBF != TargetBF) { + // Assumption: sibling fragments cannot be placed next to each other + // Builtin unreachable of same function + if (PredBF == &Function) + return true; + // Builtin unreachable of sibling fragments + else if (Function.isFragment() && registerFragment(Function, *PredBF)) + return true; + else if (PredBF->isFragment() && registerFragment(*PredBF, Function)) + return true; + } // Property 1: Entry must target a valid function body if (!TargetBF) { @@ -586,11 +600,8 @@ const uint64_t Address, const JumpTable::JumpTableType Type, BinaryFunction &BF, const uint64_t NextJTAddress, JumpTable::AddressesType *EntriesAsAddress) { - // Is one of the targets __builtin_unreachable? - bool HasUnreachable = false; - // Number of targets other than __builtin_unreachable. - uint64_t NumRealEntries = 0; + uint64_t NumEntries = 0; BinaryFunction *ValidExternalTargetBF = nullptr; auto addEntryAddress = [&](uint64_t EntryAddress) { @@ -642,21 +653,13 @@ ? Address + *getSignedValueAtAddress(EntryAddress, EntrySize) : *getPointerAtAddress(EntryAddress); - // __builtin_unreachable() case. - if (Value == BF.getAddress() + BF.getSize()) { - addEntryAddress(Value); - HasUnreachable = true; - LLVM_DEBUG(dbgs() << "OK: __builtin_unreachable\n"); - continue; - } - BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Value); // Verify if Value is a valid jump table entry if (!isValidJumpTableEntry(Value, BF, ValidExternalTargetBF)) break; - ++NumRealEntries; + ++NumEntries; if (TargetBF != nullptr && TargetBF != &BF) BF.setHasIndirectTargetToSplitFragment(true); @@ -666,7 +669,7 @@ // It's a jump table if the number of real entries is more than 1, or there's // one real entry and "unreachable" targets. If there are only multiple // "unreachable" targets, then it's not a jump table. - return NumRealEntries + HasUnreachable >= 2; + return NumEntries >= 2; } void BinaryContext::populateJumpTables() { diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -1655,35 +1655,26 @@ bool HasOneParent = (JT.Parents.size() == 1); for (unsigned I = 0; I < JT.EntriesAsAddress.size(); ++I) { uint64_t EntryAddress = JT.EntriesAsAddress[I]; - // builtin_unreachable does not belong to any function - // Need to handle separately - bool IsBuiltIn = false; - for (BinaryFunction *Parent : JT.Parents) { - if (EntryAddress == Parent->getAddress() + Parent->getSize()) { - IsBuiltIn = true; - // Specify second parameter as true to accept builtin_unreachable - MCSymbol *Label = getOrCreateLocalLabel(EntryAddress, true); + BinaryFunction *TargetBF = + BC.getBinaryFunctionContainingAddress(EntryAddress); + BinaryFunction *PredBF = + BC.getBinaryFunctionContainingAddress(EntryAddress - 1); + + // Verify builtin_unreachable + if (!BC.IsStripped && PredBF != nullptr && PredBF != TargetBF) { + if (PredBF == JT.Parents[0] || + JT.Parents[0]->isParentFragment(PredBF) || + PredBF->isParentFragment(JT.Parents[0])) { + MCSymbol *Label = PredBF->getOrCreateLocalLabel(EntryAddress, true); JT.Entries.push_back(Label); - break; + continue; } } - if (IsBuiltIn) - continue; + assert(TargetBF && "jump table entry must belong to a function"); - // Functions can be optionally skipped by users, or marked as ignored - // during branch target analysis. Since the target function is not - // processed, functions that access jump tables which point to these - // target functions also need to be marked as ignored - BinaryFunction *TargetBF = - BC.getBinaryFunctionContainingAddress(EntryAddress); - if (TargetBF->getState() != BinaryFunction::State::Disassembled || - TargetBF->isIgnored()) { - setIgnored(); - return; - } // Create local label for targets cannot be reached by other fragments - // Otherwise, register as a secondary entry point to target function + // Otherwise, secondary entry point to target function if (TargetBF->getAddress() != EntryAddress) { MCSymbol *Label = (HasOneParent && TargetBF == this)