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 @@ -1884,8 +1884,12 @@ if (!EHInfo || !EHInfo->first) continue; - BinaryBasicBlock *LPBlock = getBasicBlockForLabel(EHInfo->first); - if (!BBLandingPads.count(LPBlock)) { + // If LPFunc has no CFG, LPBlock does not exist + // Solution: rerun recomputeLandingPads after buildCFG for all functions + const MCSymbol *LPLabel = EHInfo->first; + BinaryFunction *LPFunc = BC.getFunctionForSymbol(LPLabel); + BinaryBasicBlock *LPBlock = LPFunc->getBasicBlockForLabel(LPLabel); + if (LPBlock != nullptr && !BBLandingPads.count(LPBlock)) { BBLandingPads.insert(LPBlock); BB->LandingPads.emplace_back(LPBlock); LPBlock->Throwers.emplace_back(BB); diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp --- a/bolt/lib/Core/Exceptions.cpp +++ b/bolt/lib/Core/Exceptions.cpp @@ -177,12 +177,11 @@ uint64_t LPOffset = LPStart + LandingPad; uint64_t LPAddress = Address + LPOffset; + BinaryFunction *Fragment = BC.getBinaryFunctionContainingAddress(LPAddress); // Verify if landing pad code is located outside current function // 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()); @@ -196,37 +195,51 @@ "functions"); setHasIndirectTargetToSplitFragment(true); BC.addFragmentsToSkip(this); - return; } - if (opts::PrintExceptions) { - outs() << "Call Site: [0x" << Twine::utohexstr(RangeBase + Start) - << ", 0x" << Twine::utohexstr(RangeBase + Start + Length) - << "); landing pad: 0x" << Twine::utohexstr(LPOffset) - << "; action entry: 0x" << Twine::utohexstr(ActionEntry) << "\n"; - outs() << " current offset is " << (CallSitePtr - CallSiteTableStart) - << '\n'; - } + // Special case, consider builtin_unreachable as part of this function + if (LPAddress == Address + getSize()) + Fragment = this; // Create a handler entry if necessary. - MCSymbol *LPSymbol = nullptr; - if (LPOffset) { - if (!getInstructionAtOffset(LPOffset)) { + MCSymbol *LPLabel = nullptr; + + // Assumption: landing pad cannot target current fragment entry + // Note: split landing pad can target other fragment entry + if (LPAddress != Address) { + uint64_t FragmentOffset = LPAddress - Fragment->getAddress(); + if (!Fragment->getInstructionAtOffset(FragmentOffset)) { if (opts::Verbosity >= 1) errs() << "BOLT-WARNING: landing pad " << Twine::utohexstr(LPOffset) - << " not pointing to an instruction in function " << *this + << " not pointing to an instruction in function " << *Fragment << " - ignoring.\n"; } else { - auto Label = Labels.find(LPOffset); - if (Label != Labels.end()) { - LPSymbol = Label->second; - } else { - LPSymbol = BC.Ctx->createNamedTempSymbol("LP"); - Labels[LPOffset] = LPSymbol; - } + // Create or fetch landing pad label + // For landing pad in same function, create a local label + // For landing pad in a sibling fragment, register as a secondary entry + auto Label = Fragment->Labels.find(FragmentOffset); + LPLabel = (Label != Fragment->Labels.end()) + ? Label->second + : ((Fragment != this) + ? Fragment->addEntryPointAtOffset(FragmentOffset) + : BC.Ctx->createNamedTempSymbol("LP")); + // Support recomputeLandingPad to identify split landing pad + BC.setSymbolToFunctionMap(LPLabel, Fragment); + Labels[LPOffset] = LPLabel; } } + if (opts::PrintExceptions) { + outs() << "Call Site: [0x" << Twine::utohexstr(RangeBase + Start) + << ", 0x" << Twine::utohexstr(RangeBase + Start + Length) + << "); landing pad: 0x" << Twine::utohexstr(LPOffset) + << "; action entry: 0x" << Twine::utohexstr(ActionEntry) << "\n"; + outs() << " current offset is " << (CallSitePtr - CallSiteTableStart) + << '\n'; + if (LPLabel != nullptr) + outs() << " landing pad label: " << LPLabel->getName() << "\n"; + } + // Mark all call instructions in the range. auto II = Instructions.find(Start); auto IE = Instructions.end(); @@ -240,7 +253,7 @@ // Add extra operands to a call instruction making it an invoke from // now on. BC.MIB->addEHInfo(Instruction, - MCPlus::MCLandingPad(LPSymbol, ActionEntry)); + MCPlus::MCLandingPad(LPLabel, ActionEntry)); } ++II; } while (II != IE && II->first < Start + Length); diff --git a/bolt/lib/Passes/SplitFunctions.cpp b/bolt/lib/Passes/SplitFunctions.cpp --- a/bolt/lib/Passes/SplitFunctions.cpp +++ b/bolt/lib/Passes/SplitFunctions.cpp @@ -333,8 +333,11 @@ if (!EHInfo || !EHInfo->first) continue; + // In case of split landing pad, LPFunc != BF const MCSymbol *LPLabel = EHInfo->first; - BinaryBasicBlock *LPBlock = BF.getBasicBlockForLabel(LPLabel); + BinaryFunction *LPFunc = + BF.getBinaryContext().getFunctionForSymbol(LPLabel); + BinaryBasicBlock *LPBlock = LPFunc->getBasicBlockForLabel(LPLabel); if (BB->isCold() == LPBlock->isCold()) continue; @@ -346,13 +349,14 @@ // Create a trampoline basic block in the same fragment as the thrower. // Note: there's no need to insert the jump instruction, it will be // added by fixBranches(). - BinaryBasicBlock *TrampolineBB = BF.addBasicBlock(); + BinaryBasicBlock *TrampolineBB = LPFunc->addBasicBlock(); TrampolineBB->setIsCold(BB->isCold()); TrampolineBB->setExecutionCount(LPBlock->getExecutionCount()); TrampolineBB->addSuccessor(LPBlock, TrampolineBB->getExecutionCount()); TrampolineBB->setCFIState(LPBlock->getCFIState()); TrampolineLabel = TrampolineBB->getLabel(); LPTrampolines.insert(std::make_pair(LPLabel, TrampolineLabel)); + BF.getBinaryContext().setSymbolToFunctionMap(TrampolineLabel, LPFunc); } // Substitute the landing pad with the trampoline. diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -2922,9 +2922,10 @@ if (!shouldDisassemble(Function)) continue; - if (!Function.isSimple()) { - assert((!BC->HasRelocations || Function.getSize() == 0 || - Function.hasIndirectTargetToSplitFragment()) && + // Process non-simple cases due to split jump table or split landing pad + // Ignore all other non-simple cases + if (!Function.isSimple() && !Function.hasIndirectTargetToSplitFragment()) { + assert((!BC->HasRelocations || Function.getSize() == 0) && "unexpected non-simple function in relocation mode"); continue; } @@ -2978,6 +2979,18 @@ /*ForceSequential*/ opts::SequentialDisassembly || opts::PrintAll); BC->postProcessSymbolTable(); + + // recomputeLandingPads is invoked during buildCFG for every function. + // For split landing pad, it is possible that the target function does + // not yet have CFG, so recomputeLandingPads would fail to capture + // cross-function targets. + // + // Solution: + // (a) Safely ignore split landing pad during buildCFG + // (b) Rerun recomputeLandingPads after all functions have CFG + for (BinaryFunction *Function : BC->getAllBinaryFunctions()) + if (Function->getState() == BinaryFunction::State::CFG) + Function->recomputeLandingPads(); } void RewriteInstance::postProcessFunctions() { diff --git a/bolt/test/X86/split-landing-pad.s b/bolt/test/X86/split-landing-pad.s --- a/bolt/test/X86/split-landing-pad.s +++ b/bolt/test/X86/split-landing-pad.s @@ -25,10 +25,13 @@ # REQUIRES: system-linux # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o # RUN: %clang++ %cxxflags %t.o -o %t.exe -Wl,-q -# RUN: llvm-bolt -v=3 %t.exe -o %t.out 2>&1 | FileCheck %s +# RUN: llvm-bolt -v=3 %t.exe -o %t.out -print-exceptions 2>&1 | FileCheck %s -# CHECK: BOLT-WARNING: Ignoring foo -# CHECK: BOLT-WARNING: Ignoring foo.cold.1 +# CHECK: BOLT-INFO: marking [[FOO_COLD:.+]] as a fragment of [[FOO:.+]] +# CHECK: landing pad label: [[TMP2:.+]] +# CHECK: landing pad label: [[TMP5:.+]] +# CHECK: BOLT-WARNING: Ignoring [[FOO]] +# CHECK: BOLT-WARNING: Ignoring [[FOO_COLD]] # CHECK: BOLT-WARNING: skipped 2 functions due to cold fragments .text