diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h --- a/bolt/include/bolt/Core/BinaryContext.h +++ b/bolt/include/bolt/Core/BinaryContext.h @@ -211,7 +211,7 @@ std::map DwarfLineTablesCUMap; public: - static std::unique_ptr + static Expected> createBinaryContext(const ObjectFile *File, bool IsPIC, std::unique_ptr DwCtx); diff --git a/bolt/include/bolt/Rewrite/MachORewriteInstance.h b/bolt/include/bolt/Rewrite/MachORewriteInstance.h --- a/bolt/include/bolt/Rewrite/MachORewriteInstance.h +++ b/bolt/include/bolt/Rewrite/MachORewriteInstance.h @@ -65,7 +65,15 @@ void rewriteFile(); public: - MachORewriteInstance(object::MachOObjectFile *InputFile, StringRef ToolPath); + // This constructor has complex initialization that can fail during + // construction. Constructors can’t return errors, so clients must test \p Err + // after the object is constructed. Use createMachORewriteInstance instead. + MachORewriteInstance(object::MachOObjectFile *InputFile, StringRef ToolPath, + Error &Err); + + static Expected> + createMachORewriteInstance(object::MachOObjectFile *InputFile, + StringRef ToolPath); ~MachORewriteInstance(); Error setProfile(StringRef FileName); diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h --- a/bolt/include/bolt/Rewrite/RewriteInstance.h +++ b/bolt/include/bolt/Rewrite/RewriteInstance.h @@ -40,8 +40,15 @@ /// events. class RewriteInstance { public: + // This constructor has complex initialization that can fail during + // construction. Constructors can’t return errors, so clients must test \p Err + // after the object is constructed. Use createRewriteInstance instead. RewriteInstance(llvm::object::ELFObjectFileBase *File, const int Argc, - const char *const *Argv, StringRef ToolPath); + const char *const *Argv, StringRef ToolPath, Error &Err); + + static Expected> + createRewriteInstance(llvm::object::ELFObjectFileBase *File, const int Argc, + const char *const *Argv, StringRef ToolPath); ~RewriteInstance(); /// Assign profile from \p Filename to this instance. diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -32,6 +32,7 @@ #include "llvm/MC/MCSubtargetInfo.h" #include "llvm/MC/MCSymbol.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/Error.h" #include "llvm/Support/Regex.h" #include #include @@ -114,7 +115,7 @@ /// Create BinaryContext for a given architecture \p ArchName and /// triple \p TripleName. -std::unique_ptr +Expected> BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC, std::unique_ptr DwCtx) { StringRef ArchName = ""; @@ -130,8 +131,8 @@ "+fullfp16,+spe,+fuse-aes,+rcpc"; break; default: - errs() << "BOLT-ERROR: Unrecognized machine in ELF file.\n"; - return nullptr; + return createStringError(errc::not_supported, + "BOLT-ERROR: Unrecognized machine in ELF file"); } auto TheTriple = std::make_unique(File->makeTriple()); @@ -140,39 +141,36 @@ std::string Error; const Target *TheTarget = TargetRegistry::lookupTarget(std::string(ArchName), *TheTriple, Error); - if (!TheTarget) { - errs() << "BOLT-ERROR: " << Error; - return nullptr; - } + if (!TheTarget) + return createStringError(errc::not_supported, Twine("BOLT-ERROR: ", Error)); std::unique_ptr MRI( TheTarget->createMCRegInfo(TripleName)); - if (!MRI) { - errs() << "BOLT-ERROR: no register info for target " << TripleName << "\n"; - return nullptr; - } + if (!MRI) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no register info for target ", TripleName)); // Set up disassembler. std::unique_ptr AsmInfo( TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions())); - if (!AsmInfo) { - errs() << "BOLT-ERROR: no assembly info for target " << TripleName << "\n"; - return nullptr; - } + if (!AsmInfo) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no assembly info for target ", TripleName)); std::unique_ptr STI( TheTarget->createMCSubtargetInfo(TripleName, "", FeaturesStr)); - if (!STI) { - errs() << "BOLT-ERROR: no subtarget info for target " << TripleName << "\n"; - return nullptr; - } + if (!STI) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no subtarget info for target ", TripleName)); std::unique_ptr MII(TheTarget->createMCInstrInfo()); - if (!MII) { - errs() << "BOLT-ERROR: no instruction info for target " << TripleName - << "\n"; - return nullptr; - } + if (!MII) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no instruction info for target ", TripleName)); std::unique_ptr Ctx( new MCContext(*TheTriple, AsmInfo.get(), MRI.get(), STI.get())); @@ -197,28 +195,27 @@ std::unique_ptr DisAsm( TheTarget->createMCDisassembler(*STI, *Ctx)); - if (!DisAsm) { - errs() << "BOLT-ERROR: no disassembler for target " << TripleName << "\n"; - return nullptr; - } + if (!DisAsm) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no disassembler info for target ", TripleName)); std::unique_ptr MIA( TheTarget->createMCInstrAnalysis(MII.get())); - if (!MIA) { - errs() << "BOLT-ERROR: failed to create instruction analysis for target" - << TripleName << "\n"; - return nullptr; - } + if (!MIA) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: failed to create instruction analysis for target ", + TripleName)); int AsmPrinterVariant = AsmInfo->getAssemblerDialect(); std::unique_ptr InstructionPrinter( TheTarget->createMCInstPrinter(*TheTriple, AsmPrinterVariant, *AsmInfo, *MII, *MRI)); - if (!InstructionPrinter) { - errs() << "BOLT-ERROR: no instruction printer for target " << TripleName - << '\n'; - return nullptr; - } + if (!InstructionPrinter) + return createStringError( + errc::not_supported, + Twine("BOLT-ERROR: no instruction printer for target ", TripleName)); InstructionPrinter->setPrintImmHex(true); std::unique_ptr MCE( diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp --- a/bolt/lib/Rewrite/DWARFRewriter.cpp +++ b/bolt/lib/Rewrite/DWARFRewriter.cpp @@ -28,6 +28,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Endian.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/ThreadPool.h" #include "llvm/Support/ToolOutputFile.h" @@ -901,11 +902,11 @@ std::unique_ptr createDwarfOnlyBC(const object::ObjectFile &File) { - return BinaryContext::createBinaryContext( + return cantFail(BinaryContext::createBinaryContext( &File, false, DWARFContext::create(File, DWARFContext::ProcessDebugRelocations::Ignore, nullptr, "", WithColor::defaultErrorHandler, - WithColor::defaultWarningHandler)); + WithColor::defaultWarningHandler))); } StringMap diff --git a/bolt/lib/Rewrite/MachORewriteInstance.cpp b/bolt/lib/Rewrite/MachORewriteInstance.cpp --- a/bolt/lib/Rewrite/MachORewriteInstance.cpp +++ b/bolt/lib/Rewrite/MachORewriteInstance.cpp @@ -23,6 +23,7 @@ #include "llvm/MC/MCObjectStreamer.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/ToolOutputFile.h" +#include namespace opts { @@ -81,11 +82,28 @@ #define DEBUG_TYPE "bolt" +Expected> +MachORewriteInstance::createMachORewriteInstance( + object::MachOObjectFile *InputFile, StringRef ToolPath) { + Error Err = Error::success(); + auto MachORI = + std::make_unique(InputFile, ToolPath, Err); + if (Err) + return std::move(Err); + return MachORI; +} + MachORewriteInstance::MachORewriteInstance(object::MachOObjectFile *InputFile, - StringRef ToolPath) - : InputFile(InputFile), ToolPath(ToolPath), - BC(BinaryContext::createBinaryContext(InputFile, /* IsPIC */ true, - DWARFContext::create(*InputFile))) { + StringRef ToolPath, Error &Err) + : InputFile(InputFile), ToolPath(ToolPath) { + ErrorAsOutParameter EAO(&Err); + auto BCOrErr = BinaryContext::createBinaryContext( + InputFile, /* IsPIC */ true, DWARFContext::create(*InputFile)); + if (Error E = BCOrErr.takeError()) { + Err = std::move(E); + return; + } + BC = std::move(BCOrErr.get()); BC->initializeTarget(std::unique_ptr(createMCPlusBuilder( BC->TheTriple->getArch(), BC->MIA.get(), BC->MII.get(), BC->MRI.get()))); if (opts::Instrument) 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 @@ -46,6 +46,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/DataExtractor.h" #include "llvm/Support/Errc.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/LEB128.h" #include "llvm/Support/ManagedStatic.h" @@ -54,6 +55,7 @@ #include "llvm/Support/raw_ostream.h" #include #include +#include #include #undef DEBUG_TYPE @@ -352,14 +354,28 @@ } // anonymous namespace +Expected> +RewriteInstance::createRewriteInstance(ELFObjectFileBase *File, const int Argc, + const char *const *Argv, + StringRef ToolPath) { + Error Err = Error::success(); + auto RI = std::make_unique(File, Argc, Argv, ToolPath, Err); + if (Err) + return std::move(Err); + return RI; +} + RewriteInstance::RewriteInstance(ELFObjectFileBase *File, const int Argc, - const char *const *Argv, StringRef ToolPath) + const char *const *Argv, StringRef ToolPath, + Error &Err) : InputFile(File), Argc(Argc), Argv(Argv), ToolPath(ToolPath), SHStrTab(StringTableBuilder::ELF) { + ErrorAsOutParameter EAO(&Err); auto ELF64LEFile = dyn_cast(InputFile); if (!ELF64LEFile) { - errs() << "BOLT-ERROR: only 64-bit LE ELF binaries are supported\n"; - exit(1); + Err = createStringError(errc::not_supported, + "Only 64-bit LE ELF binaries are supported"); + return; } bool IsPIC = false; @@ -370,13 +386,17 @@ IsPIC = true; } - BC = BinaryContext::createBinaryContext( + auto BCOrErr = BinaryContext::createBinaryContext( File, IsPIC, DWARFContext::create(*File, DWARFContext::ProcessDebugRelocations::Ignore, nullptr, opts::DWPPathName, WithColor::defaultErrorHandler, WithColor::defaultWarningHandler)); - + if (Error E = BCOrErr.takeError()) { + Err = std::move(E); + return; + } + BC = std::move(BCOrErr.get()); BC->initializeTarget(std::unique_ptr(createMCPlusBuilder( BC->TheTriple->getArch(), BC->MIA.get(), BC->MII.get(), BC->MRI.get()))); diff --git a/bolt/tools/driver/llvm-bolt.cpp b/bolt/tools/driver/llvm-bolt.cpp --- a/bolt/tools/driver/llvm-bolt.cpp +++ b/bolt/tools/driver/llvm-bolt.cpp @@ -215,7 +215,11 @@ Binary &Binary = *BinaryOrErr.get().getBinary(); if (auto *e = dyn_cast(&Binary)) { - RewriteInstance RI(e, argc, argv, ToolPath); + auto RIOrErr = + RewriteInstance::createRewriteInstance(e, argc, argv, ToolPath); + if (Error E = RIOrErr.takeError()) + report_error(opts::InputFilename, std::move(E)); + RewriteInstance &RI = *RIOrErr.get(); if (!opts::PerfData.empty()) { if (!opts::AggregateOnly) { errs() << ToolName @@ -238,7 +242,11 @@ RI.run(); } else if (auto *O = dyn_cast(&Binary)) { - MachORewriteInstance MachORI(O, ToolPath); + auto MachORIOrErr = + MachORewriteInstance::createMachORewriteInstance(O, ToolPath); + if (Error E = MachORIOrErr.takeError()) + report_error(opts::InputFilename, std::move(E)); + MachORewriteInstance &MachORI = *MachORIOrErr.get(); if (!opts::InputDataFilename.empty()) if (Error E = MachORI.setProfile(opts::InputDataFilename)) @@ -265,10 +273,18 @@ Binary &Binary2 = *BinaryOrErr2.get().getBinary(); if (auto *ELFObj1 = dyn_cast(&Binary1)) { if (auto *ELFObj2 = dyn_cast(&Binary2)) { - RewriteInstance RI1(ELFObj1, argc, argv, ToolPath); + auto RI1OrErr = + RewriteInstance::createRewriteInstance(ELFObj1, argc, argv, ToolPath); + if (Error E = RI1OrErr.takeError()) + report_error(opts::InputFilename, std::move(E)); + RewriteInstance &RI1 = *RI1OrErr.get(); if (Error E = RI1.setProfile(opts::InputDataFilename)) report_error(opts::InputDataFilename, std::move(E)); - RewriteInstance RI2(ELFObj2, argc, argv, ToolPath); + auto RI2OrErr = + RewriteInstance::createRewriteInstance(ELFObj2, argc, argv, ToolPath); + if (Error E = RI2OrErr.takeError()) + report_error(opts::InputFilename2, std::move(E)); + RewriteInstance &RI2 = *RI2OrErr.get(); if (Error E = RI2.setProfile(opts::InputDataFilename2)) report_error(opts::InputDataFilename2, std::move(E)); outs() << "BOLT-DIFF: *** Analyzing binary 1: " << opts::InputFilename diff --git a/bolt/tools/heatmap/heatmap.cpp b/bolt/tools/heatmap/heatmap.cpp --- a/bolt/tools/heatmap/heatmap.cpp +++ b/bolt/tools/heatmap/heatmap.cpp @@ -84,7 +84,12 @@ Binary &Binary = *BinaryOrErr.get().getBinary(); if (auto *e = dyn_cast(&Binary)) { - RewriteInstance RI(e, argc, argv, ToolPath); + auto RIOrErr = + RewriteInstance::createRewriteInstance(e, argc, argv, ToolPath); + if (Error E = RIOrErr.takeError()) + report_error("RewriteInstance", std::move(E)); + + RewriteInstance &RI = *RIOrErr.get(); if (Error E = RI.setProfile(opts::PerfData)) report_error(opts::PerfData, std::move(E)); diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -47,8 +47,8 @@ } void initializeBolt() { - BC = BinaryContext::createBinaryContext( - ObjFile.get(), true, DWARFContext::create(*ObjFile.get())); + BC = cantFail(BinaryContext::createBinaryContext( + ObjFile.get(), true, DWARFContext::create(*ObjFile.get()))); ASSERT_FALSE(!BC); BC->initializeTarget(std::unique_ptr(createMCPlusBuilder( GetParam(), BC->MIA.get(), BC->MII.get(), BC->MRI.get())));