diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h index f1db1fbded6a4..5dc5fdb150be2 100644 --- a/bolt/include/bolt/Core/BinaryContext.h +++ b/bolt/include/bolt/Core/BinaryContext.h @@ -145,6 +145,29 @@ class FilterIterator { } }; +/// BOLT-exclusive errors generated in core BOLT libraries, optionally holding a +/// string message and whether it is fatal or not. In case it is fatal and if +/// BOLT is running as a standalone process, the process might be killed as soon +/// as the error is checked. +class BOLTError : public ErrorInfo { +public: + static char ID; + + BOLTError(bool IsFatal, const Twine &S = Twine()); + void log(raw_ostream &OS) const override; + bool isFatal() const { return IsFatal; } + + const std::string &getMessage() const { return Msg; } + std::error_code convertToErrorCode() const override; + +private: + bool IsFatal; + std::string Msg; +}; + +Error createNonFatalBOLTError(const Twine &S); +Error createFatalBOLTError(const Twine &S); + class BinaryContext { BinaryContext() = delete; diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 3a1eae3311bd7..a177178769e45 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1910,12 +1910,11 @@ class BinaryFunction { /// Support dynamic relocations in constant islands, which may happen if /// binary is linked with -z notext option. - void markIslandDynamicRelocationAtAddress(uint64_t Address) { - if (!isInConstantIsland(Address)) { - errs() << "BOLT-ERROR: dynamic relocation found for text section at 0x" - << Twine::utohexstr(Address) << "\n"; - exit(1); - } + Error markIslandDynamicRelocationAtAddress(uint64_t Address) { + if (!isInConstantIsland(Address)) + return createFatalBOLTError( + Twine("dynamic relocation found for text section at 0x") + + Twine::utohexstr(Address) + Twine("\n")); // Mark island to have dynamic relocation Islands->HasDynamicRelocations = true; @@ -1924,6 +1923,7 @@ class BinaryFunction { // move binary data during updateOutputValues, making us emit // dynamic relocation with the right offset value. getOrCreateIslandAccess(Address); + return Error::success(); } bool hasDynamicRelocationAtIsland() const { @@ -2054,9 +2054,10 @@ class BinaryFunction { /// state to State:Disassembled. /// /// Returns false if disassembly failed. - bool disassemble(); + Error disassemble(); - void handlePCRelOperand(MCInst &Instruction, uint64_t Address, uint64_t Size); + Error handlePCRelOperand(MCInst &Instruction, uint64_t Address, + uint64_t Size); MCSymbol *handleExternalReference(MCInst &Instruction, uint64_t Size, uint64_t Offset, uint64_t TargetAddress, @@ -2100,7 +2101,7 @@ class BinaryFunction { /// /// Returns true on success and update the current function state to /// State::CFG. Returns false if CFG cannot be built. - bool buildCFG(MCPlusBuilder::AllocatorIdTy); + Error buildCFG(MCPlusBuilder::AllocatorIdTy); /// Perform post-processing of the CFG. void postProcessCFG(); @@ -2217,7 +2218,7 @@ class BinaryFunction { } /// Process LSDA information for the function. - void parseLSDA(ArrayRef LSDAData, uint64_t LSDAAddress); + Error parseLSDA(ArrayRef LSDAData, uint64_t LSDAAddress); /// Update exception handling ranges for the function. void updateEHRanges(); diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index 70914f59157d2..a85dbf28950e3 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -112,7 +112,7 @@ class BinarySection { static StringRef getName(SectionRef Section) { return cantFail(Section.getName()); } - static StringRef getContents(SectionRef Section) { + static StringRef getContentsOrQuit(SectionRef Section) { if (Section.getObject()->isELF() && ELFSectionRef(Section).getType() == ELF::SHT_NOBITS) return StringRef(); @@ -159,7 +159,7 @@ class BinarySection { BinarySection(BinaryContext &BC, SectionRef Section) : BC(BC), Name(getName(Section)), Section(Section), - Contents(getContents(Section)), Address(Section.getAddress()), + Contents(getContentsOrQuit(Section)), Address(Section.getAddress()), Size(Section.getSize()), Alignment(Section.getAlignment().value()), OutputName(Name), SectionNumber(++Count) { if (isELF()) { diff --git a/bolt/include/bolt/Passes/FrameOptimizer.h b/bolt/include/bolt/Passes/FrameOptimizer.h index a0d93c003d9e5..64055bd251729 100644 --- a/bolt/include/bolt/Passes/FrameOptimizer.h +++ b/bolt/include/bolt/Passes/FrameOptimizer.h @@ -98,8 +98,8 @@ class FrameOptimizerPass : public BinaryFunctionPass { void removeUnusedStores(const FrameAnalysis &FA, BinaryFunction &BF); /// Perform shrinkwrapping step - void performShrinkWrapping(const RegAnalysis &RA, const FrameAnalysis &FA, - BinaryContext &BC); + Error performShrinkWrapping(const RegAnalysis &RA, const FrameAnalysis &FA, + BinaryContext &BC); public: explicit FrameOptimizerPass(const cl::opt &PrintPass) diff --git a/bolt/include/bolt/Passes/LongJmp.h b/bolt/include/bolt/Passes/LongJmp.h index 94883370196d6..3d02d75ac4a27 100644 --- a/bolt/include/bolt/Passes/LongJmp.h +++ b/bolt/include/bolt/Passes/LongJmp.h @@ -131,14 +131,14 @@ class LongJmpPass : public BinaryFunctionPass { uint64_t DotAddress) const; /// Expand the range of the stub in StubBB if necessary - bool relaxStub(BinaryBasicBlock &StubBB); + Error relaxStub(BinaryBasicBlock &StubBB, bool &Modified); /// Helper to resolve a symbol address according to our tentative layout uint64_t getSymbolAddress(const BinaryContext &BC, const MCSymbol *Target, const BinaryBasicBlock *TgtBB) const; /// Relax function by adding necessary stubs or relaxing existing stubs - bool relax(BinaryFunction &BF); + Error relax(BinaryFunction &BF, bool &Modified); public: /// BinaryPass public interface diff --git a/bolt/include/bolt/Passes/ReorderFunctions.h b/bolt/include/bolt/Passes/ReorderFunctions.h index 6ca2dc3391733..b75937da2314d 100644 --- a/bolt/include/bolt/Passes/ReorderFunctions.h +++ b/bolt/include/bolt/Passes/ReorderFunctions.h @@ -44,7 +44,7 @@ class ReorderFunctions : public BinaryFunctionPass { const char *getName() const override { return "reorder-functions"; } Error runOnFunctions(BinaryContext &BC) override; - static std::vector readFunctionOrderFile(); + static Error readFunctionOrderFile(std::vector &FunctionNames); }; } // namespace bolt diff --git a/bolt/include/bolt/Passes/ShrinkWrapping.h b/bolt/include/bolt/Passes/ShrinkWrapping.h index cccbc518dd26b..016bea75f649a 100644 --- a/bolt/include/bolt/Passes/ShrinkWrapping.h +++ b/bolt/include/bolt/Passes/ShrinkWrapping.h @@ -467,8 +467,9 @@ class ShrinkWrapping { /// If \p CreatePushOrPop is true, create a push/pop instead. Current SP/FP /// values, as determined by StackPointerTracking, should be informed via /// \p SPVal and \p FPVal in order to emit the correct offset form SP/FP. - MCInst createStackAccess(int SPVal, int FPVal, const FrameIndexEntry &FIE, - bool CreatePushOrPop); + Expected createStackAccess(int SPVal, int FPVal, + const FrameIndexEntry &FIE, + bool CreatePushOrPop); /// Update the CFI referenced by \p Inst with \p NewOffset, if the CFI has /// an offset. @@ -484,22 +485,23 @@ class ShrinkWrapping { /// InsertionPoint for other instructions that need to be inserted at the same /// original location, since this insertion may have invalidated the previous /// location. - BBIterTy processInsertion(BBIterTy InsertionPoint, BinaryBasicBlock *CurBB, - const WorklistItem &Item, int64_t SPVal, - int64_t FPVal); + Expected processInsertion(BBIterTy InsertionPoint, + BinaryBasicBlock *CurBB, + const WorklistItem &Item, int64_t SPVal, + int64_t FPVal); /// Auxiliary function to processInsertions(), helping perform all the /// insertion tasks in the todo list associated with a single insertion point. /// Return true if at least one insertion was performed. - BBIterTy processInsertionsList(BBIterTy InsertionPoint, - BinaryBasicBlock *CurBB, - std::vector &TodoList, - int64_t SPVal, int64_t FPVal); + Expected processInsertionsList(BBIterTy InsertionPoint, + BinaryBasicBlock *CurBB, + std::vector &TodoList, + int64_t SPVal, int64_t FPVal); /// Apply all insertion todo tasks regarding insertion of new stores/loads or /// push/pops at annotated points. Return false if the entire function had /// no todo tasks annotation and this pass has nothing to do. - bool processInsertions(); + Expected processInsertions(); /// Apply all deletion todo tasks (or tasks to change a push/pop to a memory /// access no-op) @@ -519,7 +521,7 @@ class ShrinkWrapping { BC.MIB->removeAnnotation(Inst, getAnnotationIndex()); } - bool perform(bool HotOnly = false); + Expected perform(bool HotOnly = false); static void printStats(); }; diff --git a/bolt/include/bolt/Rewrite/BinaryPassManager.h b/bolt/include/bolt/Rewrite/BinaryPassManager.h index 84ab192c415a4..2297c3bf52fd6 100644 --- a/bolt/include/bolt/Rewrite/BinaryPassManager.h +++ b/bolt/include/bolt/Rewrite/BinaryPassManager.h @@ -46,10 +46,10 @@ class BinaryFunctionPassManager { } /// Run all registered passes in the order they were added. - void runPasses(); + Error runPasses(); /// Runs all enabled implemented passes on all functions. - static void runAllPasses(BinaryContext &BC); + static Error runAllPasses(BinaryContext &BC); }; } // namespace bolt diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp index df835d2876804..1c33544f40a11 100644 --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -83,6 +83,37 @@ cl::opt CompDirOverride( namespace llvm { namespace bolt { +char BOLTError::ID = 0; + +BOLTError::BOLTError(bool IsFatal, const Twine &S) + : IsFatal(IsFatal), Msg(S.str()) {} + +void BOLTError::log(raw_ostream &OS) const { + if (IsFatal) + OS << "FATAL "; + StringRef ErrMsg = StringRef(Msg); + // Prepend our error prefix if it is missing + if (ErrMsg.empty()) { + OS << "BOLT-ERROR\n"; + } else { + if (!ErrMsg.starts_with("BOLT-ERROR")) + OS << "BOLT-ERROR: "; + OS << ErrMsg << "\n"; + } +} + +std::error_code BOLTError::convertToErrorCode() const { + return inconvertibleErrorCode(); +} + +Error createNonFatalBOLTError(const Twine &S) { + return make_error(/*IsFatal*/ false, S); +} + +Error createFatalBOLTError(const Twine &S) { + return make_error(/*IsFatal*/ true, S); +} + BinaryContext::BinaryContext(std::unique_ptr Ctx, std::unique_ptr DwCtx, std::unique_ptr TheTriple, diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 0ac47a53a4467..3f3da3b800410 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -1021,24 +1021,25 @@ bool BinaryFunction::isZeroPaddingAt(uint64_t Offset) const { return true; } -void BinaryFunction::handlePCRelOperand(MCInst &Instruction, uint64_t Address, - uint64_t Size) { +Error BinaryFunction::handlePCRelOperand(MCInst &Instruction, uint64_t Address, + uint64_t Size) { auto &MIB = BC.MIB; uint64_t TargetAddress = 0; if (!MIB->evaluateMemOperandTarget(Instruction, TargetAddress, Address, Size)) { - errs() << "BOLT-ERROR: PC-relative operand can't be evaluated:\n"; - BC.InstPrinter->printInst(&Instruction, 0, "", *BC.STI, errs()); - errs() << '\n'; - Instruction.dump_pretty(errs(), BC.InstPrinter.get()); - errs() << '\n'; - errs() << "BOLT-ERROR: cannot handle PC-relative operand at 0x" - << Twine::utohexstr(Address) << ". Skipping function " << *this - << ".\n"; + std::string Msg; + raw_string_ostream SS(Msg); + SS << "BOLT-ERROR: PC-relative operand can't be evaluated:\n"; + BC.InstPrinter->printInst(&Instruction, 0, "", *BC.STI, SS); + SS << '\n'; + Instruction.dump_pretty(SS, BC.InstPrinter.get()); + SS << '\n'; + SS << "BOLT-ERROR: cannot handle PC-relative operand at 0x" + << Twine::utohexstr(Address) << ". Skipping function " << *this << ".\n"; if (BC.HasRelocations) - exit(1); + return createFatalBOLTError(Msg); IsSimple = false; - return; + return createNonFatalBOLTError(Msg); } if (TargetAddress == 0 && opts::Verbosity >= 1) { outs() << "BOLT-INFO: PC-relative operand is zero in function " << *this @@ -1054,6 +1055,7 @@ void BinaryFunction::handlePCRelOperand(MCInst &Instruction, uint64_t Address, Instruction, TargetSymbol, static_cast(TargetOffset), &*BC.Ctx); (void)ReplaceSuccess; assert(ReplaceSuccess && "Failed to replace mem operand with symbol+off."); + return Error::success(); } MCSymbol *BinaryFunction::handleExternalReference(MCInst &Instruction, @@ -1164,7 +1166,7 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst &Instruction, } } -bool BinaryFunction::disassemble() { +Error BinaryFunction::disassemble() { NamedRegionTimer T("disassemble", "Disassemble function", "buildfuncs", "Build Binary Functions", opts::TimeBuild); ErrorOr> ErrorOrFunctionData = getData(); @@ -1332,8 +1334,19 @@ bool BinaryFunction::disassemble() { if (MIB->isIndirectBranch(Instruction)) handleIndirectBranch(Instruction, Size, Offset); // Indirect call. We only need to fix it if the operand is RIP-relative. - if (IsSimple && MIB->hasPCRelOperand(Instruction)) - handlePCRelOperand(Instruction, AbsoluteInstrAddr, Size); + if (IsSimple && MIB->hasPCRelOperand(Instruction)) { + if (auto NewE = handleErrors( + handlePCRelOperand(Instruction, AbsoluteInstrAddr, Size), + [&](const BOLTError &E) -> Error { + if (E.isFatal()) + return Error(std::make_unique(std::move(E))); + if (!E.getMessage().empty()) + E.log(errs()); + return Error::success(); + })) { + return Error(std::move(NewE)); + } + } if (BC.isAArch64()) handleAArch64IndirectCall(Instruction, Offset); @@ -1372,8 +1385,18 @@ bool BinaryFunction::disassemble() { UsedReloc = true; } - if (!BC.isRISCV() && MIB->hasPCRelOperand(Instruction) && !UsedReloc) - handlePCRelOperand(Instruction, AbsoluteInstrAddr, Size); + if (!BC.isRISCV() && MIB->hasPCRelOperand(Instruction) && !UsedReloc) { + if (auto NewE = handleErrors( + handlePCRelOperand(Instruction, AbsoluteInstrAddr, Size), + [&](const BOLTError &E) -> Error { + if (E.isFatal()) + return Error(std::make_unique(std::move(E))); + if (!E.getMessage().empty()) + E.log(errs()); + return Error::success(); + })) + return Error(std::move(NewE)); + } } add_instruction: @@ -1413,12 +1436,12 @@ bool BinaryFunction::disassemble() { if (!IsSimple) { clearList(Instructions); - return false; + return createNonFatalBOLTError(""); } updateState(State::Disassembled); - return true; + return Error::success(); } bool BinaryFunction::scanExternalRefs() { @@ -1946,17 +1969,17 @@ void BinaryFunction::recomputeLandingPads() { } } -bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { +Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { auto &MIB = BC.MIB; if (!isSimple()) { assert(!BC.HasRelocations && "cannot process file with non-simple function in relocs mode"); - return false; + return createNonFatalBOLTError(""); } if (CurrentState != State::Disassembled) - return false; + return createNonFatalBOLTError(""); assert(BasicBlocks.empty() && "basic block list should be empty"); assert((Labels.find(getFirstInstructionOffset()) != Labels.end()) && @@ -2093,7 +2116,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { if (BasicBlocks.empty()) { setSimple(false); - return false; + return createNonFatalBOLTError(""); } // Intermediate dump. @@ -2204,7 +2227,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { clearList(ExternallyReferencedOffsets); clearList(UnknownIndirectBranchOffsets); - return true; + return Error::success(); } void BinaryFunction::postProcessCFG() { diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp index 97bc251935475..564c63e81914c 100644 --- a/bolt/lib/Core/BinarySection.cpp +++ b/bolt/lib/Core/BinarySection.cpp @@ -198,7 +198,7 @@ BinarySection::~BinarySection() { if (!isAllocatable() && !hasValidSectionID() && (!hasSectionRef() || - OutputContents.data() != getContents(Section).data())) { + OutputContents.data() != getContentsOrQuit(Section).data())) { delete[] getOutputData(); } } diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp index ab1885f6bb585..dd3fa46b6f128 100644 --- a/bolt/lib/Core/Exceptions.cpp +++ b/bolt/lib/Core/Exceptions.cpp @@ -98,12 +98,12 @@ namespace bolt { // site table will be the same size as GCC uses uleb encodings for PC offsets. // // Note: some functions have LSDA entries with 0 call site entries. -void BinaryFunction::parseLSDA(ArrayRef LSDASectionData, - uint64_t LSDASectionAddress) { +Error BinaryFunction::parseLSDA(ArrayRef LSDASectionData, + uint64_t LSDASectionAddress) { assert(CurrentState == State::Disassembled && "unexpected function state"); if (!getLSDAAddress()) - return; + return Error::success(); DWARFDataExtractor Data( StringRef(reinterpret_cast(LSDASectionData.data()), @@ -121,7 +121,7 @@ void BinaryFunction::parseLSDA(ArrayRef LSDASectionData, if (!MaybeLPStart) { errs() << "BOLT-ERROR: unsupported LPStartEncoding: " << (unsigned)LPStartEncoding << '\n'; - exit(1); + return createFatalBOLTError(""); } LPStart = *MaybeLPStart; } @@ -209,7 +209,7 @@ void BinaryFunction::parseLSDA(ArrayRef LSDASectionData, "BOLT-ERROR: cannot have landing pads in different functions"); setHasIndirectTargetToSplitFragment(true); BC.addFragmentsToSkip(this); - return; + return Error::success(); } const uint64_t LPOffset = LandingPad - getAddress(); @@ -354,6 +354,7 @@ void BinaryFunction::parseLSDA(ArrayRef LSDASectionData, LSDATypeIndexTable = LSDASectionData.slice(TypeIndexTableStart, MaxTypeIndexTableOffset); } + return Error::success(); } void BinaryFunction::updateEHRanges() { diff --git a/bolt/lib/Passes/ADRRelaxationPass.cpp b/bolt/lib/Passes/ADRRelaxationPass.cpp index 681b5ab688e0d..aa715a3f37871 100644 --- a/bolt/lib/Passes/ADRRelaxationPass.cpp +++ b/bolt/lib/Passes/ADRRelaxationPass.cpp @@ -110,7 +110,7 @@ Error ADRRelaxationPass::runOnFunctions(BinaryContext &BC) { "ADRRelaxationPass"); if (PassFailed) - exit(1); + return createFatalBOLTError(""); return Error::success(); } diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp index e90f01acf93b8..4d92b27462513 100644 --- a/bolt/lib/Passes/BinaryPasses.cpp +++ b/bolt/lib/Passes/BinaryPasses.cpp @@ -528,12 +528,14 @@ Error FixupBranches::runOnFunctions(BinaryContext &BC) { } Error FinalizeFunctions::runOnFunctions(BinaryContext &BC) { + std::atomic HasFatal{false}; ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) { if (!BF.finalizeCFIState()) { if (BC.HasRelocations) { errs() << "BOLT-ERROR: unable to fix CFI state for function " << BF << ". Exiting.\n"; - exit(1); + HasFatal = true; + return; } BF.setSimple(false); return; @@ -552,6 +554,8 @@ Error FinalizeFunctions::runOnFunctions(BinaryContext &BC) { ParallelUtilities::runOnEachFunction( BC, ParallelUtilities::SchedulingPolicy::SP_CONSTANT, WorkFun, SkipPredicate, "FinalizeFunctions"); + if (HasFatal) + return createFatalBOLTError("finalize CFI state failure"); return Error::success(); } @@ -1452,9 +1456,9 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) { << format(" (%.1f%% of all stale)", PctStaleBlocksWithEqualIcount) << " have matching icount.\n"; if (PctStale > opts::StaleThreshold) { - errs() << "BOLT-ERROR: stale functions exceed specified threshold of " - << opts::StaleThreshold << "%. Exiting.\n"; - exit(1); + return createFatalBOLTError( + Twine("BOLT-ERROR: stale functions exceed specified threshold of ") + + Twine(opts::StaleThreshold.getValue()) + Twine("%. Exiting.\n")); } } if (NumInferredFunctions) { diff --git a/bolt/lib/Passes/FrameOptimizer.cpp b/bolt/lib/Passes/FrameOptimizer.cpp index 83a5a36ae0255..30bcfb9084e9c 100644 --- a/bolt/lib/Passes/FrameOptimizer.cpp +++ b/bolt/lib/Passes/FrameOptimizer.cpp @@ -285,7 +285,8 @@ Error FrameOptimizerPass::runOnFunctions(BinaryContext &BC) { { NamedRegionTimer T1("shrinkwrapping", "shrink wrapping", "FOP", "FOP breakdown", opts::TimeOpts); - performShrinkWrapping(*RA, *FA, BC); + if (Error E = performShrinkWrapping(*RA, *FA, BC)) + return Error(std::move(E)); } outs() << "BOLT-INFO: FOP optimized " << NumRedundantLoads @@ -306,9 +307,9 @@ Error FrameOptimizerPass::runOnFunctions(BinaryContext &BC) { return Error::success(); } -void FrameOptimizerPass::performShrinkWrapping(const RegAnalysis &RA, - const FrameAnalysis &FA, - BinaryContext &BC) { +Error FrameOptimizerPass::performShrinkWrapping(const RegAnalysis &RA, + const FrameAnalysis &FA, + BinaryContext &BC) { // Initialize necessary annotations to allow safe parallel accesses to // annotation index in MIB BC.MIB->getOrCreateAnnotationIndex(CalleeSavedAnalysis::getSaveTagName()); @@ -358,12 +359,21 @@ void FrameOptimizerPass::performShrinkWrapping(const RegAnalysis &RA, const bool HotOnly = opts::FrameOptimization == FOP_HOT; + Error SWError = Error::success(); + ParallelUtilities::WorkFuncWithAllocTy WorkFunction = [&](BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocatorId) { DataflowInfoManager Info(BF, &RA, &FA, AllocatorId); ShrinkWrapping SW(FA, BF, Info, AllocatorId); - if (SW.perform(HotOnly)) { + auto ChangedOrErr = SW.perform(HotOnly); + if (auto E = ChangedOrErr.takeError()) { + std::lock_guard Lock(FuncsChangedMutex); + SWError = joinErrors(std::move(SWError), Error(std::move(E))); + return; + } + const bool Changed = *ChangedOrErr; + if (Changed) { std::lock_guard Lock(FuncsChangedMutex); FuncsChanged.insert(&BF); LLVM_DEBUG(LogFunc(BF)); @@ -379,6 +389,7 @@ void FrameOptimizerPass::performShrinkWrapping(const RegAnalysis &RA, for (const auto &Elmt : Top10Funcs) outs() << Elmt.first << " : " << Elmt.second->getPrintName() << "\n"; } + return SWError; } } // namespace bolt diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp index b6dc7705bf61a..26b4a67fec422 100644 --- a/bolt/lib/Passes/Instrumentation.cpp +++ b/bolt/lib/Passes/Instrumentation.cpp @@ -567,10 +567,9 @@ Error Instrumentation::runOnFunctions(BinaryContext &BC) { ErrorOr SetupSection = BC.getUniqueSectionByName("I__setup"); - if (!SetupSection) { - llvm::errs() << "Cannot find I__setup section\n"; - exit(1); - } + if (!SetupSection) + return createFatalBOLTError("Cannot find I__setup section\n"); + MCSymbol *Target = BC.registerNameAtAddress( "__bolt_instr_setup", SetupSection->getAddress(), 0, 0); MCInst NewInst; @@ -586,10 +585,9 @@ Error Instrumentation::runOnFunctions(BinaryContext &BC) { BinaryBasicBlock &BB = Ctor->front(); ErrorOr FiniSection = BC.getUniqueSectionByName("I__fini"); - if (!FiniSection) { - llvm::errs() << "Cannot find I__fini section\n"; - exit(1); - } + if (!FiniSection) + return createFatalBOLTError("Cannot find I__fini section"); + MCSymbol *Target = BC.registerNameAtAddress( "__bolt_instr_fini", FiniSection->getAddress(), 0, 0); auto IsLEA = [&BC](const MCInst &Inst) { return BC.MIB->isLEA64r(Inst); }; diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index b524339c33a3c..72823b9c193d3 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -459,13 +459,13 @@ uint64_t LongJmpPass::getSymbolAddress(const BinaryContext &BC, return Iter->second; } -bool LongJmpPass::relaxStub(BinaryBasicBlock &StubBB) { +Error LongJmpPass::relaxStub(BinaryBasicBlock &StubBB, bool &Modified) { const BinaryFunction &Func = *StubBB.getFunction(); const BinaryContext &BC = Func.getBinaryContext(); const int Bits = StubBits[&StubBB]; // Already working with the largest range? if (Bits == static_cast(BC.AsmInfo->getCodePointerSize() * 8)) - return false; + return Error::success(); const static int RangeShortJmp = BC.MIB->getShortJmpEncodingSize(); const static int RangeSingleInstr = BC.MIB->getUncondBranchEncodingSize(); @@ -481,12 +481,12 @@ bool LongJmpPass::relaxStub(BinaryBasicBlock &StubBB) { : TgtAddress - DotAddress; // If it fits in one instruction, do not relax if (!(PCRelTgtAddress & SingleInstrMask)) - return false; + return Error::success(); // Fits short jmp if (!(PCRelTgtAddress & ShortJmpMask)) { if (Bits >= RangeShortJmp) - return false; + return Error::success(); LLVM_DEBUG(dbgs() << "Relaxing stub to short jump. PCRelTgtAddress = " << Twine::utohexstr(PCRelTgtAddress) @@ -494,22 +494,23 @@ bool LongJmpPass::relaxStub(BinaryBasicBlock &StubBB) { << "\n"); relaxStubToShortJmp(StubBB, RealTargetSym); StubBits[&StubBB] = RangeShortJmp; - return true; + Modified = true; + return Error::success(); } // The long jmp uses absolute address on AArch64 // So we could not use it for PIC binaries - if (BC.isAArch64() && !BC.HasFixedLoadAddress) { - errs() << "BOLT-ERROR: Unable to relax stub for PIC binary\n"; - exit(1); - } + if (BC.isAArch64() && !BC.HasFixedLoadAddress) + return createFatalBOLTError( + "BOLT-ERROR: Unable to relax stub for PIC binary\n"); LLVM_DEBUG(dbgs() << "Relaxing stub to long jump. PCRelTgtAddress = " << Twine::utohexstr(PCRelTgtAddress) << " RealTargetSym = " << RealTargetSym->getName() << "\n"); relaxStubToLongJmp(StubBB, RealTargetSym); StubBits[&StubBB] = static_cast(BC.AsmInfo->getCodePointerSize() * 8); - return true; + Modified = true; + return Error::success(); } bool LongJmpPass::needsStub(const BinaryBasicBlock &BB, const MCInst &Inst, @@ -539,9 +540,8 @@ bool LongJmpPass::needsStub(const BinaryBasicBlock &BB, const MCInst &Inst, return PCOffset < MinVal || PCOffset > MaxVal; } -bool LongJmpPass::relax(BinaryFunction &Func) { +Error LongJmpPass::relax(BinaryFunction &Func, bool &Modified) { const BinaryContext &BC = Func.getBinaryContext(); - bool Modified = false; assert(BC.isAArch64() && "Unsupported arch"); constexpr int InsnSize = 4; // AArch64 @@ -613,7 +613,8 @@ bool LongJmpPass::relax(BinaryFunction &Func) { if (!Stubs[&Func].count(&BB) || !BB.isValid()) continue; - Modified |= relaxStub(BB); + if (auto E = relaxStub(BB, Modified)) + return Error(std::move(E)); } for (std::pair> &Elmt : @@ -625,7 +626,7 @@ bool LongJmpPass::relax(BinaryFunction &Func) { Func.insertBasicBlocks(Elmt.first, std::move(NewBBs), true); } - return Modified; + return Error::success(); } Error LongJmpPass::runOnFunctions(BinaryContext &BC) { @@ -639,13 +640,12 @@ Error LongJmpPass::runOnFunctions(BinaryContext &BC) { tentativeLayout(BC, Sorted); updateStubGroups(); for (BinaryFunction *Func : Sorted) { - if (relax(*Func)) { - // Don't ruin non-simple functions, they can't afford to have the layout - // changed. - if (Func->isSimple()) - Func->fixBranches(); - Modified = true; - } + if (auto E = relax(*Func, Modified)) + return Error(std::move(E)); + // Don't ruin non-simple functions, they can't afford to have the layout + // changed. + if (Modified && Func->isSimple()) + Func->fixBranches(); } } while (Modified); outs() << "BOLT-INFO: Inserted " << NumHotStubs diff --git a/bolt/lib/Passes/PatchEntries.cpp b/bolt/lib/Passes/PatchEntries.cpp index ddef85409f022..0b0e15f1b5bd0 100644 --- a/bolt/lib/Passes/PatchEntries.cpp +++ b/bolt/lib/Passes/PatchEntries.cpp @@ -103,7 +103,7 @@ Error PatchEntries::runOnFunctions(BinaryContext &BC) { if (opts::ForcePatch) { errs() << "BOLT-ERROR: unable to patch entries in " << Function << "\n"; - exit(1); + return createFatalBOLTError(""); } continue; diff --git a/bolt/lib/Passes/ReorderFunctions.cpp b/bolt/lib/Passes/ReorderFunctions.cpp index 86fc03d4e0f01..77e51c34746e3 100644 --- a/bolt/lib/Passes/ReorderFunctions.cpp +++ b/bolt/lib/Passes/ReorderFunctions.cpp @@ -250,18 +250,18 @@ void ReorderFunctions::printStats(const std::vector &Clusters, TotalCalls2MB, 100 * TotalCalls2MB / TotalCalls); } -std::vector ReorderFunctions::readFunctionOrderFile() { - std::vector FunctionNames; +Error ReorderFunctions::readFunctionOrderFile( + std::vector &FunctionNames) { std::ifstream FuncsFile(opts::FunctionOrderFile, std::ios::in); - if (!FuncsFile) { - errs() << "Ordered functions file \"" << opts::FunctionOrderFile - << "\" can't be opened.\n"; - exit(1); - } + if (!FuncsFile) + return createFatalBOLTError(Twine("Ordered functions file \"") + + Twine(opts::FunctionOrderFile) + + Twine("\" can't be opened.")); + std::string FuncName; while (std::getline(FuncsFile, FuncName)) FunctionNames.push_back(FuncName); - return FunctionNames; + return Error::success(); } Error ReorderFunctions::runOnFunctions(BinaryContext &BC) { @@ -373,7 +373,11 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) { uint32_t Index = 0; uint32_t InvalidEntries = 0; - for (const std::string &Function : readFunctionOrderFile()) { + std::vector FunctionNames; + if (Error E = readFunctionOrderFile(FunctionNames)) + return Error(std::move(E)); + + for (const std::string &Function : FunctionNames) { std::vector FuncAddrs; BinaryData *BD = BC.getBinaryDataByName(Function); @@ -444,7 +448,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) { if (!FuncsFile) { errs() << "BOLT-ERROR: ordered functions file " << opts::GenerateFunctionOrderFile << " cannot be opened\n"; - exit(1); + return createFatalBOLTError(""); } } @@ -455,7 +459,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) { if (!LinkSectionsFile) { errs() << "BOLT-ERROR: link sections file " << opts::LinkSectionsFile << " cannot be opened\n"; - exit(1); + return createFatalBOLTError(""); } } diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp index d7b25c1279dc8..2f2405b9463a7 100644 --- a/bolt/lib/Passes/ShrinkWrapping.cpp +++ b/bolt/lib/Passes/ShrinkWrapping.cpp @@ -1646,9 +1646,9 @@ void ShrinkWrapping::rebuildCFIForSP() { ++I; } -MCInst ShrinkWrapping::createStackAccess(int SPVal, int FPVal, - const FrameIndexEntry &FIE, - bool CreatePushOrPop) { +Expected ShrinkWrapping::createStackAccess(int SPVal, int FPVal, + const FrameIndexEntry &FIE, + bool CreatePushOrPop) { MCInst NewInst; if (SPVal != StackPointerTracking::SUPERPOSITION && SPVal != StackPointerTracking::EMPTY) { @@ -1656,15 +1656,15 @@ MCInst ShrinkWrapping::createStackAccess(int SPVal, int FPVal, if (!BC.MIB->createRestoreFromStack(NewInst, BC.MIB->getStackPointer(), FIE.StackOffset - SPVal, FIE.RegOrImm, FIE.Size)) { - errs() << "createRestoreFromStack: not supported on this platform\n"; - abort(); + return createFatalBOLTError( + "createRestoreFromStack: not supported on this platform\n"); } } else { if (!BC.MIB->createSaveToStack(NewInst, BC.MIB->getStackPointer(), FIE.StackOffset - SPVal, FIE.RegOrImm, FIE.Size)) { - errs() << "createSaveToStack: not supported on this platform\n"; - abort(); + return createFatalBOLTError( + "createSaveToStack: not supported on this platform\n"); } } if (CreatePushOrPop) @@ -1678,15 +1678,15 @@ MCInst ShrinkWrapping::createStackAccess(int SPVal, int FPVal, if (!BC.MIB->createRestoreFromStack(NewInst, BC.MIB->getFramePointer(), FIE.StackOffset - FPVal, FIE.RegOrImm, FIE.Size)) { - errs() << "createRestoreFromStack: not supported on this platform\n"; - abort(); + return createFatalBOLTError( + "createRestoreFromStack: not supported on this platform\n"); } } else { if (!BC.MIB->createSaveToStack(NewInst, BC.MIB->getFramePointer(), FIE.StackOffset - FPVal, FIE.RegOrImm, FIE.Size)) { - errs() << "createSaveToStack: not supported on this platform\n"; - abort(); + return createFatalBOLTError( + "createSaveToStack: not supported on this platform\n"); } } return NewInst; @@ -1743,10 +1743,11 @@ BBIterTy ShrinkWrapping::insertCFIsForPushOrPop(BinaryBasicBlock &BB, return Pos; } -BBIterTy ShrinkWrapping::processInsertion(BBIterTy InsertionPoint, - BinaryBasicBlock *CurBB, - const WorklistItem &Item, - int64_t SPVal, int64_t FPVal) { +Expected ShrinkWrapping::processInsertion(BBIterTy InsertionPoint, + BinaryBasicBlock *CurBB, + const WorklistItem &Item, + int64_t SPVal, + int64_t FPVal) { // Trigger CFI reconstruction for this CSR if necessary - writing to // PushOffsetByReg/PopOffsetByReg *will* trigger CFI update if ((Item.FIEToInsert.IsStore && @@ -1772,9 +1773,12 @@ BBIterTy ShrinkWrapping::processInsertion(BBIterTy InsertionPoint, << " Is push = " << (Item.Action == WorklistItem::InsertPushOrPop) << "\n"; }); - MCInst NewInst = + Expected NewInstOrErr = createStackAccess(SPVal, FPVal, Item.FIEToInsert, Item.Action == WorklistItem::InsertPushOrPop); + if (auto E = NewInstOrErr.takeError()) + return Error(std::move(E)); + MCInst &NewInst = *NewInstOrErr; if (InsertionPoint != CurBB->end()) { LLVM_DEBUG({ dbgs() << "Adding before Inst: "; @@ -1791,7 +1795,7 @@ BBIterTy ShrinkWrapping::processInsertion(BBIterTy InsertionPoint, return CurBB->end(); } -BBIterTy ShrinkWrapping::processInsertionsList( +Expected ShrinkWrapping::processInsertionsList( BBIterTy InsertionPoint, BinaryBasicBlock *CurBB, std::vector &TodoList, int64_t SPVal, int64_t FPVal) { bool HasInsertions = llvm::any_of(TodoList, [&](WorklistItem &Item) { @@ -1840,8 +1844,11 @@ BBIterTy ShrinkWrapping::processInsertionsList( Item.Action == WorklistItem::ChangeToAdjustment) continue; - InsertionPoint = + auto InsertionPointOrErr = processInsertion(InsertionPoint, CurBB, Item, SPVal, FPVal); + if (auto E = InsertionPointOrErr.takeError()) + return Error(std::move(E)); + InsertionPoint = *InsertionPointOrErr; if (Item.Action == WorklistItem::InsertPushOrPop && Item.FIEToInsert.IsStore) SPVal -= Item.FIEToInsert.Size; @@ -1852,7 +1859,7 @@ BBIterTy ShrinkWrapping::processInsertionsList( return InsertionPoint; } -bool ShrinkWrapping::processInsertions() { +Expected ShrinkWrapping::processInsertions() { PredictiveStackPointerTracking PSPT(BF, Todo, Info, AllocatorId); PSPT.run(); @@ -1875,14 +1882,20 @@ bool ShrinkWrapping::processInsertions() { auto Iter = I; std::pair SPTState = *PSPT.getStateAt(Iter == BB.begin() ? (ProgramPoint)&BB : &*(--Iter)); - I = processInsertionsList(I, &BB, List, SPTState.first, SPTState.second); + auto IterOrErr = + processInsertionsList(I, &BB, List, SPTState.first, SPTState.second); + if (auto E = IterOrErr.takeError()) + return Error(std::move(E)); + I = *IterOrErr; } // Process insertions at the end of bb auto WRI = Todo.find(&BB); if (WRI != Todo.end()) { std::pair SPTState = *PSPT.getStateAt(*BB.rbegin()); - processInsertionsList(BB.end(), &BB, WRI->second, SPTState.first, - SPTState.second); + if (auto E = processInsertionsList(BB.end(), &BB, WRI->second, + SPTState.first, SPTState.second) + .takeError()) + return Error(std::move(E)); Changes = true; } } @@ -1945,7 +1958,7 @@ void ShrinkWrapping::rebuildCFI() { } } -bool ShrinkWrapping::perform(bool HotOnly) { +Expected ShrinkWrapping::perform(bool HotOnly) { HasDeletedOffsetCFIs = BitVector(BC.MRI->getNumRegs(), false); PushOffsetByReg = std::vector(BC.MRI->getNumRegs(), 0LL); PopOffsetByReg = std::vector(BC.MRI->getNumRegs(), 0LL); @@ -1998,7 +2011,11 @@ bool ShrinkWrapping::perform(bool HotOnly) { }); SLM.performChanges(); // Early exit if processInsertions doesn't detect any todo items - if (!processInsertions()) + auto ModifiedOrErr = processInsertions(); + if (auto E = ModifiedOrErr.takeError()) + return Error(std::move(E)); + const bool Modified = *ModifiedOrErr; + if (!Modified) return false; processDeletions(); if (foldIdenticalSplitEdges()) { diff --git a/bolt/lib/Passes/VeneerElimination.cpp b/bolt/lib/Passes/VeneerElimination.cpp index 06cfecc04ddb6..d844a068184dd 100644 --- a/bolt/lib/Passes/VeneerElimination.cpp +++ b/bolt/lib/Passes/VeneerElimination.cpp @@ -79,8 +79,8 @@ Error VeneerElimination::runOnFunctions(BinaryContext &BC) { VeneerCallers++; if (!BC.MIB->replaceBranchTarget( Instr, VeneerDestinations[TargetSymbol], BC.Ctx.get())) { - errs() << "BOLT-ERROR: updating veneer call destination failed\n"; - exit(1); + return createFatalBOLTError( + "BOLT-ERROR: updating veneer call destination failed\n"); } } } diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp index 06763fc4d9334..cad80189c23b8 100644 --- a/bolt/lib/Rewrite/BinaryPassManager.cpp +++ b/bolt/lib/Rewrite/BinaryPassManager.cpp @@ -268,7 +268,7 @@ const char BinaryFunctionPassManager::TimerGroupName[] = "passman"; const char BinaryFunctionPassManager::TimerGroupDesc[] = "Binary Function Pass Manager"; -void BinaryFunctionPassManager::runPasses() { +Error BinaryFunctionPassManager::runPasses() { auto &BFs = BC.getBinaryFunctions(); for (size_t PassIdx = 0; PassIdx < Passes.size(); PassIdx++) { const std::pair> @@ -286,8 +286,14 @@ void BinaryFunctionPassManager::runPasses() { NamedRegionTimer T(Pass->getName(), Pass->getName(), TimerGroupName, TimerGroupDesc, TimeOpts); - callWithDynoStats([this, &Pass] { cantFail(Pass->runOnFunctions(BC)); }, - BFs, Pass->getName(), opts::DynoStatsAll, BC.isAArch64()); + Error E = Error::success(); + callWithDynoStats( + [this, &E, &Pass] { + E = joinErrors(std::move(E), Pass->runOnFunctions(BC)); + }, + BFs, Pass->getName(), opts::DynoStatsAll, BC.isAArch64()); + if (E) + return Error(std::move(E)); if (opts::VerifyCFG && !std::accumulate( @@ -296,9 +302,9 @@ void BinaryFunctionPassManager::runPasses() { const std::pair &It) { return Valid && It.second.validateCFG(); })) { - errs() << "BOLT-ERROR: Invalid CFG detected after pass " - << Pass->getName() << "\n"; - exit(1); + return createFatalBOLTError( + Twine("BOLT-ERROR: Invalid CFG detected after pass ") + + Twine(Pass->getName()) + Twine("\n")); } if (opts::Verbosity > 0) @@ -321,9 +327,10 @@ void BinaryFunctionPassManager::runPasses() { Function.dumpGraphForPass(PassIdName); } } + return Error::success(); } -void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) { +Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) { BinaryFunctionPassManager Manager(BC); const DynoStats InitialDynoStats = @@ -516,7 +523,7 @@ void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) { // in parallel and restore them Manager.registerPass(std::make_unique(NeverPrint)); - Manager.runPasses(); + return Manager.runPasses(); } } // namespace bolt diff --git a/bolt/lib/Rewrite/MachORewriteInstance.cpp b/bolt/lib/Rewrite/MachORewriteInstance.cpp index 8be8257f15c1c..c2a6c1f88ab07 100644 --- a/bolt/lib/Rewrite/MachORewriteInstance.cpp +++ b/bolt/lib/Rewrite/MachORewriteInstance.cpp @@ -337,7 +337,7 @@ void MachORewriteInstance::disassembleFunctions() { BinaryFunction &Function = BFI.second; if (!Function.isSimple()) continue; - Function.disassemble(); + cantFail(Function.disassemble()); if (opts::PrintDisasm) Function.print(outs(), "after disassembly"); } @@ -387,7 +387,7 @@ void MachORewriteInstance::runOptimizationPasses() { Manager.registerPass( std::make_unique(opts::PrintFinalized)); - Manager.runPasses(); + cantFail(Manager.runPasses()); } void MachORewriteInstance::mapInstrumentationSection( diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index c909e31e93c95..829568cefff01 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1283,7 +1283,7 @@ void RewriteInstance::discoverFileObjects() { /*UseMaxSize*/ true); if (BF) { assert(Rel.isRelative() && "Expected relative relocation for island"); - BF->markIslandDynamicRelocationAtAddress(RelAddress); + cantFail(BF->markIslandDynamicRelocationAtAddress(RelAddress)); } } } @@ -2859,8 +2859,9 @@ void RewriteInstance::selectFunctionsToProcess() { StringSet<> ReorderFunctionsUserSet; StringSet<> ReorderFunctionsLTOCommonSet; if (opts::ReorderFunctions == ReorderFunctions::RT_USER) { - for (const std::string &Function : - ReorderFunctions::readFunctionOrderFile()) { + std::vector FunctionNames; + cantFail(ReorderFunctions::readFunctionOrderFile(FunctionNames)); + for (const std::string &Function : FunctionNames) { ReorderFunctionsUserSet.insert(Function); if (std::optional LTOCommonName = getLTOCommonName(Function)) ReorderFunctionsLTOCommonSet.insert(*LTOCommonName); @@ -3133,7 +3134,13 @@ void RewriteInstance::disassembleFunctions() { continue; } - if (!Function.disassemble()) { + bool DisasmFailed{false}; + handleAllErrors(Function.disassemble(), [&](const BOLTError &E) { + DisasmFailed = true; + if (E.isFatal()) { + E.log(errs()); + exit(1); + } if (opts::processAllFunctions()) BC->exitWithBugReport("function cannot be properly disassembled. " "Unable to continue in relocation mode.", @@ -3143,8 +3150,10 @@ void RewriteInstance::disassembleFunctions() { << ". Will ignore.\n"; // Forcefully ignore the function. Function.setIgnored(); + }); + + if (DisasmFailed) continue; - } if (opts::PrintAll || opts::PrintDisasm) Function.print(outs(), "after disassembly"); @@ -3199,7 +3208,7 @@ void RewriteInstance::disassembleFunctions() { check_error(LSDASection.getError(), "failed to get LSDA section"); ArrayRef LSDAData = ArrayRef( LSDASection->getData(), LSDASection->getContents().size()); - Function.parseLSDA(LSDAData, LSDASection->getAddress()); + cantFail(Function.parseLSDA(LSDAData, LSDASection->getAddress())); } } } @@ -3214,7 +3223,16 @@ void RewriteInstance::buildFunctionsCFG() { ParallelUtilities::WorkFuncWithAllocTy WorkFun = [&](BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId) { - if (!BF.buildCFG(AllocId)) + bool HadErrors{false}; + handleAllErrors(BF.buildCFG(AllocId), [&](const BOLTError &E) { + if (!E.getMessage().empty()) + E.log(errs()); + if (E.isFatal()) + exit(1); + HadErrors = true; + }); + + if (HadErrors) return; if (opts::PrintAll) { @@ -3281,7 +3299,11 @@ void RewriteInstance::postProcessFunctions() { void RewriteInstance::runOptimizationPasses() { NamedRegionTimer T("runOptimizationPasses", "run optimization passes", TimerGroupName, TimerGroupDesc, opts::TimeRewrite); - BinaryFunctionPassManager::runAllPasses(*BC); + handleAllErrors(BinaryFunctionPassManager::runAllPasses(*BC), + [](const BOLTError &E) { + E.log(errs()); + exit(1); + }); } void RewriteInstance::preregisterSections() { diff --git a/bolt/lib/Target/X86/X86MCSymbolizer.cpp b/bolt/lib/Target/X86/X86MCSymbolizer.cpp index ca7fe137152fd..dead6856d2f69 100644 --- a/bolt/lib/Target/X86/X86MCSymbolizer.cpp +++ b/bolt/lib/Target/X86/X86MCSymbolizer.cpp @@ -134,7 +134,12 @@ bool X86MCSymbolizer::tryAddingSymbolicOperand( // a PC-relative 8-byte fixup, which is what we need to cover this. The // only way to do this is to use the symbol name _GLOBAL_OFFSET_TABLE_. if (Relocation::isX86GOTPC64(Relocation->Type)) { - auto [Sym, Addend] = handleGOTPC64(*Relocation, InstAddress); + auto PairOrErr = handleGOTPC64(*Relocation, InstAddress); + if (auto E = PairOrErr.takeError()) { + Function.setSimple(false); + return false; + } + auto [Sym, Addend] = *PairOrErr; addOperand(Sym, Addend); return true; } @@ -158,14 +163,16 @@ bool X86MCSymbolizer::tryAddingSymbolicOperand( return true; } -std::pair +Expected> X86MCSymbolizer::handleGOTPC64(const Relocation &R, uint64_t InstrAddr) { BinaryContext &BC = Function.getBinaryContext(); const BinaryData *GOTSymBD = BC.getGOTSymbol(); if (!GOTSymBD || !GOTSymBD->getAddress()) { - errs() << "BOLT-ERROR: R_X86_GOTPC64 relocation is present but we did " - "not detect a valid _GLOBAL_OFFSET_TABLE_ in symbol table\n"; - exit(1); + // This error is pretty serious but we can't kill the disassembler + // because of it, so don't make it fatal. Log it and warn the user. + return createNonFatalBOLTError( + "R_X86_GOTPC64 relocation is present but we did not detect " + "a valid _GLOBAL_OFFSET_TABLE_ in symbol table\n"); } // R_X86_GOTPC64 are not relative to the Reloc nor end of instruction, // but the start of the MOVABSQ instruction. So the Target Address is diff --git a/bolt/lib/Target/X86/X86MCSymbolizer.h b/bolt/lib/Target/X86/X86MCSymbolizer.h index 9ed18b69c74ce..189941e949e33 100644 --- a/bolt/lib/Target/X86/X86MCSymbolizer.h +++ b/bolt/lib/Target/X86/X86MCSymbolizer.h @@ -20,8 +20,8 @@ class X86MCSymbolizer : public MCSymbolizer { BinaryFunction &Function; bool CreateNewSymbols{true}; - std::pair handleGOTPC64(const Relocation &R, - uint64_t InstrAddr); + Expected> handleGOTPC64(const Relocation &R, + uint64_t InstrAddr); public: X86MCSymbolizer(BinaryFunction &Function, bool CreateNewSymbols = true)