diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h --- a/llvm/include/llvm/Object/ELFObjectFile.h +++ b/llvm/include/llvm/Object/ELFObjectFile.h @@ -408,11 +408,8 @@ const Elf_Rel *getRel(DataRefImpl Rel) const; const Elf_Rela *getRela(DataRefImpl Rela) const; - const Elf_Sym *getSymbol(DataRefImpl Sym) const { - auto Ret = EF.template getEntry(Sym.d.a, Sym.d.b); - if (!Ret) - report_fatal_error(errorToErrorCode(Ret.takeError()).message()); - return *Ret; + Expected getSymbol(DataRefImpl Sym) const { + return EF.template getEntry(Sym.d.a, Sym.d.b); } /// Get the relocation section that contains \a Rel. @@ -499,7 +496,9 @@ template Expected ELFObjectFile::getSymbolName(DataRefImpl Sym) const { - const Elf_Sym *ESym = getSymbol(Sym); + Expected SymOrErr = getSymbol(Sym); + if (!SymOrErr) + return SymOrErr.takeError(); auto SymTabOrErr = EF.getSection(Sym.d.a); if (!SymTabOrErr) return SymTabOrErr.takeError(); @@ -511,12 +510,12 @@ auto SymStrTabOrErr = EF.getStringTable(*StringTableSec); if (!SymStrTabOrErr) return SymStrTabOrErr.takeError(); - Expected Name = ESym->getName(*SymStrTabOrErr); + Expected Name = (*SymOrErr)->getName(*SymStrTabOrErr); if (Name && !Name->empty()) return Name; // If the symbol name is empty use the section name. - if (ESym->getType() == ELF::STT_SECTION) { + if ((*SymOrErr)->getType() == ELF::STT_SECTION) { if (Expected SecOrErr = getSymbolSection(Sym)) { consumeError(Name.takeError()); return (*SecOrErr)->getName(); @@ -542,15 +541,18 @@ template uint64_t ELFObjectFile::getSymbolValueImpl(DataRefImpl Symb) const { - const Elf_Sym *ESym = getSymbol(Symb); - uint64_t Ret = ESym->st_value; - if (ESym->st_shndx == ELF::SHN_ABS) + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + + uint64_t Ret = (*SymOrErr)->st_value; + if ((*SymOrErr)->st_shndx == ELF::SHN_ABS) return Ret; const Elf_Ehdr &Header = EF.getHeader(); // Clear the ARM/Thumb or microMIPS indicator flag. if ((Header.e_machine == ELF::EM_ARM || Header.e_machine == ELF::EM_MIPS) && - ESym->getType() == ELF::STT_FUNC) + (*SymOrErr)->getType() == ELF::STT_FUNC) Ret &= ~1; return Ret; @@ -565,8 +567,11 @@ return SymbolValueOrErr.takeError(); uint64_t Result = *SymbolValueOrErr; - const Elf_Sym *ESym = getSymbol(Symb); - switch (ESym->st_shndx) { + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + return SymOrErr.takeError(); + + switch ((*SymOrErr)->st_shndx) { case ELF::SHN_COMMON: case ELF::SHN_UNDEF: case ELF::SHN_ABS: @@ -589,7 +594,7 @@ } Expected SectionOrErr = - EF.getSection(*ESym, *SymTabOrErr, ShndxTable); + EF.getSection(**SymOrErr, *SymTabOrErr, ShndxTable); if (!SectionOrErr) return SectionOrErr.takeError(); const Elf_Shdr *Section = *SectionOrErr; @@ -602,9 +607,11 @@ template uint32_t ELFObjectFile::getSymbolAlignment(DataRefImpl Symb) const { - const Elf_Sym *Sym = getSymbol(Symb); - if (Sym->st_shndx == ELF::SHN_COMMON) - return Sym->st_value; + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + if ((*SymOrErr)->st_shndx == ELF::SHN_COMMON) + return (*SymOrErr)->st_value; return 0; } @@ -619,35 +626,49 @@ template uint64_t ELFObjectFile::getSymbolSize(DataRefImpl Sym) const { - return getSymbol(Sym)->st_size; + Expected SymOrErr = getSymbol(Sym); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + return (*SymOrErr)->st_size; } template uint64_t ELFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Symb) const { - return getSymbol(Symb)->st_size; + return getSymbolSize(Symb); } template uint8_t ELFObjectFile::getSymbolBinding(DataRefImpl Symb) const { - return getSymbol(Symb)->getBinding(); + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + return (*SymOrErr)->getBinding(); } template uint8_t ELFObjectFile::getSymbolOther(DataRefImpl Symb) const { - return getSymbol(Symb)->st_other; + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + return (*SymOrErr)->st_other; } template uint8_t ELFObjectFile::getSymbolELFType(DataRefImpl Symb) const { - return getSymbol(Symb)->getType(); + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + report_fatal_error(SymOrErr.takeError()); + return (*SymOrErr)->getType(); } template Expected ELFObjectFile::getSymbolType(DataRefImpl Symb) const { - const Elf_Sym *ESym = getSymbol(Symb); + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + return SymOrErr.takeError(); - switch (ESym->getType()) { + switch ((*SymOrErr)->getType()) { case ELF::STT_NOTYPE: return SymbolRef::ST_Unknown; case ELF::STT_SECTION: @@ -667,8 +688,11 @@ template Expected ELFObjectFile::getSymbolFlags(DataRefImpl Sym) const { - const Elf_Sym *ESym = getSymbol(Sym); + Expected SymOrErr = getSymbol(Sym); + if (!SymOrErr) + return SymOrErr.takeError(); + const Elf_Sym *ESym = *SymOrErr; uint32_t Result = SymbolRef::SF_None; if (ESym->getBinding() != ELF::STB_LOCAL) @@ -760,12 +784,14 @@ template Expected ELFObjectFile::getSymbolSection(DataRefImpl Symb) const { - const Elf_Sym *Sym = getSymbol(Symb); + Expected SymOrErr = getSymbol(Symb); + if (!SymOrErr) + return SymOrErr.takeError(); + auto SymTabOrErr = EF.getSection(Symb.d.a); if (!SymTabOrErr) return SymTabOrErr.takeError(); - const Elf_Shdr *SymTab = *SymTabOrErr; - return getSymbolSection(Sym, SymTab); + return getSymbolSection(*SymOrErr, *SymTabOrErr); } template diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp --- a/llvm/tools/llvm-objdump/ELFDump.cpp +++ b/llvm/tools/llvm-objdump/ELFDump.cpp @@ -85,8 +85,13 @@ if (!Undef) { symbol_iterator SI = RelRef.getSymbol(); - const typename ELFT::Sym *Sym = Obj->getSymbol(SI->getRawDataRefImpl()); - if (Sym->getType() == ELF::STT_SECTION) { + Expected SymOrErr = + Obj->getSymbol(SI->getRawDataRefImpl()); + // TODO: test this error. + if (!SymOrErr) + return SymOrErr.takeError(); + + if ((*SymOrErr)->getType() == ELF::STT_SECTION) { Expected SymSI = SI->getSection(); if (!SymSI) return SymSI.takeError(); diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1340,13 +1340,21 @@ static uint8_t getElfSymbolType(const ObjectFile *Obj, const SymbolRef &Sym) { assert(Obj->isELF()); if (auto *Elf32LEObj = dyn_cast(Obj)) - return Elf32LEObj->getSymbol(Sym.getRawDataRefImpl())->getType(); + return unwrapOrError(Elf32LEObj->getSymbol(Sym.getRawDataRefImpl()), + Obj->getFileName()) + ->getType(); if (auto *Elf64LEObj = dyn_cast(Obj)) - return Elf64LEObj->getSymbol(Sym.getRawDataRefImpl())->getType(); + return unwrapOrError(Elf64LEObj->getSymbol(Sym.getRawDataRefImpl()), + Obj->getFileName()) + ->getType(); if (auto *Elf32BEObj = dyn_cast(Obj)) - return Elf32BEObj->getSymbol(Sym.getRawDataRefImpl())->getType(); + return unwrapOrError(Elf32BEObj->getSymbol(Sym.getRawDataRefImpl()), + Obj->getFileName()) + ->getType(); if (auto *Elf64BEObj = cast(Obj)) - return Elf64BEObj->getSymbol(Sym.getRawDataRefImpl())->getType(); + return unwrapOrError(Elf64BEObj->getSymbol(Sym.getRawDataRefImpl()), + Obj->getFileName()) + ->getType(); llvm_unreachable("Unsupported binary format"); } @@ -1362,7 +1370,9 @@ // ELFSymbolRef::getAddress() returns size instead of value for common // symbols which is not desirable for disassembly output. Overriding. if (SymbolType == ELF::STT_COMMON) - Address = Obj->getSymbol(Symbol.getRawDataRefImpl())->st_value; + Address = unwrapOrError(Obj->getSymbol(Symbol.getRawDataRefImpl()), + Obj->getFileName()) + ->st_value; StringRef Name = unwrapOrError(Symbol.getName(), Obj->getFileName()); if (Name.empty()) diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp --- a/llvm/unittests/Object/ELFObjectFileTest.cpp +++ b/llvm/unittests/Object/ELFObjectFileTest.cpp @@ -397,3 +397,75 @@ EXPECT_EQ((const char *)Data - Buf.getBufferStart(), 0x4000); EXPECT_EQ(Data[0], 0x99); } + +// This is a test for API that is related to symbols. +// We check that errors are properly reported here. +TEST(ELFObjectFileTest, InvalidSymbolTest) { + SmallString<0> Storage; + Expected> ElfOrErr = toBinary(Storage, R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_DYN + Machine: EM_X86_64 +Sections: + - Name: .symtab + Type: SHT_SYMTAB +)"); + + ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded()); + const ELFFile &Elf = ElfOrErr->getELFFile(); + const ELFObjectFile &Obj = *ElfOrErr; + + Expected SymtabSecOrErr = Elf.getSection(1); + ASSERT_THAT_EXPECTED(SymtabSecOrErr, Succeeded()); + ASSERT_EQ((*SymtabSecOrErr)->sh_type, ELF::SHT_SYMTAB); + + // We create a symbol with an index that is too large to exist in the object. + constexpr unsigned BrokenSymIndex = 0xFFFFFFFF; + ELFSymbolRef BrokenSym = Obj.toSymbolRef(*SymtabSecOrErr, BrokenSymIndex); + + const char *ErrMsg = "unable to access section [index 1] data at " + "0x1800000028: offset goes past the end of file"; + // 1) Check the behavior of ELFObjectFile::getSymbolName(). + // SymbolRef::getName() calls it internally. We can't test it directly, + // because it is protected. + EXPECT_THAT_ERROR(BrokenSym.getName().takeError(), FailedWithMessage(ErrMsg)); + + // 2) Check the behavior of ELFObjectFile::getSymbol(). + EXPECT_THAT_ERROR(Obj.getSymbol(BrokenSym.getRawDataRefImpl()).takeError(), + FailedWithMessage(ErrMsg)); + + // 3) Check the behavior of ELFObjectFile::getSymbolSection(). + // SymbolRef::getSection() calls it internally. We can't test it directly, + // because it is protected. + EXPECT_THAT_ERROR(BrokenSym.getSection().takeError(), + FailedWithMessage(ErrMsg)); + + // 4) Check the behavior of ELFObjectFile::getSymbolFlags(). + // SymbolRef::getFlags() calls it internally. We can't test it directly, + // because it is protected. + EXPECT_THAT_ERROR(BrokenSym.getFlags().takeError(), + FailedWithMessage(ErrMsg)); + + // 5) Check the behavior of ELFObjectFile::getSymbolType(). + // SymbolRef::getType() calls it internally. We can't test it directly, + // because it is protected. + EXPECT_THAT_ERROR(BrokenSym.getType().takeError(), FailedWithMessage(ErrMsg)); + + // 6) Check the behavior of ELFObjectFile::getSymbolAddress(). + // SymbolRef::getAddress() calls it internally. We can't test it directly, + // because it is protected. + EXPECT_THAT_ERROR(BrokenSym.getAddress().takeError(), + FailedWithMessage(ErrMsg)); + + // Finally, check the `ELFFile::getEntry` API. This is an underlying + // method that generates errors for all cases above. + EXPECT_THAT_EXPECTED(Elf.getEntry(**SymtabSecOrErr, 0), + Succeeded()); + EXPECT_THAT_ERROR( + Elf.getEntry(**SymtabSecOrErr, BrokenSymIndex) + .takeError(), + FailedWithMessage(ErrMsg)); +}