This is an archive of the discontinued LLVM Phabricator instance.

ELF: Implement --wrap.
ClosedPublic

Authored by ruiu on Jan 5 2016, 11:21 AM.

Details

Reviewers
grimar
Summary

In this patch, all symbols are resolved normally and then wrap options
are applied. Renaming is implemented by mutating Body pointers of
Symbols. (As a result, Symtab.find(SymbolName)->getName() may return
a string that's different from SymbolName, but that is by design.
I designed the symbol and the symbol table to allow this kind of
operations.)

Diff Detail

Event Timeline

ruiu updated this revision to Diff 44030.Jan 5 2016, 11:21 AM
ruiu retitled this revision from to ELF: Implement --wrap..
ruiu updated this object.
ruiu added a reviewer: grimar.
ruiu added a subscriber: llvm-commits.
grimar added inline comments.Jan 6 2016, 6:35 AM
ELF/Options.td
114

gold and bfd help descriptions says "Use wrapper functions for SYMBOL".
Thats probably not just consistent but also a bit closer to what flag do since there are 2 wrappers used: real_XXX and wrap_XXX, not just only one

ELF/Symbols.h
109

Not sure here about naming. We already have setter named:

void setBackref(Symbol *P) { Backref = P; }

but your getter is named getSymbol. May be would be better to name them consistently (setBackref,getBackref or setSymbol,getSymbol) or even just do Backref member to be public and remove getter and setter ?

test/ELF/wrap.s
6

What about also testing alias and --wrap forms ?

ruiu marked 2 inline comments as done.Jan 6 2016, 11:54 AM
ruiu added inline comments.
ELF/Symbols.h
109

Yeah, there's a confusion on the naming scheme here. The root problem is that we split a symbol into Symbol and SymbolBody, but we often just say "symbol" to talk about the latter. getSymbol() is the right name because it's a member function of SymbolBody to retrieve a Symbol for that SymbolBody.

I have no clear idea about what to do here. We might want to change setBackref to setSymbol, but I'm not sure if it helps improve readability. I'd like to keep it as is for now until we find a better name.

ruiu updated this revision to Diff 44142.Jan 6 2016, 11:55 AM

Updated as per George's comments.

grimar accepted this revision.Jan 7 2016, 1:26 AM
grimar edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 7 2016, 1:26 AM
ruiu closed this revision.Jan 7 2016, 9:25 AM