[PATCH] D13501: [ELF2] Add -wrap switch

[Igor Kudrin via llvm-commits]

On 07.10.2015 20:22, Rui Ueyama wrote:
> On Wed, Oct 7, 2015 at 6:59 AM, Igor Kudrin <ikudrin.dev at gmail.com 
> <mailto:ikudrin.dev at gmail.com>> wrote:
>
>     ikudrin added a comment.
>
>     In http://reviews.llvm.org/D13501#261600, @ruiu wrote:
>
>     > This patch needs redesigning because we don't want to look up
>     hash tables more than once for each symbol. In this patch, names
>     for undefined symbols are looked up twice -- once in the
>     InputFile.cpp and the other in SymbolTable.cpp.
>
>
>     We have to look up for different names for defined and undefined
>     symbols, so we can't use just one hash map.
>
>     In most cases, when the -wrap switch is not used and
>     UndefSymNameReplacement is empty, addition lookup will be very
>     cheap, without calculating a hash value at all. On the other hand,
>     we can use just std::map which expected to work really quick in
>     our case.
>
>
> No, we don't have to look up hash table more than once. I thought a 
> bit more about this and noticed that the LLD architecture (the 
> separation of Symbol and SymbolBody) would really play well here. 
> Symbols are just pointers to SymbolBodies, and symbol renaming is as 
> easy as single pointer mutation. Here's the idea.
>
> First we resolve all symbols without considering --wrap option (so no 
> overhead for --wrap during file parsing and symbol resolution). When 
> symbol resolution is done, we basically have three Symbols for 
> --wrap'ed symbol S in the symbol table: S, __wrap_S, and __real_S. Let 
> X, Y and Z be their SymbolBodies, respectively.
>
> Originally the relationships are
>
>   S -> X
>   __wrap_S -> Y
>   __real_S -> Z
>
> We swap the pointers so that their relationships are
>
>   S -> Y
>   __wrap_S -> Y
>   __real_S -> X
>
> Now you can see that all symbols that would have been resolved to S 
> without --wrap are now resolved as if they were __wrap_S. __real_S is 
> also properly resolved to the symbol that was once the real symbol 
> pointed to. This is the same result as what --wrap expects.

Thank you for explaining your idea. It can work in some cases, but I 
dread it's unable to cover all situations.

1) Suppose we have all three symbols defined. The command like "ld.lld2 
a.o -wrap=S" in your case will eliminate Z and change content for 
symbols S and __real_S (see 
SymbolTableSection<ELFT>::writeGlobalSymbols). That's not right, GNU's 
ld preserves symbols in this case.

2) Suppose that S and __wrap_S are defined in different object files, 
which lay in archives. With switch "-wrap=S" (and if we have a reference 
to S) we should not get the content of the first object file at all, but 
how can we avoid parsing it if there is the record 'S' in the SymbolTable?

3) If we generate DSO and have an unresolved undefined symbol S we have 
to rename it and store as __wrap_S. It'll require more complicated 
update of symbols in the symbol table to support this case.

Finally, my approach, I hope, preserves the original idea of 
relationships in the symbol table. With my case, we will have links like 
these:
(Body for undefined S, name is "__wrap_S") -> (Symbol with name 
"__wrap_S") -> (Body for defined symbol "__wrap_S")
(Body for defined __wrap_S) -> (Symbol with name "__wrap_S") -> (Body 
for defined symbol "__wrap_S")
(Body for defined S) -> (Symbol with name "S") -> (Body for defined 
symbol "S")

With your proposal we'll end up with links like:
(Body for undefined S, name is "S") -> (Symbol with name "S") -> (Body 
for defined symbol "__wrap_S")
(Body for defined __wrap_S) -> (Symbol with name "__wrap_S") -> (Body 
for defined symbol "__wrap_S")
(Body for defined S) -> (Symbol with name "S") -> (Body for defined 
symbol "__wrap_S")

The last line is a bit unexpected and can be a source for unwitting bugs.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151007/96f191fd/attachment.html>