This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Improve error reporting for relocations
ClosedPublic

Authored by phosek on Aug 18 2016, 12:20 PM.

Details

Summary

We should always include symbol name when reporting relocations error to simplify debugging of these issues. Without symbol names users have to manually investigate which of the libraries contain invalid relocations which can be cumbersome when linking multiple libraries.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek updated this revision to Diff 68591.Aug 18 2016, 12:20 PM
phosek retitled this revision from to [ELF] Improve error reporting for relocations.
phosek updated this object.
phosek added reviewers: ruiu, rafael.
phosek added a project: lld.
phosek added a subscriber: llvm-commits.
ruiu edited edge metadata.Aug 18 2016, 12:24 PM

Thank you for doing this!

ELF/Relocations.cpp
425–430 ↗(On Diff #68591)

You can call getName() on a local symbol, can't you?

phosek added inline comments.Aug 18 2016, 12:52 PM
ELF/Relocations.cpp
425–430 ↗(On Diff #68591)

There is an assert(!isLocal()) in SymbolBody::getName(). The name for local symbols is empty, but it I could use the name offset and find name in the File string table (if it's there), does that sound fine to you?

ruiu added inline comments.Aug 18 2016, 1:01 PM
ELF/Relocations.cpp
425–430 ↗(On Diff #68591)

Ah, right. Please define something like this and use it.

template <class ELFT>
static StringRef getLocalSymName(elf::ObjectFile<ELFT> &F, SymbolBody &B) {
  return F.getStringTable().data() + B.getNameOffset();
}
phosek updated this revision to Diff 68603.Aug 18 2016, 2:05 PM
phosek edited edge metadata.
phosek marked 3 inline comments as done.
ruiu accepted this revision.Aug 18 2016, 2:07 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 18 2016, 2:07 PM
This revision was automatically updated to reflect the committed changes.