[PATCH] D33621: Fix for -wrap linker option and LTO, PR 33145

[Dmitry Mikulin via Phabricator via llvm-commits]

dmikulin added inline comments.


================
Comment at: lld/ELF/SymbolTable.cpp:203-231
 template <class ELFT> void SymbolTable<ELFT>::wrap(StringRef Name) {
-  SymbolBody *B = find(Name);
-  if (!B)
+  SymbolBody *SB = find(Name);
+  if (!SB)
     return;
-  Symbol *Sym = B->symbol();
-  Symbol *Real = addUndefined(Saver.save("__real_" + Name));
-  Symbol *Wrap = addUndefined(Saver.save("__wrap_" + Name));
+  Symbol *Sym = SB->symbol();
+  SymbolBody *RB = find("__real_" + Name.str());
+  SymbolBody *WB = find("__wrap_" + Name.str());
----------------
pcc wrote:
> dmikulin wrote:
> > pcc wrote:
> > > dmikulin wrote:
> > > > pcc wrote:
> > > > > Can we now replace these functions with something that applies the rename to each symbol in `Config->RenamedSymbols`? I think it should be possible with this change to how we declare `RenamedSymbols`.
> > > > > 
> > > > > ```
> > > > > struct RenamedSymbol {
> > > > >   uint8_t OrigBinding;
> > > > >   Symbol *Target;
> > > > > };
> > > > > MapVector<Symbol*, RenamedSymbol> RenamedSymbols;
> > > > > ```
> > > > But __wrap_ symbols are different: they need to be on the list, but they are not being replaced.
> > > They only need to be marked as `IsUsedInRegularObj`. And that's taken care of for you by `addUndefined`.
> > Map it to NULL?
> I mean that you can just set `IsUsedInRegularObj` on the `__wrap_` symbol before you assign it to `Target`. Then LTO will keep it alive. There's no need to add it (as a key) to `RenamedSymbols` then.
> http://llvm-cs.pcc.me.uk/tools/lld/ELF/LTO.cpp#136
This works fine for -wrap, but not for -defsym.

For -wrap, we want to keep the `__real_` and the original symbols on the list, so the mappings are:
Real --> Orig and Orig --> Wrap
After we're done with LTO, we copy targets of the mapping back to their keys, Orig --> Real and Wrap --> Orig.

For -defsym, we want to preserve the original symbol, so we would add Orig --> Alias mapping to communicate with LTO. But after we're done, we need to copy Orig --> Alias. I added a flag to the mapping structure to reverse the direction of the copy, so now in the -defsym test the call to bar3() gets inlined and the call to bar2() goes to its alias, bar3. This sounds reasonable to me, just want to make sure it's acceptable.


https://reviews.llvm.org/D33621