diff options
author | Amir Ayupov <aaupov@fb.com> | 2024-02-12 14:51:15 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-12 14:51:15 -0800 |
commit | 13d60ce2f262ef9055389908b63824e53b3054a1 (patch) | |
tree | dfc36e0b47e8c7ecf53fbdee55b9f06649124a9a /bolt | |
parent | fa7dd4919aa705f18f268fab5b2887d45f89d8dd (diff) | |
download | llvm-13d60ce2f262ef9055389908b63824e53b3054a1.zip llvm-13d60ce2f262ef9055389908b63824e53b3054a1.tar.gz llvm-13d60ce2f262ef9055389908b63824e53b3054a1.tar.bz2 |
[BOLT][NFC] Propagate BOLTErrors from Core, RewriteInstance, and passes (2/2) (#81523)
As part of the effort to refactor old error handling code that
would directly call exit(1), in this patch continue the migration
on libCore, libRewrite and libPasses to use the new BOLTError
class whenever a failure occurs.
Test Plan: NFC
Co-authored-by: Rafael Auler <rafaelauler@fb.com>
Diffstat (limited to 'bolt')
-rw-r--r-- | bolt/include/bolt/Core/BinaryFunction.h | 21 | ||||
-rw-r--r-- | bolt/include/bolt/Core/BinarySection.h | 4 | ||||
-rw-r--r-- | bolt/include/bolt/Passes/FrameOptimizer.h | 4 | ||||
-rw-r--r-- | bolt/include/bolt/Passes/LongJmp.h | 4 | ||||
-rw-r--r-- | bolt/include/bolt/Passes/ReorderFunctions.h | 2 | ||||
-rw-r--r-- | bolt/include/bolt/Passes/ShrinkWrapping.h | 24 | ||||
-rw-r--r-- | bolt/include/bolt/Rewrite/BinaryPassManager.h | 4 | ||||
-rw-r--r-- | bolt/lib/Core/BinaryFunction.cpp | 71 | ||||
-rw-r--r-- | bolt/lib/Core/BinarySection.cpp | 2 | ||||
-rw-r--r-- | bolt/lib/Core/Exceptions.cpp | 11 | ||||
-rw-r--r-- | bolt/lib/Passes/BinaryPasses.cpp | 6 | ||||
-rw-r--r-- | bolt/lib/Passes/FrameOptimizer.cpp | 21 | ||||
-rw-r--r-- | bolt/lib/Passes/LongJmp.cpp | 42 | ||||
-rw-r--r-- | bolt/lib/Passes/ReorderFunctions.cpp | 22 | ||||
-rw-r--r-- | bolt/lib/Passes/ShrinkWrapping.cpp | 65 | ||||
-rw-r--r-- | bolt/lib/Rewrite/BinaryPassManager.cpp | 23 | ||||
-rw-r--r-- | bolt/lib/Rewrite/MachORewriteInstance.cpp | 4 | ||||
-rw-r--r-- | bolt/lib/Rewrite/RewriteInstance.cpp | 38 | ||||
-rw-r--r-- | bolt/lib/Target/X86/X86MCSymbolizer.cpp | 17 | ||||
-rw-r--r-- | bolt/lib/Target/X86/X86MCSymbolizer.h | 4 |
20 files changed, 242 insertions, 147 deletions
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 3a1eae3..a17717876 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1910,12 +1910,11 @@ public: /// 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 @@ public: // 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 @@ public: /// 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 @@ public: /// /// 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 @@ public: } /// Process LSDA information for the function. - void parseLSDA(ArrayRef<uint8_t> LSDAData, uint64_t LSDAAddress); + Error parseLSDA(ArrayRef<uint8_t> 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 70914f5..a85dbf2 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 @@ public: 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 a0d93c0..64055bd 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<bool> &PrintPass) diff --git a/bolt/include/bolt/Passes/LongJmp.h b/bolt/include/bolt/Passes/LongJmp.h index 9488337..3d02d75 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 6ca2dc3..b75937d 100644 --- a/bolt/include/bolt/Passes/ReorderFunctions.h +++ b/bolt/include/bolt/Passes/ReorderFunctions.h @@ -44,7 +44,7 @@ public: const char *getName() const override { return "reorder-functions"; } Error runOnFunctions(BinaryContext &BC) override; - static std::vector<std::string> readFunctionOrderFile(); + static Error readFunctionOrderFile(std::vector<std::string> &FunctionNames); }; } // namespace bolt diff --git a/bolt/include/bolt/Passes/ShrinkWrapping.h b/bolt/include/bolt/Passes/ShrinkWrapping.h index cccbc51..016bea7 100644 --- a/bolt/include/bolt/Passes/ShrinkWrapping.h +++ b/bolt/include/bolt/Passes/ShrinkWrapping.h @@ -467,8 +467,9 @@ private: /// 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<MCInst> 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 @@ private: /// 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<BBIterTy> 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<WorklistItem> &TodoList, - int64_t SPVal, int64_t FPVal); + Expected<BBIterTy> processInsertionsList(BBIterTy InsertionPoint, + BinaryBasicBlock *CurBB, + std::vector<WorklistItem> &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<bool> processInsertions(); /// Apply all deletion todo tasks (or tasks to change a push/pop to a memory /// access no-op) @@ -519,7 +521,7 @@ public: BC.MIB->removeAnnotation(Inst, getAnnotationIndex()); } - bool perform(bool HotOnly = false); + Expected<bool> perform(bool HotOnly = false); static void printStats(); }; diff --git a/bolt/include/bolt/Rewrite/BinaryPassManager.h b/bolt/include/bolt/Rewrite/BinaryPassManager.h index 84ab192..2297c3b 100644 --- a/bolt/include/bolt/Rewrite/BinaryPassManager.h +++ b/bolt/include/bolt/Rewrite/BinaryPassManager.h @@ -46,10 +46,10 @@ public: } /// 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/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 0ac47a5..3f3da3b 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<int64_t>(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<ArrayRef<uint8_t>> 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<BOLTError>(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<BOLTError>(std::move(E))); + if (!E.getMessage().empty()) + E.log(errs()); + return Error::success(); + })) + return Error(std::move(NewE)); + } } add_instruction: @@ -1413,12 +1436,12 @@ add_instruction: 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 97bc251..564c63e 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 ab1885f..dd3fa46 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<uint8_t> LSDASectionData, - uint64_t LSDASectionAddress) { +Error BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData, + uint64_t LSDASectionAddress) { assert(CurrentState == State::Disassembled && "unexpected function state"); if (!getLSDAAddress()) - return; + return Error::success(); DWARFDataExtractor Data( StringRef(reinterpret_cast<const char *>(LSDASectionData.data()), @@ -121,7 +121,7 @@ void BinaryFunction::parseLSDA(ArrayRef<uint8_t> 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<uint8_t> 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<uint8_t> LSDASectionData, LSDATypeIndexTable = LSDASectionData.slice(TypeIndexTableStart, MaxTypeIndexTableOffset); } + return Error::success(); } void BinaryFunction::updateEHRanges() { diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp index 281488b..4d92b27 100644 --- a/bolt/lib/Passes/BinaryPasses.cpp +++ b/bolt/lib/Passes/BinaryPasses.cpp @@ -1456,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 83a5a36..30bcfb9 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<std::mutex> Lock(FuncsChangedMutex); + SWError = joinErrors(std::move(SWError), Error(std::move(E))); + return; + } + const bool Changed = *ChangedOrErr; + if (Changed) { std::lock_guard<std::mutex> 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/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index b524339..72823b9 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<int>(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<int>(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<BinaryBasicBlock *, std::unique_ptr<BinaryBasicBlock>> &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/ReorderFunctions.cpp b/bolt/lib/Passes/ReorderFunctions.cpp index 9cecd08..77e51c3 100644 --- a/bolt/lib/Passes/ReorderFunctions.cpp +++ b/bolt/lib/Passes/ReorderFunctions.cpp @@ -250,18 +250,18 @@ void ReorderFunctions::printStats(const std::vector<Cluster> &Clusters, TotalCalls2MB, 100 * TotalCalls2MB / TotalCalls); } -std::vector<std::string> ReorderFunctions::readFunctionOrderFile() { - std::vector<std::string> FunctionNames; +Error ReorderFunctions::readFunctionOrderFile( + std::vector<std::string> &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<std::string> FunctionNames; + if (Error E = readFunctionOrderFile(FunctionNames)) + return Error(std::move(E)); + + for (const std::string &Function : FunctionNames) { std::vector<uint64_t> FuncAddrs; BinaryData *BD = BC.getBinaryDataByName(Function); diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp index d7b25c1..2f2405b 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<MCInst> 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<BBIterTy> 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<MCInst> 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<BBIterTy> ShrinkWrapping::processInsertionsList( BBIterTy InsertionPoint, BinaryBasicBlock *CurBB, std::vector<WorklistItem> &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<bool> ShrinkWrapping::processInsertions() { PredictiveStackPointerTracking PSPT(BF, Todo, Info, AllocatorId); PSPT.run(); @@ -1875,14 +1882,20 @@ bool ShrinkWrapping::processInsertions() { auto Iter = I; std::pair<int, int> 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<int, int> 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<bool> ShrinkWrapping::perform(bool HotOnly) { HasDeletedOffsetCFIs = BitVector(BC.MRI->getNumRegs(), false); PushOffsetByReg = std::vector<int64_t>(BC.MRI->getNumRegs(), 0LL); PopOffsetByReg = std::vector<int64_t>(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/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp index 06763fc..cad8018 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<const bool, std::unique_ptr<BinaryFunctionPass>> @@ -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<const uint64_t, BinaryFunction> &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<CleanMCState>(NeverPrint)); - Manager.runPasses(); + return Manager.runPasses(); } } // namespace bolt diff --git a/bolt/lib/Rewrite/MachORewriteInstance.cpp b/bolt/lib/Rewrite/MachORewriteInstance.cpp index 8be8257..c2a6c1f 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<FinalizeFunctions>(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 c909e31..829568c 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<std::string> FunctionNames; + cantFail(ReorderFunctions::readFunctionOrderFile(FunctionNames)); + for (const std::string &Function : FunctionNames) { ReorderFunctionsUserSet.insert(Function); if (std::optional<StringRef> 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<uint8_t> LSDAData = ArrayRef<uint8_t>( 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 ca7fe13..dead685 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<MCSymbol *, uint64_t> +Expected<std::pair<MCSymbol *, uint64_t>> 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 9ed18b6..189941e 100644 --- a/bolt/lib/Target/X86/X86MCSymbolizer.h +++ b/bolt/lib/Target/X86/X86MCSymbolizer.h @@ -20,8 +20,8 @@ protected: BinaryFunction &Function; bool CreateNewSymbols{true}; - std::pair<MCSymbol *, uint64_t> handleGOTPC64(const Relocation &R, - uint64_t InstrAddr); + Expected<std::pair<MCSymbol *, uint64_t>> handleGOTPC64(const Relocation &R, + uint64_t InstrAddr); public: X86MCSymbolizer(BinaryFunction &Function, bool CreateNewSymbols = true) |