Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/include/llvm/BinaryFormat/ELF.h
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,8 @@ struct Elf64_Shdr {
Elf64_Xword sh_entsize;
};

enum { PN_XNUM = 0xffff };

// Special section indices.
enum {
SHN_UNDEF = 0, // Undefined, missing, irrelevant, or meaningless
Expand Down
106 changes: 87 additions & 19 deletions llvm/include/llvm/Object/ELF.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,43 @@ class ELFFile {
std::vector<Elf_Shdr> FakeSections;
SmallString<0> FakeSectionStrings;

// When the number of program headers is >= 0xffff, the actual number is
// contained in the sh_info field of the section header at index 0.
std::optional<uint32_t> RealPhNum;
// When the number of section headers is >= 0xff00, the actual number is
// contained in the sh_size field of the section header at index 0.
std::optional<uint64_t> RealShNum;
// When the index of str section is >= 0xff00, the actual number is
// contained in the sh_link field of the section header at index 0.
std::optional<uint32_t> RealShStrNdx;

ELFFile(StringRef Object);

Error readShdrZero();

public:
Expected<uint32_t> getPhNum() const {
if (!RealPhNum) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
return std::move(E);
}
return *RealPhNum;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: new lines between functions, please.

Expected<uint64_t> getShNum() const {
if (!RealShNum) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
return std::move(E);
}
return *RealShNum;
}
Expected<uint32_t> getShStrNdx() const {
if (!RealShStrNdx) {
if (Error E = const_cast<ELFFile<ELFT> *>(this)->readShdrZero())
return std::move(E);
}
return *RealShStrNdx;
}

const Elf_Ehdr &getHeader() const {
return *reinterpret_cast<const Elf_Ehdr *>(base());
}
Expand Down Expand Up @@ -379,22 +413,26 @@ class ELFFile {

/// Iterate over program header table.
Expected<Elf_Phdr_Range> program_headers() const {
if (getHeader().e_phnum && getHeader().e_phentsize != sizeof(Elf_Phdr))
uint32_t NumPh;
if (Expected<uint32_t> PhNumOrErr = getPhNum())
NumPh = *PhNumOrErr;
else
return PhNumOrErr.takeError();
if (NumPh && getHeader().e_phentsize != sizeof(Elf_Phdr))
return createError("invalid e_phentsize: " +
Twine(getHeader().e_phentsize));

uint64_t HeadersSize =
(uint64_t)getHeader().e_phnum * getHeader().e_phentsize;
uint64_t HeadersSize = (uint64_t)NumPh * getHeader().e_phentsize;
uint64_t PhOff = getHeader().e_phoff;
if (PhOff + HeadersSize < PhOff || PhOff + HeadersSize > getBufSize())
return createError("program headers are longer than binary of size " +
Twine(getBufSize()) + ": e_phoff = 0x" +
Twine::utohexstr(getHeader().e_phoff) +
", e_phnum = " + Twine(getHeader().e_phnum) +
", e_phnum = " + Twine(NumPh) +
", e_phentsize = " + Twine(getHeader().e_phentsize));

auto *Begin = reinterpret_cast<const Elf_Phdr *>(base() + PhOff);
return ArrayRef(Begin, Begin + getHeader().e_phnum);
return ArrayRef(Begin, Begin + NumPh);
}

/// Get an iterator over notes in a program header.
Expand Down Expand Up @@ -772,19 +810,14 @@ template <class ELFT>
Expected<StringRef>
ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
WarningHandler WarnHandler) const {
uint32_t Index = getHeader().e_shstrndx;
if (Index == ELF::SHN_XINDEX) {
// If the section name string table section index is greater than
// or equal to SHN_LORESERVE, then the actual index of the section name
// string table section is contained in the sh_link field of the section
// header at index 0.
if (Sections.empty())
return createError(
"e_shstrndx == SHN_XINDEX, but the section header table is empty");

Index = Sections[0].sh_link;
Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
if (!ShStrNdxOrErr || (*ShStrNdxOrErr == ELF::SHN_XINDEX && RealShNum == 0)) {
consumeError(ShStrNdxOrErr.takeError());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be consuming the ShStrNdxOrErr here. Indeed, I'd be somewhat surprised if this didn't cause a failure in some configurations, if there wasn't actually an Error in the Expected (because the value is set to 0).

Also, the second half of the if isn't technically correct, since the section header 0 section might have sh_size 0 but sh_link 0xffff (in a dodgy ELF). This is probably a case where referencing the original e_shstrndx from the header makes sense.

I think the code should look like something this:

Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
if (!ShStrNdxOrErr)
  return ShStrNdxOrErr.takeError();

if (getHeader().e_shstrndx == SHN_XINDEX && Sections.empty())
  return createError(...);

I'm assuming that Sections.empty() works fine here, since that's what the old code did, but if not, due to other changes we've made, I'd be okay with *RealShStrNdx == 0 in place of it.

NB: there might be some edge cases where the error message ends up being different to the old code. In this specific case, I'm okay with that, as long as we have test coverage for the existing error produced by this function, when there are no sections.

return createError(
"e_shstrndx == SHN_XINDEX, but the section header table is empty");
}

uint32_t Index = *ShStrNdxOrErr;
// There is no section name string table. Return FakeSectionStrings which
// is non-empty if we have created fake sections.
if (!Index)
Expand Down Expand Up @@ -891,6 +924,39 @@ Expected<uint64_t> ELFFile<ELFT>::getDynSymtabSize() const {

template <class ELFT> ELFFile<ELFT>::ELFFile(StringRef Object) : Buf(Object) {}

template <class ELFT> Error ELFFile<ELFT>::readShdrZero() {
const Elf_Ehdr &Header = getHeader();

if ((Header.e_phnum == ELF::PN_XNUM || Header.e_shnum == 0 ||
Header.e_shstrndx == ELF::SHN_XINDEX) &&
Header.e_shoff != 0) {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't start blocks with a blank line.

// Pretend we have section 0 or sections() would call getShNum and thus
// become an infinite recursion
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// become an infinite recursion
// become an infinite recursion.

Nit: comments should be full sentences, with trailing full stops.

if (Header.e_shnum == 0)
RealShNum = 1;
else
RealShNum = Header.e_shnum;
auto SecOrErr = getSection(0);
if (!SecOrErr) {
RealShNum = std::nullopt;
return SecOrErr.takeError();
}

RealPhNum =
Header.e_phnum == ELF::PN_XNUM ? (*SecOrErr)->sh_info : Header.e_phnum;
RealShNum = Header.e_shnum == 0 ? (*SecOrErr)->sh_size : Header.e_shnum;
RealShStrNdx = Header.e_shstrndx == ELF::SHN_XINDEX ? (*SecOrErr)->sh_link
: Header.e_shstrndx;
} else {
RealPhNum = Header.e_phnum;
RealShNum = Header.e_shnum;
RealShStrNdx = Header.e_shstrndx;
}

return Error::success();
}

template <class ELFT>
Expected<ELFFile<ELFT>> ELFFile<ELFT>::create(StringRef Object) {
if (sizeof(Elf_Ehdr) > Object.size())
Expand Down Expand Up @@ -956,9 +1022,11 @@ Expected<typename ELFT::ShdrRange> ELFFile<ELFT>::sections() const {
const Elf_Shdr *First =
reinterpret_cast<const Elf_Shdr *>(base() + SectionTableOffset);

uintX_t NumSections = getHeader().e_shnum;
if (NumSections == 0)
NumSections = First->sh_size;
uintX_t NumSections = 0;
if (Expected<uint64_t> ShNumOrErr = getShNum())
NumSections = *ShNumOrErr;
else
return ShNumOrErr.takeError();

if (NumSections > UINT64_MAX / sizeof(Elf_Shdr))
return createError("invalid number of sections specified in the NULL "
Expand Down