This is an archive of the discontinued LLVM Phabricator instance.

[lld][ELF][ARM] Mapping symbols for PLT entries
ClosedPublic

Authored by denis-protivensky on Apr 3 2015, 1:55 AM.

Details

Summary

Make PLT entry atoms represent mapping symbols in the Release mode, while in the Debug mode they are still function-like symbols with regular names.
It's legal that mapping symbols denote unnamed parts of code, and PLT entries are not required to have function-like names (in fact, they didn't have these names before in the Release mode).
This trick is done to simplify the code and not keep mapping symbol atoms along with regular PLT atoms.

Diff Detail

Event Timeline

denis-protivensky retitled this revision from to [lld][ELF][ARM] Mapping symbols for PLT entries.
denis-protivensky updated this object.
denis-protivensky edited the test plan for this revision. (Show Details)
denis-protivensky added reviewers: ruiu, shankarke.
denis-protivensky added a project: lld.
denis-protivensky added a subscriber: Unknown Object (MLST).
ruiu edited edge metadata.Apr 3 2015, 11:21 AM

The code generally looks good, but I'm wondering if it's a good thing to link an executable in a different way only when the linker itself was compiled with NDEBUG. Should the debug flag affects outputs?

I saw this pattern in many places in the ELF port of the LLD, so this comment is not actually specific to your patch, though.

lib/ReaderWriter/ELF/ARM/ARMRelocationPass.cpp
71

Maybe replacing this line (and similar lines) with a return statement could make this code a bit concise? (I didn't try myself, so I don't know how it would look like -- it's up to you.)

return part.empty() ? "$a" : "$a." + part;

Concerning different output when compiling with NDEBUG: these extra symbols do really help debugging sometimes. That's a good way to check that some call refers PLT entry, or veneer is used for some operations. I treat these symbol names as extra debug output and nothing more.

lib/ReaderWriter/ELF/ARM/ARMRelocationPass.cpp
71

I'll take it into account.

ruiu accepted this revision.Apr 6 2015, 10:19 AM
ruiu edited edge metadata.

LGTM. The thing about NDEBUG is not new, so we can discuss that later.

This revision is now accepted and ready to land.Apr 6 2015, 10:19 AM
This revision was automatically updated to reflect the committed changes.