This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Check that .dynsym is present in DSO if SHT_GNU_versym section is.
ClosedPublic

Authored by grimar on Oct 13 2016, 4:51 AM.

Details

Summary

When we have SHT_GNU_versym section, it is should be associated with symbol table
section. Usually (and in out implementation) it is .dynsym.
In case when .dynsym is absent (due to broken object for example),
lld crashes in parseVerdefs() when accesses null pointer:

Versym = reinterpret_cast<const Elf_Versym *>(this->ELFObj.base() +
                                              VersymSec->sh_offset) +
         this->Symtab->sh_info;

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 74495.Oct 13 2016, 4:51 AM
grimar retitled this revision from to [ELF] - Check that .dynsym is present in DSO if SHT_GNU_versym section is..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, evgeny777.
ruiu added inline comments.Oct 13 2016, 10:55 AM
ELF/InputFiles.cpp
540 ↗(On Diff #74495)

Can this actually happen?

grimar updated this revision to Diff 74654.Oct 14 2016, 3:58 AM
  • Changed testcase to use yaml2obj.
  • Restored check removed by mistake.
grimar added inline comments.Oct 14 2016, 4:00 AM
ELF/InputFiles.cpp
540 ↗(On Diff #74495)

I was able to get it using yaml2obj. That is the only produccer I know, but that seems enough to fix ?
Updated testcase.

ELF/SymbolTable.cpp
79 ↗(On Diff #74654)

I used this check so we able to error() instead of fatal().
fatal() is more consistent for broken inputs, but I think error() is still more preferable sometimes (at least it is better for case when lld used as a lib) ?

rafael accepted this revision.Oct 28 2016, 1:41 PM
rafael edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 28 2016, 1:41 PM
This revision was automatically updated to reflect the committed changes.