[Object] Change ObjectFile::getSymbolValue() return type to Expected<…

…uint64_t>

Summary:
In D77860, we have changed `getSymbolFlags()` return type to `Expected<uint32_t>`.
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

llvm/include/llvm/Object/ELFObjectFile.h

@@ -516,7 +516,12 @@ uint64_t ELFObjectFile<ELFT>::getSymbolValueImpl(DataRefImpl Symb) const {
 template <class ELFT>
 Expected<uint64_t>
 ELFObjectFile<ELFT>::getSymbolAddress(DataRefImpl Symb) const {
-  uint64_t Result = getSymbolValue(Symb);
+  Expected<uint64_t> 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:

llvm/include/llvm/Object/ObjectFile.h

@@ -188,7 +188,7 @@ class SymbolRef : public BasicSymbolRef {
 
   /// Return the value of the symbol depending on the object this can be an
   /// offset or a virtual address.
-  uint64_t getValue() const;
+  Expected<uint64_t> getValue() const;
 
   /// Get the alignment of this symbol as the actual value (not log 2).
   uint32_t getAlignment() const;
@@ -289,7 +289,7 @@ class ObjectFile : public SymbolicFile {
   virtual void getRelocationTypeName(DataRefImpl Rel,
                                      SmallVectorImpl<char> &Result) const = 0;
 
-  uint64_t getSymbolValue(DataRefImpl Symb) const;
+  Expected<uint64_t> getSymbolValue(DataRefImpl Symb) const;
 
 public:
   ObjectFile() = delete;
@@ -390,7 +390,7 @@ inline Expected<uint64_t> SymbolRef::getAddress() const {
   return getObject()->getSymbolAddress(getRawDataRefImpl());
 }
 
-inline uint64_t SymbolRef::getValue() const {
+inline Expected<uint64_t> SymbolRef::getValue() const {
   return getObject()->getSymbolValue(getRawDataRefImpl());
 }
 

llvm/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<uint64_t> 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";

llvm/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,

llvm/lib/Object/COFFObjectFile.cpp

@@ -166,7 +166,7 @@ uint32_t COFFObjectFile::getSymbolAlignment(DataRefImpl Ref) const {
 }
 
 Expected<uint64_t> 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();
 

llvm/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<uint64_t> ObjectFile::getSymbolValue(DataRefImpl Ref) const {
   if (Expected<uint32_t> 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);
 }
 

llvm/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<uint64_t> 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()) {

llvm/lib/XRay/InstrumentationMap.cpp

@@ -114,8 +114,11 @@ loadObj(StringRef Filename, object::OwningBinary<object::ObjectFile> &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<uint64_t> 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});

llvm/tools/dsymutil/DebugMap.cpp

@@ -254,7 +254,12 @@ MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
                            << toString(std::move(Err)) << '\n';
     } else {
       for (const auto &Sym : Object->symbols()) {
-        uint64_t Address = Sym.getValue();
+        Expected<uint64_t> AddressOrErr = Sym.getValue();
+        if (!AddressOrErr) {
+          // TODO: Actually report errors helpfully.
+          consumeError(AddressOrErr.takeError());
+          continue;
+        }
         Expected<StringRef> Name = Sym.getName();
         Expected<uint32_t> FlagsOrErr = Sym.getFlags();
         if (!Name || !FlagsOrErr ||
@@ -266,7 +271,7 @@ MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
             consumeError(Name.takeError());
           continue;
         }
-        SymbolAddresses[*Name] = Address;
+        SymbolAddresses[*Name] = *AddressOrErr;
       }
     }
   }

llvm/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<StringRef> 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<StringRef> NameOrErr = Sym.getName();
     if (!NameOrErr) {
       // TODO: Actually report errors helpfully.

llvm/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));
   }