From 0cc48af0ab9665e66039fbdcab8dea1da7ed4cff Mon Sep 17 00:00:00 2001 From: Xing GUO Date: Sat, 2 May 2020 14:04:04 +0800 Subject: [PATCH] [Object] Change ObjectFile::getSymbolValue() return type to Expected Summary: In D77860, we have changed `getSymbolFlags()` return type to `Expected`. This change helps bubble the error further up the stack. Reviewers: jhenderson, grimar, JDevlieghere, MaskRay Reviewed By: jhenderson Subscribers: hiraditya, MaskRay, rupprecht, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D79075 --- include/llvm/Object/ELFObjectFile.h | 7 ++++++- include/llvm/Object/ObjectFile.h | 6 +++--- lib/DebugInfo/GSYM/ObjectFileTransformer.cpp | 13 ++++++++---- .../RuntimeDyld/RuntimeDyldCOFF.cpp | 2 +- lib/Object/COFFObjectFile.cpp | 2 +- lib/Object/ObjectFile.cpp | 6 +++--- lib/Object/SymbolSize.cpp | 7 +++++-- lib/XRay/InstrumentationMap.cpp | 7 +++++-- tools/dsymutil/DebugMap.cpp | 9 +++++++-- tools/dsymutil/MachODebugMapParser.cpp | 4 ++-- tools/llvm-objdump/MachODump.cpp | 20 ++++++++++--------- 11 files changed, 53 insertions(+), 30 deletions(-) diff --git a/include/llvm/Object/ELFObjectFile.h b/include/llvm/Object/ELFObjectFile.h index b0aa86942a3..f6435d8b7cc 100644 --- a/include/llvm/Object/ELFObjectFile.h +++ b/include/llvm/Object/ELFObjectFile.h @@ -516,7 +516,12 @@ uint64_t ELFObjectFile::getSymbolValueImpl(DataRefImpl Symb) const { template Expected ELFObjectFile::getSymbolAddress(DataRefImpl Symb) const { - uint64_t Result = getSymbolValue(Symb); + Expected SymbolValueOrErr = getSymbolValue(Symb); + if (!SymbolValueOrErr) + // TODO: Test this error. + return SymbolValueOrErr.takeError(); + + uint64_t Result = *SymbolValueOrErr; const Elf_Sym *ESym = getSymbol(Symb); switch (ESym->st_shndx) { case ELF::SHN_COMMON: diff --git a/include/llvm/Object/ObjectFile.h b/include/llvm/Object/ObjectFile.h index e7d1dcaec9c..4d51430ffaf 100644 --- a/include/llvm/Object/ObjectFile.h +++ b/include/llvm/Object/ObjectFile.h @@ -188,7 +188,7 @@ public: /// Return the value of the symbol depending on the object this can be an /// offset or a virtual address. - uint64_t getValue() const; + Expected getValue() const; /// Get the alignment of this symbol as the actual value (not log 2). uint32_t getAlignment() const; @@ -289,7 +289,7 @@ protected: virtual void getRelocationTypeName(DataRefImpl Rel, SmallVectorImpl &Result) const = 0; - uint64_t getSymbolValue(DataRefImpl Symb) const; + Expected getSymbolValue(DataRefImpl Symb) const; public: ObjectFile() = delete; @@ -390,7 +390,7 @@ inline Expected SymbolRef::getAddress() const { return getObject()->getSymbolAddress(getRawDataRefImpl()); } -inline uint64_t SymbolRef::getValue() const { +inline Expected SymbolRef::getValue() const { return getObject()->getSymbolValue(getRawDataRefImpl()); } diff --git a/lib/DebugInfo/GSYM/ObjectFileTransformer.cpp b/lib/DebugInfo/GSYM/ObjectFileTransformer.cpp index c21083dde48..ad35aefe777 100644 --- a/lib/DebugInfo/GSYM/ObjectFileTransformer.cpp +++ b/lib/DebugInfo/GSYM/ObjectFileTransformer.cpp @@ -86,9 +86,14 @@ llvm::Error ObjectFileTransformer::convert(const object::ObjectFile &Obj, consumeError(SymType.takeError()); continue; } - const uint64_t Addr = Sym.getValue(); + Expected AddrOrErr = Sym.getValue(); + if (!AddrOrErr) + // TODO: Test this error. + return AddrOrErr.takeError(); + if (SymType.get() != SymbolRef::Type::ST_Function || - !Gsym.IsValidTextAddress(Addr) || Gsym.hasFunctionInfoForAddress(Addr)) + !Gsym.IsValidTextAddress(*AddrOrErr) || + Gsym.hasFunctionInfoForAddress(*AddrOrErr)) continue; // Function size for MachO files will be 0 constexpr bool NoCopy = false; @@ -102,8 +107,8 @@ llvm::Error ObjectFileTransformer::convert(const object::ObjectFile &Obj, // for mach-o files. if (IsMachO) Name->consume_front("_"); - Gsym.addFunctionInfo(FunctionInfo(Addr, size, - Gsym.insertString(*Name, NoCopy))); + Gsym.addFunctionInfo( + FunctionInfo(*AddrOrErr, size, Gsym.insertString(*Name, NoCopy))); } size_t FunctionsAddedCount = Gsym.getNumFunctionInfos() - NumBefore; Log << "Loaded " << FunctionsAddedCount << " functions from symbol table.\n"; diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp index da268efbeda..1d8f1ac8ac8 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp @@ -76,7 +76,7 @@ RuntimeDyldCOFF::loadObject(const object::ObjectFile &O) { uint64_t RuntimeDyldCOFF::getSymbolOffset(const SymbolRef &Sym) { // The value in a relocatable COFF object is the offset. - return Sym.getValue(); + return cantFail(Sym.getValue()); } uint64_t RuntimeDyldCOFF::getDLLImportOffset(unsigned SectionID, StubMap &Stubs, diff --git a/lib/Object/COFFObjectFile.cpp b/lib/Object/COFFObjectFile.cpp index ebb29ff8c9f..28233f8bdc7 100644 --- a/lib/Object/COFFObjectFile.cpp +++ b/lib/Object/COFFObjectFile.cpp @@ -166,7 +166,7 @@ uint32_t COFFObjectFile::getSymbolAlignment(DataRefImpl Ref) const { } Expected COFFObjectFile::getSymbolAddress(DataRefImpl Ref) const { - uint64_t Result = getSymbolValue(Ref); + uint64_t Result = cantFail(getSymbolValue(Ref)); COFFSymbolRef Symb = getCOFFSymbol(Ref); int32_t SectionNumber = Symb.getSectionNumber(); diff --git a/lib/Object/ObjectFile.cpp b/lib/Object/ObjectFile.cpp index fe903781144..61b36ea0f44 100644 --- a/lib/Object/ObjectFile.cpp +++ b/lib/Object/ObjectFile.cpp @@ -54,15 +54,15 @@ bool SectionRef::containsSymbol(SymbolRef S) const { return *this == **SymSec; } -uint64_t ObjectFile::getSymbolValue(DataRefImpl Ref) const { +Expected ObjectFile::getSymbolValue(DataRefImpl Ref) const { if (Expected FlagsOrErr = getSymbolFlags(Ref)) { if (*FlagsOrErr & SymbolRef::SF_Undefined) return 0; if (*FlagsOrErr & SymbolRef::SF_Common) return getCommonSymbolSize(Ref); } else - // TODO: Actually report errors helpfully. - report_fatal_error(FlagsOrErr.takeError()); + // TODO: Test this error. + return FlagsOrErr.takeError(); return getSymbolValueImpl(Ref); } diff --git a/lib/Object/SymbolSize.cpp b/lib/Object/SymbolSize.cpp index 04257f11d7d..84eed4d169d 100644 --- a/lib/Object/SymbolSize.cpp +++ b/lib/Object/SymbolSize.cpp @@ -61,8 +61,11 @@ llvm::object::computeSymbolSizes(const ObjectFile &O) { unsigned SymNum = 0; for (symbol_iterator I = O.symbol_begin(), E = O.symbol_end(); I != E; ++I) { SymbolRef Sym = *I; - uint64_t Value = Sym.getValue(); - Addresses.push_back({I, Value, SymNum, getSymbolSectionID(O, Sym)}); + Expected ValueOrErr = Sym.getValue(); + if (!ValueOrErr) + // TODO: Actually report errors helpfully. + report_fatal_error(ValueOrErr.takeError()); + Addresses.push_back({I, *ValueOrErr, SymNum, getSymbolSectionID(O, Sym)}); ++SymNum; } for (SectionRef Sec : O.sections()) { diff --git a/lib/XRay/InstrumentationMap.cpp b/lib/XRay/InstrumentationMap.cpp index b095d7134a5..cadaa4afeef 100644 --- a/lib/XRay/InstrumentationMap.cpp +++ b/lib/XRay/InstrumentationMap.cpp @@ -114,8 +114,11 @@ loadObj(StringRef Filename, object::OwningBinary &ObjFile, if (SupportsRelocation && SupportsRelocation(Reloc.getType())) { auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend(); auto A = AddendOrErr ? *AddendOrErr : 0; - uint64_t resolved = Resolver(Reloc, Reloc.getSymbol()->getValue(), A); - Relocs.insert({Reloc.getOffset(), resolved}); + Expected ValueOrErr = Reloc.getSymbol()->getValue(); + if (!ValueOrErr) + // TODO: Test this error. + return ValueOrErr.takeError(); + Relocs.insert({Reloc.getOffset(), Resolver(Reloc, *ValueOrErr, A)}); } else if (Reloc.getType() == RelativeRelocation) { if (auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend()) Relocs.insert({Reloc.getOffset(), *AddendOrErr}); diff --git a/tools/dsymutil/DebugMap.cpp b/tools/dsymutil/DebugMap.cpp index 3cd1bb0f7b3..0c637504224 100644 --- a/tools/dsymutil/DebugMap.cpp +++ b/tools/dsymutil/DebugMap.cpp @@ -254,7 +254,12 @@ MappingTraits::YamlDMO::denormalize(IO &IO) { << toString(std::move(Err)) << '\n'; } else { for (const auto &Sym : Object->symbols()) { - uint64_t Address = Sym.getValue(); + Expected AddressOrErr = Sym.getValue(); + if (!AddressOrErr) { + // TODO: Actually report errors helpfully. + consumeError(AddressOrErr.takeError()); + continue; + } Expected Name = Sym.getName(); Expected FlagsOrErr = Sym.getFlags(); if (!Name || !FlagsOrErr || @@ -266,7 +271,7 @@ MappingTraits::YamlDMO::denormalize(IO &IO) { consumeError(Name.takeError()); continue; } - SymbolAddresses[*Name] = Address; + SymbolAddresses[*Name] = *AddressOrErr; } } } diff --git a/tools/dsymutil/MachODebugMapParser.cpp b/tools/dsymutil/MachODebugMapParser.cpp index 42b3f2ecb2b..a61de9617b5 100644 --- a/tools/dsymutil/MachODebugMapParser.cpp +++ b/tools/dsymutil/MachODebugMapParser.cpp @@ -478,7 +478,7 @@ void MachODebugMapParser::loadCurrentObjectFileSymbols( CurrentObjectAddresses.clear(); for (auto Sym : Obj.symbols()) { - uint64_t Addr = Sym.getValue(); + uint64_t Addr = cantFail(Sym.getValue()); Expected Name = Sym.getName(); if (!Name) { // TODO: Actually report errors helpfully. @@ -562,7 +562,7 @@ void MachODebugMapParser::loadMainBinarySymbols( Section = *SectionOrErr; if (Section == MainBinary.section_end() || Section->isText()) continue; - uint64_t Addr = Sym.getValue(); + uint64_t Addr = cantFail(Sym.getValue()); Expected NameOrErr = Sym.getName(); if (!NameOrErr) { // TODO: Actually report errors helpfully. diff --git a/tools/llvm-objdump/MachODump.cpp b/tools/llvm-objdump/MachODump.cpp index 6a66a16b700..6d46496ecd4 100644 --- a/tools/llvm-objdump/MachODump.cpp +++ b/tools/llvm-objdump/MachODump.cpp @@ -230,8 +230,10 @@ struct SymbolSorter { if (!BTypeOrErr) reportError(BTypeOrErr.takeError(), B.getObject()->getFileName()); SymbolRef::Type BType = *BTypeOrErr; - uint64_t AAddr = (AType != SymbolRef::ST_Function) ? 0 : A.getValue(); - uint64_t BAddr = (BType != SymbolRef::ST_Function) ? 0 : B.getValue(); + uint64_t AAddr = + (AType != SymbolRef::ST_Function) ? 0 : cantFail(A.getValue()); + uint64_t BAddr = + (BType != SymbolRef::ST_Function) ? 0 : cantFail(B.getValue()); return AAddr < BAddr; } }; @@ -1267,7 +1269,7 @@ static void CreateSymbolAddressMap(MachOObjectFile *O, SymbolRef::Type ST = unwrapOrError(Symbol.getType(), FileName); if (ST == SymbolRef::ST_Function || ST == SymbolRef::ST_Data || ST == SymbolRef::ST_Other) { - uint64_t Address = Symbol.getValue(); + uint64_t Address = cantFail(Symbol.getValue()); StringRef SymName = unwrapOrError(Symbol.getName(), FileName); if (!SymName.startswith(".objc")) (*AddrMap)[Address] = SymName; @@ -3352,7 +3354,7 @@ static const char *get_symbol_64(uint32_t sect_offset, SectionRef S, // and return its name. const char *SymbolName = nullptr; if (reloc_found && isExtern) { - n_value = Symbol.getValue(); + n_value = cantFail(Symbol.getValue()); StringRef Name = unwrapOrError(Symbol.getName(), info->O->getFileName()); if (!Name.empty()) { SymbolName = Name.data(); @@ -6908,7 +6910,7 @@ static const char *GuessLiteralPointer(uint64_t ReferenceValue, if (info->O->getAnyRelocationPCRel(RE)) { unsigned Type = info->O->getAnyRelocationType(RE); if (Type == MachO::X86_64_RELOC_SIGNED) { - ReferenceValue = Symbol.getValue(); + ReferenceValue = cantFail(Symbol.getValue()); } } } @@ -7449,7 +7451,7 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, unwrapOrError(Symbol.getType(), MachOOF->getFileName()); if (ST == SymbolRef::ST_Function || ST == SymbolRef::ST_Data || ST == SymbolRef::ST_Other) { - uint64_t Address = Symbol.getValue(); + uint64_t Address = cantFail(Symbol.getValue()); StringRef SymName = unwrapOrError(Symbol.getName(), MachOOF->getFileName()); AddrMap[Address] = SymName; @@ -7528,7 +7530,7 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, // Start at the address of the symbol relative to the section's address. uint64_t SectSize = Sections[SectIdx].getSize(); - uint64_t Start = Symbols[SymIdx].getValue(); + uint64_t Start = cantFail(Symbols[SymIdx].getValue()); uint64_t SectionAddress = Sections[SectIdx].getAddress(); Start -= SectionAddress; @@ -7549,7 +7551,7 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF, if (NextSymType == SymbolRef::ST_Function) { containsNextSym = Sections[SectIdx].containsSymbol(Symbols[NextSymIdx]); - NextSym = Symbols[NextSymIdx].getValue(); + NextSym = cantFail(Symbols[NextSymIdx].getValue()); NextSym -= SectionAddress; break; } @@ -8208,7 +8210,7 @@ void objdump::printMachOUnwindInfo(const MachOObjectFile *Obj) { if (Section == Obj->section_end()) continue; - uint64_t Addr = SymRef.getValue(); + uint64_t Addr = cantFail(SymRef.getValue()); Symbols.insert(std::make_pair(Addr, SymRef)); }