From 7bd8d994978d6404a79b28e04a802d070c45ba16 Mon Sep 17 00:00:00 2001 From: Kevin Enderby Date: Mon, 2 May 2016 20:28:12 +0000 Subject: Thread Expected<...> up from libObject’s getType() for symbols to allow llvm-objdump to produce a good error message. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Produce another specific error message for a malformed Mach-O file when a symbol’s section index is more than the number of sections. The existing test case in test/Object/macho-invalid.test for macho-invalid-section-index-getSectionRawName now reports the error with the message indicating that a symbol at a specific index has a bad section index and that bad section index value. Again converting interfaces to Expected<> from ErrorOr<> does involve touching a number of places. Where the existing code reported the error with a string message or an error code it was converted to do the same. Also there some were bugs in the existing code that did not deal with the old ErrorOr<> return values.  So now with Expected<> since they must be checked and the error handled, I added a TODO and a comment: "// TODO: Actually report errors helpfully" and a call something like consumeError(NameOrErr.takeError()) so the buggy code will not crash since needed to deal with the Error. llvm-svn: 268298 --- llvm/tools/llvm-objdump/COFFDump.cpp | 6 +-- llvm/tools/llvm-objdump/MachODump.cpp | 84 ++++++++++++++++++++++++-------- llvm/tools/llvm-objdump/llvm-objdump.cpp | 19 ++++---- 3 files changed, 77 insertions(+), 32 deletions(-) (limited to 'llvm/tools/llvm-objdump') diff --git a/llvm/tools/llvm-objdump/COFFDump.cpp b/llvm/tools/llvm-objdump/COFFDump.cpp index c20edfbdc68..6b98b72b5c1 100644 --- a/llvm/tools/llvm-objdump/COFFDump.cpp +++ b/llvm/tools/llvm-objdump/COFFDump.cpp @@ -165,9 +165,9 @@ resolveSectionAndAddress(const COFFObjectFile *Obj, const SymbolRef &Sym, if (std::error_code EC = ResolvedAddrOrErr.getError()) return EC; ResolvedAddr = *ResolvedAddrOrErr; - ErrorOr Iter = Sym.getSection(); - if (std::error_code EC = Iter.getError()) - return EC; + Expected Iter = Sym.getSection(); + if (!Iter) + return errorToErrorCode(Iter.takeError()); ResolvedSection = Obj->getCOFFSection(**Iter); return std::error_code(); } diff --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp index 72acabd59a8..232f17f4cd0 100644 --- a/llvm/tools/llvm-objdump/MachODump.cpp +++ b/llvm/tools/llvm-objdump/MachODump.cpp @@ -178,13 +178,23 @@ static const Target *GetTarget(const MachOObjectFile *MachOObj, struct SymbolSorter { bool operator()(const SymbolRef &A, const SymbolRef &B) { - ErrorOr ATypeOrErr = A.getType(); - if (std::error_code EC = ATypeOrErr.getError()) - report_fatal_error(EC.message()); + Expected ATypeOrErr = A.getType(); + if (!ATypeOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(ATypeOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type AType = *ATypeOrErr; - ErrorOr BTypeOrErr = B.getType(); - if (std::error_code EC = BTypeOrErr.getError()) - report_fatal_error(EC.message()); + Expected BTypeOrErr = B.getType(); + if (!BTypeOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(BTypeOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type BType = *BTypeOrErr; uint64_t AAddr = (AType != SymbolRef::ST_Function) ? 0 : A.getValue(); uint64_t BAddr = (BType != SymbolRef::ST_Function) ? 0 : B.getValue(); @@ -597,9 +607,14 @@ static void CreateSymbolAddressMap(MachOObjectFile *O, SymbolAddressMap *AddrMap) { // Create a map of symbol addresses to symbol names. for (const SymbolRef &Symbol : O->symbols()) { - ErrorOr STOrErr = Symbol.getType(); - if (std::error_code EC = STOrErr.getError()) - report_fatal_error(EC.message()); + Expected STOrErr = Symbol.getType(); + if (!STOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(STOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type ST = *STOrErr; if (ST == SymbolRef::ST_Function || ST == SymbolRef::ST_Data || ST == SymbolRef::ST_Other) { @@ -6163,9 +6178,14 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, SymbolAddressMap AddrMap; bool DisSymNameFound = false; for (const SymbolRef &Symbol : MachOOF->symbols()) { - ErrorOr STOrErr = Symbol.getType(); - if (std::error_code EC = STOrErr.getError()) - report_fatal_error(EC.message()); + Expected STOrErr = Symbol.getType(); + if (!STOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(STOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type ST = *STOrErr; if (ST == SymbolRef::ST_Function || ST == SymbolRef::ST_Data || ST == SymbolRef::ST_Other) { @@ -6229,9 +6249,14 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, } StringRef SymName = *SymNameOrErr; - ErrorOr STOrErr = Symbols[SymIdx].getType(); - if (std::error_code EC = STOrErr.getError()) - report_fatal_error(EC.message()); + Expected STOrErr = Symbols[SymIdx].getType(); + if (!STOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(STOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type ST = *STOrErr; if (ST != SymbolRef::ST_Function && ST != SymbolRef::ST_Data) continue; @@ -6256,9 +6281,14 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, uint64_t NextSym = 0; uint64_t NextSymIdx = SymIdx + 1; while (Symbols.size() > NextSymIdx) { - ErrorOr STOrErr = Symbols[NextSymIdx].getType(); - if (std::error_code EC = STOrErr.getError()) - report_fatal_error(EC.message()); + Expected STOrErr = Symbols[NextSymIdx].getType(); + if (!STOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(STOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } SymbolRef::Type NextSymType = *STOrErr; if (NextSymType == SymbolRef::ST_Function) { containsNextSym = @@ -6534,7 +6564,15 @@ static void findUnwindRelocNameAddend(const MachOObjectFile *Obj, // Go back one so that SymbolAddress <= Addr. --Sym; - section_iterator SymSection = *Sym->second.getSection(); + auto SectOrErr = Sym->second.getSection(); + if (!SectOrErr) { + std::string Buf; + raw_string_ostream OS(Buf); + logAllUnhandledErrors(SectOrErr.takeError(), OS, ""); + OS.flush(); + report_fatal_error(Buf); + } + section_iterator SymSection = *SectOrErr; if (RelocSection == *SymSection) { // There's a valid symbol in the same section before this reference. Expected NameOrErr = Sym->second.getName(); @@ -6882,7 +6920,13 @@ void llvm::printMachOUnwindInfo(const MachOObjectFile *Obj) { for (const SymbolRef &SymRef : Obj->symbols()) { // Discard any undefined or absolute symbols. They're not going to take part // in the convenience lookup for unwind info and just take up resources. - section_iterator Section = *SymRef.getSection(); + auto SectOrErr = SymRef.getSection(); + if (!SectOrErr) { + // TODO: Actually report errors helpfully. + consumeError(SectOrErr.takeError()); + continue; + } + section_iterator Section = *SectOrErr; if (Section == Obj->section_end()) continue; diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp index 2fcd59fe2b2..1eb9f61c6ac 100644 --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -490,9 +490,9 @@ static std::error_code getRelocationValueString(const ELFObjectFile *Obj, const Elf_Sym *symb = Obj->getSymbol(SI->getRawDataRefImpl()); StringRef Target; if (symb->getType() == ELF::STT_SECTION) { - ErrorOr SymSI = SI->getSection(); - if (std::error_code EC = SymSI.getError()) - return EC; + Expected SymSI = SI->getSection(); + if (!SymSI) + return errorToErrorCode(SymSI.takeError()); const Elf_Shdr *SymSec = Obj->getSection((*SymSI)->getRawDataRefImpl()); ErrorOr SecName = EF.getSectionName(SymSec); if (std::error_code EC = SecName.getError()) @@ -967,8 +967,8 @@ static void DisassembleObject(const ObjectFile *Obj, bool InlineRelocs) { if (Name->empty()) continue; - ErrorOr SectionOrErr = Symbol.getSection(); - error(SectionOrErr.getError()); + Expected SectionOrErr = Symbol.getSection(); + error(errorToErrorCode(SectionOrErr.takeError())); section_iterator SecI = *SectionOrErr; if (SecI == Obj->section_end()) continue; @@ -1357,12 +1357,13 @@ void llvm::PrintSymbolTable(const ObjectFile *o) { ErrorOr AddressOrError = Symbol.getAddress(); error(AddressOrError.getError()); uint64_t Address = *AddressOrError; - ErrorOr TypeOrError = Symbol.getType(); - error(TypeOrError.getError()); + Expected TypeOrError = Symbol.getType(); + if (!TypeOrError) + report_error(o->getFileName(), TypeOrError.takeError()); SymbolRef::Type Type = *TypeOrError; uint32_t Flags = Symbol.getFlags(); - ErrorOr SectionOrErr = Symbol.getSection(); - error(SectionOrErr.getError()); + Expected SectionOrErr = Symbol.getSection(); + error(errorToErrorCode(SectionOrErr.takeError())); section_iterator Section = *SectionOrErr; StringRef Name; if (Type == SymbolRef::ST_Debug && Section != o->section_end()) { -- cgit v1.2.3