This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Make checks in ObjectFile<ELFT>::getSection() stricter.
ClosedPublic

Authored by grimar on Oct 4 2016, 4:54 AM.

Details

Summary

One of the change we made in the past was:

" // STT_SECTION symbols can be

// associated with SHT_REL[A]/SHT_SYMTAB/SHT_STRTAB sections.
// In this case it is fine for section to be null here as we
// do not allocate sections of these types."

This patch makes the check for null section stricter, so it is only allowed for STT_SECTION symbols now.
I did it because testcase that contains local unnamed symbol crashes without that check in:

template <class ELFT>
static bool shouldKeepInSymtab(InputSectionBase<ELFT> *Sec, StringRef SymName,
                               const SymbolBody &B) {
...
  return !(Sec->getSectionHdr()->sh_flags & SHF_MERGE); // HERE, because Sec is null
}

Diff Detail

Event Timeline

grimar updated this revision to Diff 73457.Oct 4 2016, 4:54 AM
grimar retitled this revision from to [ELF] - Make checks in ObjectFile<ELFT>::getSection() stricter..
grimar updated this object.
grimar added reviewers: ruiu, rafael, davide.
grimar updated this object.
grimar added subscribers: llvm-commits, grimar.
grimar added a subscriber: evgeny777.
ruiu added inline comments.Oct 4 2016, 9:52 AM
ELF/InputFiles.cpp
370

This code seems a bit too dense. I'd relax it like this.

if (Index >= Sections.size())
  fatal(getFilename(this) + ": invalid section index: " + Twine(Index));
InputSectionBase<ELFT> *S = Sections[Index];

// We found that GNU assembler 2.17.50 [FreeBSD] 2007-07-03
// could generate broken objects. STT_SECTION symbols can be
// associated with SHT_REL[A]/SHT_SYMTAB/SHT_STRTAB sections.
// In this case it is fine for section to be null here as we
// do not allocate sections of these types.
if (!S) {
  if (Sym.getType() == STT_SECTION)
    return;
  fatal(getFilename(this) + ": invalid section index: " + Twine(Index));
}

if (S == &InputSectionBase<ELFT>::Discarded)
  return S;
return S->Repl;
grimar updated this revision to Diff 73615.Oct 5 2016, 3:35 AM
  • Addressed review comments.
ruiu accepted this revision.Oct 5 2016, 11:03 AM
ruiu edited edge metadata.

LGTM

ELF/InputFiles.cpp
370

Add a blank line before this line.

This revision is now accepted and ready to land.Oct 5 2016, 11:03 AM
grimar closed this revision.Oct 6 2016, 3:16 AM

r283426