[Object] Fix crash caused by unhandled error.

[Higuoxing]

Summary

The return type of ELFFile<ELFT>::symbols(const Elf_Shdr*) is Expected<Elf_Sym_Range>, and we should handle the error or consume it when this operation fails.

[MaskRay]

llvm/include/llvm/Object/ELFObjectFile.h

630

Use -> since you are touching this line.

[Higuoxing]

Addressed @MaskRay 's comment.

[jhenderson]

llvm/include/llvm/Object/ELFObjectFile.h

629–647

I wonder if this should be changed to not use auto? I think that would have made it less likely for this mistake to have got through unnoticed.

635

Ditto.

llvm/test/tools/llvm-nm/X86/invalid-section-size.test

1 ↗

(On Diff #254437)

"when dumping a symbol table"

I wonder if this test should be moved to the Object test folder, since this is really testing the Object library, rather than llvm-nm specifically. What do you think?

3 ↗

(On Diff #254437)

Perhaps make this last line a TODO in the test.

6 ↗

(On Diff #254437)

It might be a good idea to add 2>&1 to the llvm-nm run and --implicit-check-not=warning to the FileCheck run so that this test starts failing if somebody implements the warning handling described, so that the test can be updated at that point, rather than it becoming redundant.

[Higuoxing]

Addressed comments.

• Use Expected<Elf_Sym_Range> instead of auto.
• Add TODO label for future improvement.
• Move test to Object folder.

[Higuoxing]

Apply clang-format

[grimar]

llvm/include/llvm/Object/ELFObjectFile.h

648

Perhaps it worth to generalize this place a bit? Something like:

auto CheckSymbol = [&](Expected<typename ELFT::SymRange> R) {
  if (!R || ESym == R->begin()) {
    Result |= SymbolRef::SF_FormatSpecific;
    // TODO: Actually report errors helpfully.
    consumeError(R.takeError());
    return false;
  }
  return true;
};

if (!CheckSymbol(EF.symbols(DotSymtabSec)) ||
    !CheckSymbol(EF.symbols(DotDynSymSec)))
  return Result;

[jhenderson]

llvm/test/Object/nm-invalid-section-size.test

1 ↗

(On Diff #254451)

You should rename this to invalid-symtab-size.test and delete the "nm" part of the title. Again, this isn't about testing llvm-nm, but rather the Object library's invalid symbol table section size handling.

3 ↗

(On Diff #254451)

will -> would

[Higuoxing]

Addressed comments.

• Rename test file name to invalid-symtab-size.test
• Generalize CheckSymbol()

[Higuoxing]

.

[grimar]

llvm/include/llvm/Object/ELFObjectFile.h

241

You probably do not need it anymore as it is used in the single place now.

628–629

With this implementation the CheckSymbol name probably does not make much sense anymore?
It is unclear what does it check I think. It perhaps should be changed to IsFormatSpecificSymbol or something better.

In this case I'd expect to see a bit more related code inside.
E.g. seems the following piece from above:

if (ESym->getType() == ELF::STT_FILE || ESym->getType() == ELF::STT_SECTION)
  Result |= SymbolRef::SF_FormatSpecific;

and the code for ARM from below :

if (EF.getHeader()->e_machine == ELF::EM_ARM) {
    if (Expected<StringRef> NameOrErr = getSymbolName(Sym)) {
      StringRef Name = *NameOrErr;
      if (Name.startswith("$d") || Name.startswith("$t") ||
          Name.startswith("$a"))
        Result |= SymbolRef::SF_FormatSpecific;

can live there so that IsFormatSpecificSymbol would return true for all cases when we want to set the SF_FormatSpecific flag.

630

This condition needs a comment to explain what it does.

[grimar]

llvm/include/llvm/Object/ELFObjectFile.h

628–629

Let me debug this to see how well my suggestions works. I'll return with something soon.

[Higuoxing]

llvm/include/llvm/Object/ELFObjectFile.h

628–629

What do you think of changing the name to IsFirstNULLSymbol(SomeSec) or IsFirstNULLSymbolFrom(SomeSec), and leave other checkers unchanged?

We have to call the function on .symtab and .dynsym to check whether the symbol is NULL symbol. If we want to add one another function IsFormatSpecificSymbol(), we still have to keep IsFirstNULLSymbol()

bool IsFormatSpecificSymbol() {
  if (IsFirstNULLSymbol(.symtab) || IsFirstSymbol(.dynsym))
    return true;
  else if (EF.getHeader()->e_machine == ELF::EM_ARM) {
    ...
    return true;
  } else if (ESym->getType() == STT_FILE || ESym->getType() == STT_SECTION)
    return true;
  return false;
}

[grimar]

llvm/include/llvm/Object/ELFObjectFile.h

628–629

Right. I've experimented with the code and came to the same conclusion:

auto IsNullSymbol = [this](const Elf_Sym *Sym, const Elf_Shdr *Symtab) {
  Expected<typename ELFT::SymRange> SymbolsOrErr = EF.symbols(Symtab);
  if (!SymbolsOrErr) {
    // TODO: Actually report errors helpfully.
    consumeError(SymbolsOrErr.takeError());
    return false;
  }
  return Sym == SymbolsOrErr->begin();
};

if (IsNullSymbol(ESym, DotSymtabSec) || IsNullSymbol(ESym, DotDynSymSec))
  Result |= SymbolRef::SF_FormatSpecific;

Creating a IsFormatSpecificSymbol could be a nice follow-up refactoring probably.

So just renaming your version to to IsNullSymbol LG to me.

One more thing though: your test check the broken .symtab case, but the code also checks the .dynsym.
This should also be covered by the test I think.

[grimar]

llvm/include/llvm/Object/ELFObjectFile.h

630

And probably no need to add a comment then as IsNullSymbol describes nicely what the code does.

[Higuoxing]

Addressed comment.

• Add one more test for .dynsym section. llvm-nm --dynamic's behavior will be fixed until D76081 lands.

[jhenderson]

Thinking about it, I'm reluctant to approve this as is, without a further change to actually report the error (possibly in a seperate patch). Prior to this patch, we have a failure that at least can be identified in certain build versions, whereas now the malformed-ness is not detectable at all. Perhaps this could temporarily use report_fatal_error with a follow-up patch bubbling the error further up the stack?

llvm/include/llvm/Object/ELFObjectFile.h

628–629

I think IsNullSymbol would be fine, and more in keeping with LLVM naming standards.

llvm/test/Object/invalid-symtab-size.test

13 ↗

(On Diff #254507)

FIXME rather than TODO here.

[grimar]

I'd also recommend to start using "Edit related revisions" on the right panel of the phab.
Then you should be able to edit related parent/child revisions, because currently it is unobvious which dependencies your patches has.

llvm/include/llvm/Object/ELFObjectFile.h

630

IsNullSymbol, not IsNULLSymbol

(if you search for "IsNULL" in LLVM's code, you'll see 0 matching lines, but "IsNull" gives a lot of.
So having NULL is at least inconsistent).

[jhenderson]

In D77289#1959092, @grimar wrote:

I'd also recommend to start using "Edit related revisions" on the right panel of the phab.
Then you should be able to edit related parent/child revisions, because currently it is unobvious which dependencies your patches has.

Alternatively, you can put "Depends on D<number>" in the patch description and the stack is automatically formed without you needing to manually edit the related revisions.

[Higuoxing]

In D77289#1959092, @grimar wrote:

I'd also recommend to start using "Edit related revisions" on the right panel of the phab.
Then you should be able to edit related parent/child revisions, because currently it is unobvious which dependencies your patches has.

In D77289#1959211, @jhenderson wrote:

Alternatively, you can put "Depends on D<number>" in the patch description and the stack is automatically formed without you needing to manually edit the related revisions.

Thanks a lot! Got it.

In D77289#1959072, @jhenderson wrote:

Thinking about it, I'm reluctant to approve this as is, without a further change to actually report the error (possibly in a seperate patch). Prior to this patch, we have a failure that at least can be identified in certain build versions, whereas now the malformed-ness is not detectable at all. Perhaps this could temporarily use report_fatal_error with a follow-up patch bubbling the error further up the stack?

I've tried to change uint32_t getSymbolFlags(DataRefImpl) to Expected<uint32_t> getSymbolFlags(DataRefImpl). However, there are several tools, object file formats and functions from core library involved. I tried to update them all carefully. Could you please give me some comments?

FYI: getSymbolFlags() on ELFObjectFile is fallible while on other object files (Mach-O, COFF, XCOFF ...) are not.

[sbc100]

llvm/include/llvm/Object/ObjectFile.h

306

Use early return here?

auto SymbolFlagsOrErr = getSymbolFlags(Symb);
if (!SymbolFlagsOrErr)
   report_fatal_error(SymbolFlagsOrErr.takeError());
assert(*SymbolFlagsOrErr & SymbolRef::SF_Common);
return getCommonSymbolSizeImpl(Symb);

This would also avoid "else-after-return".

llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp

93

Ditto, use early return and avoid else after return.

llvm/tools/llvm-size/llvm-size.cpp

213

Use early return?

[Higuoxing]

Addressed comments.

• Use early-return style.

[jhenderson]

Thanks for the change. I think the interface change makes sense (and is in keeping with other similar functions). However, I think it may deserve pulling into a separate change from the new error check. That would make it an NFC interface change. This change would then implement the behaviour change and add appropriate testing for said behaviour, which would cover some of the new code paths.

As for the other code paths I've highlighted, it's probably okay to just put a TODO saying "needs testing" or something to that effect for most of them. @grimar, what do you think?

llvm/include/llvm/Object/ObjectFile.h

301

It might be worth putting a TODO here saying that this should be pushed further up the stack. Probably also wants mentioning that this is untested currently.

llvm/lib/ExecutionEngine/Orc/Mangling.cpp

98

This new bit of code should either be tested or at least have a TODO comment saying it needs to be.

llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp

130

Test?

212

Test?

llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp

60

Test?

84

TODO for pushing error up stack + test.

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

219

Test?

242

Test?

602

Test?

llvm/lib/Object/ArchiveWriter.cpp

269

Test/TODO to echo further up stack?

llvm/lib/Object/MachOObjectFile.cpp

1810

cantFail(FlagsOrErr.takeError())

Then you can delete the comment.

llvm/test/tools/llvm-nm/invalid-symbol-table-size.test

1

a helpful error message

Ditto in other test.

13

I don't think you need the CHECK-EMPTY for when an error is reported.

Ditto below.

llvm/tools/dsymutil/MachODebugMapParser.cpp

499

cantFail

llvm/tools/llvm-nm/llvm-nm.cpp

312

If it's impossible for this to fail, use cantFail instead. Same below and elsewhere.

380

cantFail

1059

cantFail

1069

cantFail

1103

Why is consumeError here okay? Should it be cantFail? A comment is probably needed anyway.

llvm/tools/llvm-objdump/MachODump.cpp

7565

cantFail

llvm/tools/llvm-size/llvm-size.cpp

204–205

You can probably directly call llvm-size's error reporting routine here, since this is in the core llvm-size code. You would also need to test it then.

llvm/tools/sancov/sancov.cpp

661

Test?

[grimar]

In D77289#1971375, @jhenderson wrote:

However, I think it may deserve pulling into a separate change from the new error check. That would make it an NFC interface change.

+1.

As for the other code paths I've highlighted, it's probably okay to just put a TODO saying "needs testing" or something to that effect for most of them. @grimar, what do you think?

I agree. It might be hard/take time to add test cases for all places, but it should not be a stopper for us I believe.

llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h

void mapSectionAddress(const void *LocalAddress,
       assert(PFC && "mapSectionAddress called on finalized LinkedObject");
       assert(PFC->RTDyld && "mapSectionAddress called on raw LinkedObject");
       PFC->RTDyld->mapSectionAddress(LocalAddress, TargetAddr);
     }
 
   private:
     void buildInitialSymbolTable(const OwnedObject &Obj) {
       for (auto &Symbol : Obj.getBinary()->symbols()) {
-        if (Symbol.getFlags() & object::SymbolRef::SF_Undefined)
+        auto SymbolFlagsOrErr = Symbol.getFlags();
+        // FIXME: Raise an error for bad symbols.
+        if (!SymbolFlagsOrErr) {
+          consumeError(SymbolFlagsOrErr.takeError());
+          continue;
+        }
+
+        if (*SymbolFlagsOrErr & object::SymbolRef::SF_Undefined)
           continue;
         Expected<StringRef> SymbolName = Symbol.getName();
         // FIXME: Raise an error for bad symbols.
         if (!SymbolName) {
           consumeError(SymbolName.takeError());
           continue;
         }
         // FIXME: Raise an error for bad symbols.

llvm/include/llvm/Object/COFF.h

 
 protected:
   void moveSymbolNext(DataRefImpl &Symb) const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
   Expected<ArrayRef<uint8_t>>

llvm/include/llvm/Object/COFFImportFile.h

public:
 
   Error printSymbolName(raw_ostream &OS, DataRefImpl Symb) const override {
     if (Symb.p == 0)
       OS << "__imp_";
     OS << StringRef(Data.getBufferStart() + sizeof(coff_import_header));
     return Error::success();
   }
 
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override {
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override {
     return SymbolRef::SF_Global;
   }
 
   basic_symbol_iterator symbol_begin() const override {
     return BasicSymbolRef(DataRefImpl(), this);
   }
 
   basic_symbol_iterator symbol_end() const override {

llvm/include/llvm/Object/ELFObjectFile.h

public:
   using uintX_t = typename ELFT::uint;
 
   using Elf_Sym = typename ELFT::Sym;
   using Elf_Shdr = typename ELFT::Shdr;
   using Elf_Ehdr = typename ELFT::Ehdr;
   using Elf_Rel = typename ELFT::Rel;
   using Elf_Rela = typename ELFT::Rela;
   using Elf_Dyn = typename ELFT::Dyn;
 
   SectionRef toSectionRef(const Elf_Shdr *Sec) const {
     return SectionRef(toDRI(Sec), this);
   }
 
 private:
   ELFObjectFile(MemoryBufferRef Object, ELFFile<ELFT> EF,
                 const Elf_Shdr *DotDynSymSec, const Elf_Shdr *DotSymtabSec,
                 ArrayRef<Elf_Word> ShndxTable);
 
 protected:
   ELFFile<ELFT> EF;
 
   const Elf_Shdr *DotDynSymSec = nullptr; // Dynamic symbol table section.
   const Elf_Shdr *DotSymtabSec = nullptr; // Symbol table section.
   ArrayRef<Elf_Word> ShndxTable;
 
   void moveSymbolNext(DataRefImpl &Symb) const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   uint8_t getSymbolBinding(DataRefImpl Symb) const override;
   uint8_t getSymbolOther(DataRefImpl Symb) const override;
   uint8_t getSymbolELFType(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(const Elf_Sym *Symb,
                                               const Elf_Shdr *SymTab) const;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
 
ELFObjectFile<ELFT>::getSymbolType(DataRefImpl Symb) const {
   case ELF::STT_TLS:
     return SymbolRef::ST_Data;
   default:
     return SymbolRef::ST_Other;
   }
 }
 
 template <class ELFT>
-uint32_t ELFObjectFile<ELFT>::getSymbolFlags(DataRefImpl Sym) const {
+Expected<uint32_t> ELFObjectFile<ELFT>::getSymbolFlags(DataRefImpl Sym) const {
   const Elf_Sym *ESym = getSymbol(Sym);
 
   uint32_t Result = SymbolRef::SF_None;
 
   if (ESym->getBinding() != ELF::STB_LOCAL)
     Result |= SymbolRef::SF_Global;
 
   if (ESym->getBinding() == ELF::STB_WEAK)
     Result |= SymbolRef::SF_Weak;
 
   if (ESym->st_shndx == ELF::SHN_ABS)
     Result |= SymbolRef::SF_Absolute;
 
   if (ESym->getType() == ELF::STT_FILE || ESym->getType() == ELF::STT_SECTION)
     Result |= SymbolRef::SF_FormatSpecific;
 
-  auto DotSymtabSecSyms = EF.symbols(DotSymtabSec);
-  if (DotSymtabSecSyms && ESym == (*DotSymtabSecSyms).begin())
+  auto IsNullSymbol = [this](const Elf_Sym *Sym,
+                             const Elf_Shdr *SymSec) -> Expected<bool> {
+    if (auto SymbolsOrErr = EF.symbols(SymSec))
+      return Sym == SymbolsOrErr->begin();
+    else
+      return SymbolsOrErr.takeError();
+  };
+
+  if (auto IsNullSymOrErr = IsNullSymbol(ESym, DotSymtabSec)) {
+    if (*IsNullSymOrErr)
       Result |= SymbolRef::SF_FormatSpecific;
-  auto DotDynSymSecSyms = EF.symbols(DotDynSymSec);
-  if (DotDynSymSecSyms && ESym == (*DotDynSymSecSyms).begin())
+  } else
+    return IsNullSymOrErr.takeError();
+
+  if (auto IsNullSymOrErr = IsNullSymbol(ESym, DotDynSymSec)) {
+    if (*IsNullSymOrErr)
       Result |= SymbolRef::SF_FormatSpecific;
+  } else
+    return IsNullSymOrErr.takeError();
 
   if (EF.getHeader()->e_machine == ELF::EM_ARM) {
     if (Expected<StringRef> NameOrErr = getSymbolName(Sym)) {
       StringRef Name = *NameOrErr;
       if (Name.startswith("$d") || Name.startswith("$t") ||
           Name.startswith("$a"))
         Result |= SymbolRef::SF_FormatSpecific;
-    } else {
-      // TODO: Actually report errors helpfully.
-      consumeError(NameOrErr.takeError());
-    }
+    } else
+      return NameOrErr.takeError();
+
     if (ESym->getType() == ELF::STT_FUNC && (ESym->st_value & 1) == 1)
       Result |= SymbolRef::SF_Thumb;
   }
 
   if (ESym->st_shndx == ELF::SHN_UNDEF)
     Result |= SymbolRef::SF_Undefined;
 
   if (ESym->getType() == ELF::STT_COMMON || ESym->st_shndx == ELF::SHN_COMMON)

llvm/include/llvm/Object/IRObjectFile.h

class IRObjectFile : public SymbolicFile {
   ModuleSymbolTable SymTab;
   IRObjectFile(MemoryBufferRef Object,
                std::vector<std::unique_ptr<Module>> Mods);
 
 public:
   ~IRObjectFile() override;
   void moveSymbolNext(DataRefImpl &Symb) const override;
   Error printSymbolName(raw_ostream &OS, DataRefImpl Symb) const override;
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
   StringRef getTargetTriple() const;
 
   static bool classof(const Binary *v) {
     return v->isIR();
   }

llvm/include/llvm/Object/MachO.h

public:
 
   std::error_code getIndirectName(DataRefImpl Symb, StringRef &Res) const;
   unsigned getSectionType(SectionRef Sec) const;
 
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
   unsigned getSymbolSectionID(SymbolRef Symb) const;
   unsigned getSectionID(SectionRef Sec) const;
 
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;

llvm/include/llvm/Object/ObjectFile.h

protected:
 
   uint64_t getSymbolValue(DataRefImpl Symb) const;
 
 public:
   ObjectFile() = delete;
   ObjectFile(const ObjectFile &other) = delete;
 
   uint64_t getCommonSymbolSize(DataRefImpl Symb) const {
-    assert(getSymbolFlags(Symb) & SymbolRef::SF_Common);
+    auto SymbolFlagsOrErr = getSymbolFlags(Symb);
+    if (!SymbolFlagsOrErr)
+      report_fatal_error(SymbolFlagsOrErr.takeError());
+    assert(*SymbolFlagsOrErr & SymbolRef::SF_Common);
     return getCommonSymbolSizeImpl(Symb);
   }
 
   virtual std::vector<SectionRef> dynamic_relocation_sections() const {
     return std::vector<SectionRef>();
   }
 
   using symbol_iterator_range = iterator_range<symbol_iterator>;
   symbol_iterator_range symbols() const {
     return symbol_iterator_range(symbol_begin(), symbol_end());
   }
 

llvm/include/llvm/Object/SymbolicFile.h

public:
   bool operator==(const BasicSymbolRef &Other) const;
   bool operator<(const BasicSymbolRef &Other) const;
 
   void moveNext();
 
   Error printName(raw_ostream &OS) const;
 
   /// Get symbol flags (bitwise OR of SymbolRef::Flags)
-  uint32_t getFlags() const;
+  Expected<uint32_t> getFlags() const;
 
   DataRefImpl getRawDataRefImpl() const;
   const SymbolicFile *getObject() const;
 };
 
 using basic_symbol_iterator = content_iterator<BasicSymbolRef>;
 
 class SymbolicFile : public Binary {
 public:
   SymbolicFile(unsigned int Type, MemoryBufferRef Source);
   ~SymbolicFile() override;
 
   // virtual interface.
   virtual void moveSymbolNext(DataRefImpl &Symb) const = 0;
 
   virtual Error printSymbolName(raw_ostream &OS, DataRefImpl Symb) const = 0;
 
-  virtual uint32_t getSymbolFlags(DataRefImpl Symb) const = 0;
+  virtual Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const = 0;
 
   virtual basic_symbol_iterator symbol_begin() const = 0;
 
   virtual basic_symbol_iterator symbol_end() const = 0;
 
   // convenience wrappers.
   using basic_symbol_iterator_range = iterator_range<basic_symbol_iterator>;
   basic_symbol_iterator_range symbols() const {

 inline void BasicSymbolRef::moveNext() {
   return OwningObject->moveSymbolNext(SymbolPimpl);
 }
 
 inline Error BasicSymbolRef::printName(raw_ostream &OS) const {
   return OwningObject->printSymbolName(OS, SymbolPimpl);
 }
 
-inline uint32_t BasicSymbolRef::getFlags() const {
+inline Expected<uint32_t> BasicSymbolRef::getFlags() const {
   return OwningObject->getSymbolFlags(SymbolPimpl);
 }
 
 inline DataRefImpl BasicSymbolRef::getRawDataRefImpl() const {
   return SymbolPimpl;
 }
 
 inline const SymbolicFile *BasicSymbolRef::getObject() const {
   return OwningObject;
 }
 
 } // end namespace object
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_SYMBOLICFILE_H

llvm/include/llvm/Object/TapiFile.h

public:
   TapiFile(MemoryBufferRef Source, const MachO::InterfaceFile &interface,
            MachO::Architecture Arch);
   ~TapiFile() override;
 
   void moveSymbolNext(DataRefImpl &DRI) const override;
 
   Error printSymbolName(raw_ostream &OS, DataRefImpl DRI) const override;
 
-  uint32_t getSymbolFlags(DataRefImpl DRI) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl DRI) const override;
 
   basic_symbol_iterator symbol_begin() const override;
 
   basic_symbol_iterator symbol_end() const override;
 
   static bool classof(const Binary *v) { return v->isTapiFile(); }
 
 private:

llvm/include/llvm/Object/Wasm.h

public:
   ArrayRef<wasm::WasmFunctionName> debugNames() const { return DebugNames; }
   uint32_t startFunction() const { return StartFunction; }
   uint32_t getNumImportedGlobals() const { return NumImportedGlobals; }
   uint32_t getNumImportedFunctions() const { return NumImportedFunctions; }
   uint32_t getNumImportedEvents() const { return NumImportedEvents; }
   uint32_t getNumSections() const { return Sections.size(); }
   void moveSymbolNext(DataRefImpl &Symb) const override;
 
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
 
   basic_symbol_iterator symbol_begin() const override;
 
   basic_symbol_iterator symbol_end() const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
 
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint64_t getWasmSymbolValue(const WasmSymbol &Sym) const;

llvm/include/llvm/Object/XCOFFObjectFile.h

private:
   void checkSectionAddress(uintptr_t Addr, uintptr_t TableAddr) const;
 
 public:
   static constexpr uint64_t InvalidRelocOffset =
       std::numeric_limits<uint64_t>::max();
 
   // Interface inherited from base classes.
   void moveSymbolNext(DataRefImpl &Symb) const override;
-  uint32_t getSymbolFlags(DataRefImpl Symb) const override;
+  Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;

llvm/lib/ExecutionEngine/Orc/Mangling.cpp

getObjectSymbolInfo(ExecutionSession &ES, MemoryBufferRef ObjBuffer) {
 
   if (!Obj)
     return Obj.takeError();
 
   bool IsMachO = isa<object::MachOObjectFile>(Obj->get());
 
   SymbolFlagsMap SymbolFlags;
   for (auto &Sym : (*Obj)->symbols()) {
+    auto SymFlagsOrErr = Sym.getFlags();
+    if (!SymFlagsOrErr)
+      return SymFlagsOrErr.takeError();
+
     // Skip symbols not defined in this object file.
-    if (Sym.getFlags() & object::BasicSymbolRef::SF_Undefined)
+    if (*SymFlagsOrErr & object::BasicSymbolRef::SF_Undefined)
       continue;
 
     // Skip symbols that are not global.
-    if (!(Sym.getFlags() & object::BasicSymbolRef::SF_Global))
+    if (!(*SymFlagsOrErr & object::BasicSymbolRef::SF_Global))
       continue;
 
     // Skip symbols that have type SF_File.
     if (auto SymType = Sym.getType()) {
       if (*SymType == object::SymbolRef::ST_File)
         continue;
     } else
       return SymType.takeError();

llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp

for (auto &Sym : (*Obj)->symbols()) {
         if (*SymType == object::SymbolRef::ST_File)
           continue;
       } else {
         ES.reportError(SymType.takeError());
         R.failMaterialization();
         return;
       }
 
+      auto SymFlags = Sym.getFlags();
+      if (!SymFlags) {
+        ES.reportError(SymFlags.takeError());
+        R.failMaterialization();
+        return;
+      }
+
       // Don't include symbols that aren't global.
-      if (!(Sym.getFlags() & object::BasicSymbolRef::SF_Global)) {
+      if (!(*SymFlags & object::BasicSymbolRef::SF_Global)) {
         if (auto SymName = Sym.getName())
           InternalSymbols->insert(*SymName);
         else {
           ES.reportError(SymName.takeError());
           R.failMaterialization();
           return;
         }
       }
Error RTDyldObjectLinkingLayer::onObjLoad(
   // (See http://llvm.org/PR40074)
   if (auto *COFFObj = dyn_cast<object::COFFObjectFile>(&Obj)) {
     auto &ES = getExecutionSession();
 
     // For all resolved symbols that are not already in the responsibilty set:
     // check whether the symbol is in a comdat section and if so mark it as
     // weak.
     for (auto &Sym : COFFObj->symbols()) {
-      if (Sym.getFlags() & object::BasicSymbolRef::SF_Undefined)
+      auto SymFlags = Sym.getFlags();
+      if (!SymFlags)
+        return SymFlags.takeError();
+      if (*SymFlags & object::BasicSymbolRef::SF_Undefined)
         continue;
       auto Name = Sym.getName();
       if (!Name)
         return Name.takeError();
       auto I = Resolved.find(*Name);
 
       // Skip unresolved symbols, internal symbols, and symbols that are
       // already in the responsibility set.

llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp

JITSymbolFlags llvm::JITSymbolFlags::fromSummary(GlobalValueSummary *S) {
   if (isa<FunctionSummary>(S))
     Flags |= JITSymbolFlags::Callable;
 
   return Flags;
 }
 
 Expected<JITSymbolFlags>
 llvm::JITSymbolFlags::fromObjectSymbol(const object::SymbolRef &Symbol) {
+  auto SymbolFlagsOrErr = Symbol.getFlags();
+  if (!SymbolFlagsOrErr)
+    return SymbolFlagsOrErr.takeError();
+
   JITSymbolFlags Flags = JITSymbolFlags::None;
-  if (Symbol.getFlags() & object::BasicSymbolRef::SF_Weak)
+  if (*SymbolFlagsOrErr & object::BasicSymbolRef::SF_Weak)
     Flags |= JITSymbolFlags::Weak;
-  if (Symbol.getFlags() & object::BasicSymbolRef::SF_Common)
+  if (*SymbolFlagsOrErr & object::BasicSymbolRef::SF_Common)
     Flags |= JITSymbolFlags::Common;
-  if (Symbol.getFlags() & object::BasicSymbolRef::SF_Exported)
+  if (*SymbolFlagsOrErr & object::BasicSymbolRef::SF_Exported)
     Flags |= JITSymbolFlags::Exported;
 
   auto SymbolType = Symbol.getType();
   if (!SymbolType)
     return SymbolType.takeError();
 
   if (*SymbolType & object::SymbolRef::ST_Function)
     Flags |= JITSymbolFlags::Callable;
 
   return Flags;
 }
 
 ARMJITSymbolFlags
 llvm::ARMJITSymbolFlags::fromObjectSymbol(const object::SymbolRef &Symbol) {
+  auto SymbolFlagsOrErr = Symbol.getFlags();
+  if (!SymbolFlagsOrErr)
+    report_fatal_error(SymbolFlagsOrErr.takeError());
   ARMJITSymbolFlags Flags;
-  if (Symbol.getFlags() & object::BasicSymbolRef::SF_Thumb)
+  if (*SymbolFlagsOrErr & object::BasicSymbolRef::SF_Thumb)
     Flags |= ARMJITSymbolFlags::Thumb;
   return Flags;
 }
 
 /// Performs lookup by, for each symbol, first calling
 ///        findSymbolInLogicalDylib and if that fails calling
 ///        findSymbol.
 void LegacyJITSymbolResolver::lookup(const LookupSet &Symbols,
                                      OnResolvedFunction OnResolved) {
   JITSymbolResolver::LookupResult Result;
   for (auto &Symbol : Symbols) {
     std::string SymName = Symbol.str();
     if (auto Sym = findSymbolInLogicalDylib(SymName)) {
       if (auto AddrOrErr = Sym.getAddress())
         Result[Symbol] = JITEvaluatedSymbol(*AddrOrErr, Sym.getFlags());

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

RuntimeDyldImpl::loadObjectImpl(const object::ObjectFile &Obj) {
   uint32_t CommonAlign = 0;
 
   // First, collect all weak and common symbols. We need to know if stronger
   // definitions occur elsewhere.
   JITSymbolResolver::LookupSet ResponsibilitySet;
   {
     JITSymbolResolver::LookupSet Symbols;
     for (auto &Sym : Obj.symbols()) {
-      uint32_t Flags = Sym.getFlags();
-      if ((Flags & SymbolRef::SF_Common) || (Flags & SymbolRef::SF_Weak)) {
+      auto FlagsOrErr = Sym.getFlags();
+      if (!FlagsOrErr)
+        return FlagsOrErr.takeError();
+      if ((*FlagsOrErr & SymbolRef::SF_Common) ||
+          (*FlagsOrErr & SymbolRef::SF_Weak)) {
         // Get symbol name.
         if (auto NameOrErr = Sym.getName())
           Symbols.insert(*NameOrErr);
         else
           return NameOrErr.takeError();
       }
     }
 
     if (auto ResultOrErr = Resolver.getResponsibilitySet(Symbols))
       ResponsibilitySet = std::move(*ResultOrErr);
     else
       return ResultOrErr.takeError();
   }
 
   // Parse symbols
   LLVM_DEBUG(dbgs() << "Parse symbols:\n");
   for (symbol_iterator I = Obj.symbol_begin(), E = Obj.symbol_end(); I != E;
        ++I) {
-    uint32_t Flags = I->getFlags();
+    auto FlagsOrErr = I->getFlags();
+    if (!FlagsOrErr)
+      return FlagsOrErr.takeError();
 
     // Skip undefined symbols.
-    if (Flags & SymbolRef::SF_Undefined)
+    if (*FlagsOrErr & SymbolRef::SF_Undefined)
       continue;
 
     // Get the symbol type.
     object::SymbolRef::Type SymType;
     if (auto SymTypeOrErr = I->getType())
       SymType = *SymTypeOrErr;
     else
       return SymTypeOrErr.takeError();
if (JITSymFlags->isWeak() || JITSymFlags->isCommon()) {
         uint64_t Size = I->getCommonSize();
         if (!CommonAlign)
           CommonAlign = Align;
         CommonSize = alignTo(CommonSize, Align) + Size;
         CommonSymbolsToAllocate.push_back(*I);
       }
     }
 
-    if (Flags & SymbolRef::SF_Absolute &&
+    if (*FlagsOrErr & SymbolRef::SF_Absolute &&
         SymType != object::SymbolRef::ST_File) {
       uint64_t Addr = 0;
       if (auto AddrOrErr = I->getAddress())
         Addr = *AddrOrErr;
       else
         return AddrOrErr.takeError();
 
       unsigned SectionID = AbsoluteSymbolSection;
 
       LLVM_DEBUG(dbgs() << "\tType: " << SymType << " (absolute) Name: " << Name
                         << " SID: " << SectionID
                         << " Offset: " << format("%p", (uintptr_t)Addr)
-                        << " flags: " << Flags << "\n");
+                        << " flags: " << *FlagsOrErr << "\n");
       GlobalSymbolTable[Name] = SymbolTableEntry(SectionID, Addr, *JITSymFlags);
     } else if (SymType == object::SymbolRef::ST_Function ||
                SymType == object::SymbolRef::ST_Data ||
                SymType == object::SymbolRef::ST_Unknown ||
                SymType == object::SymbolRef::ST_Other) {
 
       section_iterator SI = Obj.section_end();
       if (auto SIOrErr = I->getSection())
if (*FlagsOrErr & SymbolRef::SF_Absolute &&
               findOrEmitSection(Obj, *SI, IsCode, LocalSections))
         SectionID = *SectionIDOrErr;
       else
         return SectionIDOrErr.takeError();
 
       LLVM_DEBUG(dbgs() << "\tType: " << SymType << " Name: " << Name
                         << " SID: " << SectionID
                         << " Offset: " << format("%p", (uintptr_t)SectOffset)
-                        << " flags: " << Flags << "\n");
+                        << " flags: " << *FlagsOrErr << "\n");
       GlobalSymbolTable[Name] =
           SymbolTableEntry(SectionID, SectOffset, *JITSymFlags);
     }
   }
 
   // Allocate common symbols
   if (auto Err = emitCommonSymbols(Obj, CommonSymbolsToAllocate, CommonSize,
                                    CommonAlign))
if (unsigned GotSize = computeGOTSize(Obj)) {
     RWDataAlign = std::max<uint32_t>(RWDataAlign, getGOTEntrySize());
   }
 
   // Compute the size of all common symbols
   uint64_t CommonSize = 0;
   uint32_t CommonAlign = 1;
   for (symbol_iterator I = Obj.symbol_begin(), E = Obj.symbol_end(); I != E;
        ++I) {
-    uint32_t Flags = I->getFlags();
-    if (Flags & SymbolRef::SF_Common) {
+    Expected<uint32_t> FlagsOrErr = I->getFlags();
+    if (!FlagsOrErr)
+      return FlagsOrErr.takeError();
+    if (*FlagsOrErr & SymbolRef::SF_Common) {
       // Add the common symbols to a list.  We'll allocate them all below.
       uint64_t Size = I->getCommonSize();
       uint32_t Align = I->getAlignment();
       // If this is the first common symbol, use its alignment as the alignment
       // for the common symbols section.
       if (CommonSize == 0)
         CommonAlign = Align;
       CommonSize = alignTo(CommonSize, Align) + Size;

llvm/lib/Object/ArchiveWriter.cpp

static sys::TimePoint<std::chrono::seconds> now(bool Deterministic) {
   using namespace std::chrono;
 
   if (!Deterministic)
     return time_point_cast<seconds>(system_clock::now());
   return sys::TimePoint<seconds>();
 }
 
 static bool isArchiveSymbol(const object::BasicSymbolRef &S) {
-  uint32_t Symflags = S.getFlags();
-  if (Symflags & object::SymbolRef::SF_FormatSpecific)
+  Expected<uint32_t> SymFlagsOrErr = S.getFlags();
+  if (!SymFlagsOrErr)
+    report_fatal_error(SymFlagsOrErr.takeError());
+  if (*SymFlagsOrErr & object::SymbolRef::SF_FormatSpecific)
     return false;
-  if (!(Symflags & object::SymbolRef::SF_Global))
+  if (!(*SymFlagsOrErr & object::SymbolRef::SF_Global))
     return false;
-  if (Symflags & object::SymbolRef::SF_Undefined)
+  if (*SymFlagsOrErr & object::SymbolRef::SF_Undefined)
     return false;
   return true;
 }
 
 static void printNBits(raw_ostream &Out, object::Archive::Kind Kind,
                        uint64_t Val) {
   if (is64BitKind(Kind))
     print<uint64_t>(Out, Kind, Val);

llvm/lib/Object/COFFObjectFile.cpp

if (SectionNumber == COFF::IMAGE_SYM_DEBUG || Symb.isSectionDefinition())
     return SymbolRef::ST_Debug;
 
   if (!COFF::isReservedSectionNumber(SectionNumber))
     return SymbolRef::ST_Data;
 
   return SymbolRef::ST_Other;
 }
 
-uint32_t COFFObjectFile::getSymbolFlags(DataRefImpl Ref) const {
+Expected<uint32_t> COFFObjectFile::getSymbolFlags(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   uint32_t Result = SymbolRef::SF_None;
 
   if (Symb.isExternal() || Symb.isWeakExternal())
     Result |= SymbolRef::SF_Global;
 
   if (const coff_aux_weak_external *AWE = Symb.getWeakExternal()) {
     Result |= SymbolRef::SF_Weak;

llvm/lib/Object/IRObjectFile.cpp

void IRObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
   Symb.p += sizeof(ModuleSymbolTable::Symbol);
 }
 
 Error IRObjectFile::printSymbolName(raw_ostream &OS, DataRefImpl Symb) const {
   SymTab.printSymbolName(OS, getSym(Symb));
   return Error::success();
 }
 
-uint32_t IRObjectFile::getSymbolFlags(DataRefImpl Symb) const {
+Expected<uint32_t> IRObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   return SymTab.getSymbolFlags(getSym(Symb));
 }
 
 basic_symbol_iterator IRObjectFile::symbol_begin() const {
   DataRefImpl Ret;
   Ret.p = reinterpret_cast<uintptr_t>(SymTab.symbols().data());
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }

llvm/lib/Object/MachOObjectFile.cpp

uint64_t MachOObjectFile::getSymbolValueImpl(DataRefImpl Sym) const {
   return getNValue(Sym);
 }
 
 Expected<uint64_t> MachOObjectFile::getSymbolAddress(DataRefImpl Sym) const {
   return getSymbolValue(Sym);
 }
 
 uint32_t MachOObjectFile::getSymbolAlignment(DataRefImpl DRI) const {
-  uint32_t flags = getSymbolFlags(DRI);
-  if (flags & SymbolRef::SF_Common) {
+  Expected<uint32_t> FlagsOrErr = getSymbolFlags(DRI);
+  if (!FlagsOrErr)
+    // getSymbolFlags() on Mach-O symbol can not fail.
+    report_fatal_error(FlagsOrErr.takeError());
+  if (*FlagsOrErr & SymbolRef::SF_Common) {
     MachO::nlist_base Entry = getSymbolTableEntryBase(*this, DRI);
     return 1 << MachO::GET_COMM_ALIGN(Entry.n_desc);
   }
   return 0;
 }
 
 uint64_t MachOObjectFile::getCommonSymbolSizeImpl(DataRefImpl DRI) const {
   return getNValue(DRI);
case MachO::N_SECT :
       section_iterator Sec = *SecOrError;
       if (Sec->isData() || Sec->isBSS())
         return SymbolRef::ST_Data;
       return SymbolRef::ST_Function;
   }
   return SymbolRef::ST_Other;
 }
 
-uint32_t MachOObjectFile::getSymbolFlags(DataRefImpl DRI) const {
+Expected<uint32_t> MachOObjectFile::getSymbolFlags(DataRefImpl DRI) const {
   MachO::nlist_base Entry = getSymbolTableEntryBase(*this, DRI);
 
   uint8_t MachOType = Entry.n_type;
   uint16_t MachOFlags = Entry.n_desc;
 
   uint32_t Result = SymbolRef::SF_None;
 
   if ((MachOType & MachO::N_TYPE) == MachO::N_INDR)

llvm/lib/Object/ObjectFile.cpp

if (!SymSec) {
     // TODO: Actually report errors helpfully.
     consumeError(SymSec.takeError());
     return false;
   }
   return *this == **SymSec;
 }
 
 uint64_t ObjectFile::getSymbolValue(DataRefImpl Ref) const {
-  uint32_t Flags = getSymbolFlags(Ref);
-  if (Flags & SymbolRef::SF_Undefined)
+  if (auto FlagsOrErr = getSymbolFlags(Ref)) {
+    if (*FlagsOrErr & SymbolRef::SF_Undefined)
       return 0;
-  if (Flags & SymbolRef::SF_Common)
+    if (*FlagsOrErr & SymbolRef::SF_Common)
       return getCommonSymbolSize(Ref);
+  } else
+    // TODO: Actually report errors helpfully.
+    consumeError(FlagsOrErr.takeError());
   return getSymbolValueImpl(Ref);
 }
 
 Error ObjectFile::printSymbolName(raw_ostream &OS, DataRefImpl Symb) const {
   Expected<StringRef> Name = getSymbolName(Symb);
   if (!Name)
     return Name.takeError();
   OS << *Name;

llvm/lib/Object/TapiFile.cpp

 }
 
 Error TapiFile::printSymbolName(raw_ostream &OS, DataRefImpl DRI) const {
   const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
   OS << Sym->Prefix << Sym->Name;
   return Error::success();
 }
 
-uint32_t TapiFile::getSymbolFlags(DataRefImpl DRI) const {
+Expected<uint32_t> TapiFile::getSymbolFlags(DataRefImpl DRI) const {
   const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
   return Sym->Flags;
 }
 
 basic_symbol_iterator TapiFile::symbol_begin() const {
   DataRefImpl DRI;
   DRI.p = reinterpret_cast<uintptr_t>(&*Symbols.begin());
   return BasicSymbolRef{DRI, this};
 }
 
 basic_symbol_iterator TapiFile::symbol_end() const {
   DataRefImpl DRI;
   DRI.p = reinterpret_cast<uintptr_t>(&*Symbols.end());
   return BasicSymbolRef{DRI, this};
 }

llvm/lib/Object/WasmObjectFile.cpp

 }
 
 const wasm::WasmObjectHeader &WasmObjectFile::getHeader() const {
   return Header;
 }
 
 void WasmObjectFile::moveSymbolNext(DataRefImpl &Symb) const { Symb.d.b++; }
 
-uint32_t WasmObjectFile::getSymbolFlags(DataRefImpl Symb) const {
+Expected<uint32_t> WasmObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = SymbolRef::SF_None;
   const WasmSymbol &Sym = getWasmSymbol(Symb);
 
   LLVM_DEBUG(dbgs() << "getSymbolFlags: ptr=" << &Sym << " " << Sym << "\n");
   if (Sym.isBindingWeak())
     Result |= SymbolRef::SF_Weak;
   if (!Sym.isBindingLocal())
     Result |= SymbolRef::SF_Global;

llvm/lib/Object/XCOFFObjectFile.cpp

void XCOFFObjectFile::getRelocationTypeName(
     DataRefImpl Rel, SmallVectorImpl<char> &Result) const {
   if (is64Bit())
     report_fatal_error("64-bit support not implemented yet");
   const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
   StringRef Res = XCOFF::getRelocationTypeString(Reloc->Type);
   Result.append(Res.begin(), Res.end());
 }
 
-uint32_t XCOFFObjectFile::getSymbolFlags(DataRefImpl Symb) const {
+Expected<uint32_t> XCOFFObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 basic_symbol_iterator XCOFFObjectFile::symbol_begin() const {
   if (is64Bit())
     report_fatal_error("64-bit support not implemented yet");

llvm/test/tools/llvm-nm/invalid-symbol-table-size.test

+## This test ensures llvm-nm will emit helpful error message when dumping a symbol table
+## whose sh_size isn't a multiple of the symbol size (sh_size % sizeof(Elf_Sym) != 0).
+
+# RUN: yaml2obj -DBITS=32 -DSIZE=33 %s -o %t.32-bit.o
+# RUN: not llvm-nm %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 %s
+# RUN: yaml2obj -DBITS=64 -DSIZE=49 %s -o %t.64-bit.o
+# RUN: not llvm-nm %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 %s
+
+# RUN: not llvm-nm --dynamic %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 %s
+# RUN: not llvm-nm --dynamic %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 %s
+
+#       CHECK: error: {{.*}} section [index 2] has an invalid sh_size ([[SIZE]]) which is not a multiple of its sh_entsize ([[SYMSIZE]])
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_386
+Sections:
+  - Name:    .dynstr
+    Type:    SHT_STRTAB
+    Content: "00"
+  - Name:    .symtab
+    Type:    SHT_SYMTAB
+    Size:    [[SIZE]]
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+    Size:    [[SIZE]]

llvm/test/tools/llvm-objdump/ELF/invalid-symbol-table-size.test

+## This test ensures llvm-objdump will emit helpful error message when dumping a symbol table
+## whose sh_size isn't a multiple of the symbol size (sh_size % sizeof(Elf_Sym) != 0).
+
+# RUN: yaml2obj -DBITS=32 -DSIZE=33 %s -o %t.32-bit.o
+# RUN: not llvm-objdump --syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 %s
+# RUN: yaml2obj -DBITS=64 -DSIZE=49 %s -o %t.64-bit.o
+# RUN: not llvm-objdump --syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 %s
+
+# RUN: not llvm-objdump --dynamic-syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 %s
+# RUN: not llvm-objdump --dynamic-syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 %s
+
+#       CHECK: error: {{.*}} section [index 2] has an invalid sh_size ([[SIZE]]) which is not a multiple of its sh_entsize ([[SYMSIZE]])
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_386
+Sections:
+  - Name:    .dynstr
+    Type:    SHT_STRTAB
+    Content: "00"
+  - Name:    .symtab
+    Type:    SHT_SYMTAB
+    Size:    [[SIZE]]
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+    Size:    [[SIZE]]

llvm/tools/dsymutil/DebugMap.cpp

if (!ObjectEntry) {
     if (!Object) {
       auto Err = Object.takeError();
       WithColor::warning() << "Unable to open " << Path << " "
                            << toString(std::move(Err)) << '\n';
     } else {
       for (const auto &Sym : Object->symbols()) {
         uint64_t Address = Sym.getValue();
         Expected<StringRef> Name = Sym.getName();
-        if (!Name || (Sym.getFlags() &
-                      (SymbolRef::SF_Absolute | SymbolRef::SF_Common))) {
+        Expected<uint32_t> FlagsOrErr = Sym.getFlags();
+        if (!Name || !FlagsOrErr ||
+            (*FlagsOrErr & (SymbolRef::SF_Absolute | SymbolRef::SF_Common))) {
           // TODO: Actually report errors helpfully.
+          if (!FlagsOrErr)
+            consumeError(FlagsOrErr.takeError());
           if (!Name)
             consumeError(Name.takeError());
           continue;
         }
         SymbolAddresses[*Name] = Address;
       }
     }
   }

llvm/tools/dsymutil/MachODebugMapParser.cpp

for (auto Sym : Obj.symbols()) {
     // The value of some categories of symbols isn't meaningful. For
     // example common symbols store their size in the value field, not
     // their address. Absolute symbols have a fixed address that can
     // conflict with standard symbols. These symbols (especially the
     // common ones), might still be referenced by relocations. These
     // relocations will use the symbol itself, and won't need an
     // object file address. The object file address field is optional
     // in the DebugMap, leave it unassigned for these symbols.
-    uint32_t Flags = Sym.getFlags();
-    if (Flags & SymbolRef::SF_Absolute) {
+    Expected<uint32_t> FlagsOrErr = Sym.getFlags();
+    if (!FlagsOrErr)
+      // getFlags() on Mach-O symbol can not fail.
+      report_fatal_error(FlagsOrErr.takeError());
+    if (*FlagsOrErr & SymbolRef::SF_Absolute) {
       CurrentObjectAddresses[*Name] = None;
-    } else if (Flags & SymbolRef::SF_Common) {
+    } else if (*FlagsOrErr & SymbolRef::SF_Common) {
       CurrentObjectAddresses[*Name] = None;
       CommonSymbols.push_back(std::string(*Name));
     } else {
       CurrentObjectAddresses[*Name] = Addr;
     }
   }
 }
 

llvm/tools/llvm-nm/llvm-nm.cpp

struct NMSymbol {
   uint8_t NSect;
   uint16_t NDesc;
   StringRef IndirectName;
 };
 } // anonymous namespace
 
 static bool compareSymbolAddress(const NMSymbol &A, const NMSymbol &B) {
   bool ADefined;
+  Expected<uint32_t> AFlagsOrErr = A.Sym.getFlags();
+  if (!AFlagsOrErr)
+    // Symbol flags have been checked in caller.
+    report_fatal_error(AFlagsOrErr.takeError());
   if (A.Sym.getRawDataRefImpl().p)
-    ADefined = !(A.Sym.getFlags() & SymbolRef::SF_Undefined);
+    ADefined = !(*AFlagsOrErr & SymbolRef::SF_Undefined);
   else
     ADefined = A.TypeChar != 'U';
   bool BDefined;
+  Expected<uint32_t> BFlagsOrErr = B.Sym.getFlags();
+  if (!BFlagsOrErr)
+    // Symbol flags have been checked in caller.
+    report_fatal_error(BFlagsOrErr.takeError());
   if (B.Sym.getRawDataRefImpl().p)
-    BDefined = !(B.Sym.getFlags() & SymbolRef::SF_Undefined);
+    BDefined = !(*BFlagsOrErr & SymbolRef::SF_Undefined);
   else
     BDefined = B.TypeChar != 'U';
   return std::make_tuple(ADefined, A.Address, A.Name, A.Size) <
          std::make_tuple(BDefined, B.Address, B.Name, B.Size);
 }
 
 static bool compareSymbolSize(const NMSymbol &A, const NMSymbol &B) {
   return std::make_tuple(A.Size, A.Name, A.Address) <
static void darwinPrintSymbol(SymbolicFile &Obj, const NMSymbol &S,
   uint32_t Flags = 0;
   uint8_t NType = 0;
   uint8_t NSect = 0;
   uint16_t NDesc = 0;
   uint32_t NStrx = 0;
   uint64_t NValue = 0;
   MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(&Obj);
   if (Obj.isIR()) {
-    uint32_t SymFlags = S.Sym.getFlags();
-    if (SymFlags & SymbolRef::SF_Global)
+    Expected<uint32_t> SymFlags = S.Sym.getFlags();
+    if (!SymFlags)
+      // getFlags() on Mach-O symbol can not fail.
+      report_fatal_error(SymFlags.takeError());
+    if (*SymFlags & SymbolRef::SF_Global)
       NType |= MachO::N_EXT;
-    if (SymFlags & SymbolRef::SF_Hidden)
+    if (*SymFlags & SymbolRef::SF_Hidden)
       NType |= MachO::N_PEXT;
-    if (SymFlags & SymbolRef::SF_Undefined)
+    if (*SymFlags & SymbolRef::SF_Undefined)
       NType |= MachO::N_EXT | MachO::N_UNDF;
     else {
       // Here we have a symbol definition.  So to fake out a section name we
       // use 1, 2 and 3 for section numbers.  See below where they are used to
       // print out fake section names.
       NType |= MachO::N_SECT;
-      if (SymFlags & SymbolRef::SF_Const)
+      if (*SymFlags & SymbolRef::SF_Const)
         NSect = 3;
-      else if (SymFlags & SymbolRef::SF_Executable)
+      else if (*SymFlags & SymbolRef::SF_Executable)
         NSect = 1;
       else
         NSect = 2;
     }
-    if (SymFlags & SymbolRef::SF_Weak)
+    if (*SymFlags & SymbolRef::SF_Weak)
       NDesc |= MachO::N_WEAK_DEF;
   } else {
     DataRefImpl SymDRI = S.Sym.getRawDataRefImpl();
     if (MachO->is64Bit()) {
       H_64 = MachO->MachOObjectFile::getHeader64();
       Filetype = H_64.filetype;
       Flags = H_64.flags;
       if (SymDRI.p){
static void sortAndPrintSymbolList(SymbolicFile &Obj, bool printName,
   for (const NMSymbol &S : SymbolList) {
     uint32_t SymFlags;
     std::string Name = S.Name.str();
     MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(&Obj);
     if (Demangle) {
       if (Optional<std::string> Opt = demangle(S.Name, MachO))
         Name = *Opt;
     }
-    if (S.Sym.getRawDataRefImpl().p)
-      SymFlags = S.Sym.getFlags();
-    else
+    if (S.Sym.getRawDataRefImpl().p) {
+      Expected<uint32_t> SymFlagsOrErr = S.Sym.getFlags();
+      if (!SymFlagsOrErr) {
+        error(SymFlagsOrErr.takeError(), Obj.getFileName());
+        return;
+      }
+      SymFlags = *SymFlagsOrErr;
+    } else
       SymFlags = S.SymFlags;
 
     bool Undefined = SymFlags & SymbolRef::SF_Undefined;
     bool Global = SymFlags & SymbolRef::SF_Global;
     bool Weak = SymFlags & SymbolRef::SF_Weak;
     if ((!Undefined && UndefinedOnly) || (Undefined && DefinedOnly) ||
         (!Global && ExternalOnly) || (Weak && NoWeakSymbols))
       continue;
case MachO::N_SECT: {
     return 's';
   }
   }
 
   return '?';
 }
 
 static char getSymbolNMTypeChar(WasmObjectFile &Obj, basic_symbol_iterator I) {
-  uint32_t Flags = I->getFlags();
-  if (Flags & SymbolRef::SF_Executable)
+  Expected<uint32_t> FlagsOrErr = I->getFlags();
+  if (!FlagsOrErr)
+    // getFlags() on WASM symbol can not fail.
+    report_fatal_error(FlagsOrErr.takeError());
+  if (*FlagsOrErr & SymbolRef::SF_Executable)
     return 't';
   return 'd';
 }
 
 static char getSymbolNMTypeChar(IRObjectFile &Obj, basic_symbol_iterator I) {
-  uint32_t Flags = I->getFlags();
+  Expected<uint32_t> FlagsOrErr = I->getFlags();
+  if (!FlagsOrErr)
+    // getFlags() on IR symbol can not fail.
+    report_fatal_error(FlagsOrErr.takeError());
   // FIXME: should we print 'b'? At the IR level we cannot be sure if this
   // will be in bss or not, but we could approximate.
-  if (Flags & SymbolRef::SF_Executable)
+  if (*FlagsOrErr & SymbolRef::SF_Executable)
     return 't';
   else if (Triple(Obj.getTargetTriple()).isOSDarwin() &&
-           (Flags & SymbolRef::SF_Const))
+           (*FlagsOrErr & SymbolRef::SF_Const))
     return 's';
   else
     return 'd';
 }
 
 static bool isObject(SymbolicFile &Obj, basic_symbol_iterator I) {
   return !dyn_cast<ELFObjectFileBase>(&Obj)
              ? false
static StringRef getNMTypeName(SymbolicFile &Obj, basic_symbol_iterator I) {
   }
   return "";
 }
 
 // Return Posix nm class type tag (single letter), but also set SecName and
 // section and name, to be used in format=sysv output.
 static char getNMSectionTagAndName(SymbolicFile &Obj, basic_symbol_iterator I,
                                    StringRef &SecName) {
-  uint32_t Symflags = I->getFlags();
+  Expected<uint32_t> SymFlagsOrErr = I->getFlags();
+  if (!SymFlagsOrErr) {
+    consumeError(SymFlagsOrErr.takeError());
+    return '?';
+  }
   if (ELFObjectFileBase *ELFObj = dyn_cast<ELFObjectFileBase>(&Obj)) {
-    if (Symflags & object::SymbolRef::SF_Absolute)
+    if (*SymFlagsOrErr & object::SymbolRef::SF_Absolute)
       SecName = "*ABS*";
-    else if (Symflags & object::SymbolRef::SF_Common)
+    else if (*SymFlagsOrErr & object::SymbolRef::SF_Common)
       SecName = "*COM*";
-    else if (Symflags & object::SymbolRef::SF_Undefined)
+    else if (*SymFlagsOrErr & object::SymbolRef::SF_Undefined)
       SecName = "*UND*";
     else {
       elf_symbol_iterator SymI(I);
       Expected<elf_section_iterator> SecIOrErr = SymI->getSection();
       if (!SecIOrErr) {
         consumeError(SecIOrErr.takeError());
         return '?';
       }
 
       if (*SecIOrErr == ELFObj->section_end())
         return '?';
 
       Expected<StringRef> NameOrErr = (*SecIOrErr)->getName();
       if (!NameOrErr) {
         consumeError(NameOrErr.takeError());
         return '?';
       }
       SecName = *NameOrErr;
     }
   }
 
-  if ((Symflags & object::SymbolRef::SF_Weak) && !isa<MachOObjectFile>(Obj)) {
+  if ((*SymFlagsOrErr & object::SymbolRef::SF_Weak) &&
+      !isa<MachOObjectFile>(Obj)) {
     char Ret = isObject(Obj, I) ? 'v' : 'w';
-    return (!(Symflags & object::SymbolRef::SF_Undefined)) ? toupper(Ret) : Ret;
+    return (!(*SymFlagsOrErr & object::SymbolRef::SF_Undefined)) ? toupper(Ret)
+                                                                 : Ret;
   }
 
-  if (Symflags & object::SymbolRef::SF_Undefined)
+  if (*SymFlagsOrErr & object::SymbolRef::SF_Undefined)
     return 'U';
 
-  if (Symflags & object::SymbolRef::SF_Common)
+  if (*SymFlagsOrErr & object::SymbolRef::SF_Common)
     return 'C';
 
   char Ret = '?';
-  if (Symflags & object::SymbolRef::SF_Absolute)
+  if (*SymFlagsOrErr & object::SymbolRef::SF_Absolute)
     Ret = 'a';
   else if (IRObjectFile *IR = dyn_cast<IRObjectFile>(&Obj))
     Ret = getSymbolNMTypeChar(*IR, I);
   else if (COFFObjectFile *COFF = dyn_cast<COFFObjectFile>(&Obj))
     Ret = getSymbolNMTypeChar(*COFF, I);
   else if (COFFImportFile *COFFImport = dyn_cast<COFFImportFile>(&Obj))
     Ret = getSymbolNMTypeChar(*COFFImport);
   else if (MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(&Obj))
     Ret = getSymbolNMTypeChar(*MachO, I);
   else if (WasmObjectFile *Wasm = dyn_cast<WasmObjectFile>(&Obj))
     Ret = getSymbolNMTypeChar(*Wasm, I);
   else if (ELFObjectFileBase *ELF = dyn_cast<ELFObjectFileBase>(&Obj)) {
     if (ELFSymbolRef(*I).getELFType() == ELF::STT_GNU_IFUNC)
       return 'i';
     Ret = getSymbolNMTypeChar(*ELF, I);
     if (ELFSymbolRef(*I).getBinding() == ELF::STB_GNU_UNIQUE)
       return Ret;
   } else
     llvm_unreachable("unknown binary format");
 
-  if (!(Symflags & object::SymbolRef::SF_Global))
+  if (!(*SymFlagsOrErr & object::SymbolRef::SF_Global))
     return Ret;
 
   return toupper(Ret);
 }
 
 // getNsectForSegSect() is used to implement the Mach-O "-s segname sectname"
 // option to dump only those symbols from that section in a Mach-O file.
 // It is called once for each Mach-O file from dumpSymbolNamesFromObject()
static void dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
   if (!SegSect.empty() && MachO) {
     Nsect = getNsectForSegSect(MachO);
     // If this section is not in the object file no symbols are printed.
     if (Nsect == 0)
       return;
   }
   if (!(MachO && DyldInfoOnly)) {
     for (BasicSymbolRef Sym : Symbols) {
-      uint32_t SymFlags = Sym.getFlags();
-      if (!DebugSyms && (SymFlags & SymbolRef::SF_FormatSpecific))
+      Expected<uint32_t> SymFlagsOrErr = Sym.getFlags();
+      if (!SymFlagsOrErr) {
+        error(SymFlagsOrErr.takeError(), Obj.getFileName());
+        return;
+      }
+      if (!DebugSyms && (*SymFlagsOrErr & SymbolRef::SF_FormatSpecific))
         continue;
-      if (WithoutAliases && (SymFlags & SymbolRef::SF_Indirect))
+      if (WithoutAliases && (*SymFlagsOrErr & SymbolRef::SF_Indirect))
         continue;
       // If a "-s segname sectname" option was specified and this is a Mach-O
       // file and this section appears in this file, Nsect will be non-zero then
       // see if this symbol is a symbol from that section and if not skip it.
       if (Nsect && Nsect != getNsectInMachO(*MachO, Sym))
         continue;
       NMSymbol S = {};
       S.Size = 0;

llvm/tools/llvm-objdump/MachODump.cpp

for (unsigned SymIdx = 0; SymIdx != Symbols.size(); SymIdx++) {
       }
 
       uint64_t End = containsNextSym ? std::min(NextSym, SectSize) : SectSize;
       uint64_t Size;
 
       symbolTableWorked = true;
 
       DataRefImpl Symb = Symbols[SymIdx].getRawDataRefImpl();
-      bool IsThumb = MachOOF->getSymbolFlags(Symb) & SymbolRef::SF_Thumb;
+      Expected<uint32_t> SymbolFlagsOrErr = MachOOF->getSymbolFlags(Symb);
+      if (!SymbolFlagsOrErr)
+        // getSymbolFlags() on Mach-O symbol can not fail.
+        report_fatal_error(SymbolFlagsOrErr.takeError());
+      bool IsThumb = *SymbolFlagsOrErr & SymbolRef::SF_Thumb;
 
       // We only need the dedicated Thumb target if there's a real choice
       // (i.e. we're not targeting M-class) and the function is Thumb.
       bool UseThumbTarget = IsThumb && ThumbTarget;
 
       // If we are not specifying a symbol to start disassembly with and this
       // is the first symbol in the section but not at the start of the section
       // then move the disassembly index to the start of the section and

llvm/tools/llvm-objdump/llvm-objdump.cpp

void printSymbol(const ObjectFile *O, const SymbolRef &Symbol,
                  StringRef ArchitectureName, bool DumpDynamic) {
   const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(O);
   uint64_t Address = unwrapOrError(Symbol.getAddress(), FileName, ArchiveName,
                                    ArchitectureName);
   if ((Address < StartAddress) || (Address > StopAddress))
     return;
   SymbolRef::Type Type =
       unwrapOrError(Symbol.getType(), FileName, ArchiveName, ArchitectureName);
-  uint32_t Flags = Symbol.getFlags();
+  uint32_t Flags =
+      unwrapOrError(Symbol.getFlags(), FileName, ArchiveName, ArchitectureName);
 
   // Don't ask a Mach-O STAB symbol for its section unless you know that
   // STAB symbol's section field refers to a valid section index. Otherwise
   // the symbol may error trying to load a section that does not exist.
   bool IsSTAB = false;
   if (MachO) {
     DataRefImpl SymDRI = Symbol.getRawDataRefImpl();
     uint8_t NType =

llvm/tools/llvm-size/llvm-size.cpp

case ELF::SHT_RELA:
     return false;
   }
   return true;
 }
 
 /// Total size of all ELF common symbols
 static uint64_t getCommonSize(ObjectFile *Obj) {
   uint64_t TotalCommons = 0;
-  for (auto &Sym : Obj->symbols())
-    if (Obj->getSymbolFlags(Sym.getRawDataRefImpl()) & SymbolRef::SF_Common)
+  for (auto &Sym : Obj->symbols()) {
+    auto SymFlagsOrErr = Obj->getSymbolFlags(Sym.getRawDataRefImpl());
+    if (!SymFlagsOrErr)
+      // FIXME: Actually report errors helpfully.
+      report_fatal_error(SymFlagsOrErr.takeError());
+    if (*SymFlagsOrErr & SymbolRef::SF_Common)
       TotalCommons += Obj->getCommonSymbolSize(Sym.getRawDataRefImpl());
+  }
   return TotalCommons;
 }
 
 /// Print the size of each Mach-O segment and section in @p MachO.
 ///
 /// This is when used when @c OutputFormat is darwin and produces the same
 /// output as darwin's size(1) -m output.
 static void printDarwinSectionSizes(MachOObjectFile *MachO) {
   std::string fmtbuf;
   raw_string_ostream fmt(fmtbuf);
   const char *radix_fmt = getRadixFmt();
   if (Radix == hexadecimal)
     fmt << "0x";

llvm/tools/sancov/sancov.cpp

for (const object::SymbolRef &Symbol : O.symbols()) {
     Expected<uint64_t> AddressOrErr = Symbol.getAddress();
     failIfError(AddressOrErr);
     uint64_t Address = AddressOrErr.get();
 
     Expected<StringRef> NameOrErr = Symbol.getName();
     failIfError(NameOrErr);
     StringRef Name = NameOrErr.get();
 
-    if (!(Symbol.getFlags() & object::BasicSymbolRef::SF_Undefined) &&
+    Expected<uint32_t> FlagsOrErr = Symbol.getFlags();
+    failIfError(FlagsOrErr);
+    uint32_t Flags = FlagsOrErr.get();
+
+    if (!(Flags & object::BasicSymbolRef::SF_Undefined) &&
         isCoveragePointSymbol(Name)) {
       Result.insert(Address);
     }
   }
 
   if (const auto *CO = dyn_cast<object::COFFObjectFile>(&O)) {
     for (const object::ExportDirectoryEntryRef &Export :
          CO->export_directories()) {