Fix for -wrap linker option and LTO, PR 33145

[dmikulin]

Summary

An attempt to fix PR33145, --wrap doesn't work with lto.
Wrapped symbols need to excluded from inter-procedural optimizations to prevent dropping symbols and allow the linker to process re-directs.
Current limitation: doesn't work with ThinLTO.

[davide]

There are no tests associated with this patch.

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

Can you explain why you need all three?

338–340 ↗

(On Diff #100521)

empty ifs are not really common in llvm.

lld/ELF/SymbolTable.h

140 ↗

(On Diff #100521)

A StringMap<bool> is a little silly.
If you really want a set container, you should use a proper one.

llvm/include/llvm/LTO/LTO.h

385 ↗

(On Diff #100521)

I'd say, LinkerRedefined.

llvm/lib/LTO/LTO.cpp

423 ↗

(On Diff #100521)

why do you need this being part of GlobalResolution?
If you do, please add a test showing that you fail switching the linkage earlier.

[ruiu]

Do you really need this much change? I expected something smaller than this. I mean, I thought what we need to do is to just pass a list of symbols which LTO should keep.

[dmikulin]

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

We need all three if we want to match non-LTO behavior.

• _wrap_bar is needed for the case where it is defined by the user but may be eliminated during LTO.
• _real_bar is needed for the test case in the PR: the direct call to _real_bar() from foo() will be bound and inlined by LTO to the user-defined _real_bar() instead of bar() if _real_bar() is not on the list.
• similarly, if bar() is not on the list, the call to bar() from foo() will be inlined instead of being routed to _wrap_bar().

338–340 ↗

(On Diff #100521)

I'll fix this. Negating the condition seamed counter-intuitive...

lld/ELF/SymbolTable.h

140 ↗

(On Diff #100521)

Is a set container like SmallSet good enough in this case? What's the worst case for the number of -wrap functions? I picked a map because we have to do a lookup in this container for every symbol.

llvm/include/llvm/LTO/LTO.h

385 ↗

(On Diff #100521)

I'll change this.

llvm/lib/LTO/LTO.cpp

423 ↗

(On Diff #100521)

You're right, I can do it sooner, in addRegularLTO(). I'll take out the GlobalRes stuff.

[davide]

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

Can you please add a test showing when 3) happens?

lld/ELF/SymbolTable.h

140 ↗

(On Diff #100521)

What I'm trying to say is that a map from something to bool is just a Set.
If you think your set will grow a lot (unlikely, but OK, I'd recommend DenseSet).

[dmikulin]

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

3. is also demonstrated by the test case in the PR. I'm working on regression tests for all known cases and will submit them with the new version of the patch.

lld/ELF/SymbolTable.h

140 ↗

(On Diff #100521)

OK, thanks. Will fix.

[pcc]

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

_wrap_bar is needed for the case where it is defined by the user but may be eliminated during LTO.

This one only requires VisibleToRegularObj, doesn't it?

338–340 ↗

(On Diff #100521)

Is the purpose of this code just to restore the binding of the original symbol? I think it would be simpler to have a post-LTO step that manually restores the binding for each renamed symbol rather than dealing with it here.

[pcc]

lld/ELF/SymbolTable.cpp

167 ↗

(On Diff #100521)

What about the rename target? If I pass --defsym=foo=bar, the linker should not allow IPO past foo, right?

[dmikulin]

lld/ELF/SymbolTable.cpp

159–163 ↗

(On Diff #100521)

Looks like it. Do we want to handle _wrap_* symbols specially and not set the weak binding for LTO?

167 ↗

(On Diff #100521)

Right. I'll add it to the list.

338–340 ↗

(On Diff #100521)

Yes, it was to restore the binding. I changed it to have a separate pass over renamed symbols after LTO.

[dmikulin]

Revised patch. Will add test cases tomorrow.

[pcc]

lld/ELF/SymbolTable.cpp

124 ↗

(On Diff #100812)

If you move the list of renamed symbols to Config you can avoid this lambda.

192 ↗

(On Diff #100812)

You mean Alias here, right?

159–163 ↗

(On Diff #100521)

Up to you. Note that you can make LTO keep a symbol by calling addUndefined with the symbol name, so it wouldn't be *that* special.

[davide]

I'll wait for your tests before another careful review. Some drive by comments.

lld/ELF/SymbolTable.cpp

134–136 ↗

(On Diff #100812)

Do you need an explicit iterator? Can't you use a range for ?

lld/ELF/SymbolTable.h

141 ↗

(On Diff #100812)

I'd be more explicit here. e.g. "this is a map from Symbol name to binding" or something like that.

[dmikulin]

lld/ELF/SymbolTable.cpp

124 ↗

(On Diff #100812)

OK, will do.

192 ↗

(On Diff #100812)

Of course.

lld/ELF/SymbolTable.h

141 ↗

(On Diff #100812)

OK

[dmikulin]

Addressed review comments. Added test cases. Tweaked saved pre-LTO bindings to more accurately reflect the state of the symbol table.

[davide]

This is getting better.
Please rebase the patch because it didn't apply cleanly.

lld/ELF/SymbolTable.cpp

131 ↗

(On Diff #100921)

Convention says all Variable names should start with capital letter. Also, i is a little vague.

llvm/include/llvm/LTO/LTO.h

369–370 ↗

(On Diff #100921)

Is this formatted? Doesn't seem so.

llvm/lib/LTO/LTO.cpp

421 ↗

(On Diff #100921)

If I remove this line, all the tests still pass.

441–442 ↗

(On Diff #100921)

If I comment out this line, all the tests still pass.

547–548 ↗

(On Diff #100921)

This needs a comment.

[dmikulin]

lld/ELF/SymbolTable.cpp

131 ↗

(On Diff #100921)

Changed it to RSI (Renamed Symbol Iterator)

llvm/lib/LTO/LTO.cpp

421 ↗

(On Diff #100921)

This was meant for ThinLTO support, which hasn't been implemented yet. I can take it out from this patch.

441–442 ↗

(On Diff #100921)

Added a test point for this

[pcc]

lld/test/ELF/lto/defsym.ll

13–14 ↗

(On Diff #100921)

These are addresses of PLT entries, right? Can you avoid magic numbers here and make what you are testing for more explicit? I think you can do that by making the symbols hidden and checking that the calls correspond to the correct symbol addresses. Same for wrap-2.ll.

llvm/lib/LTO/LTO.cpp

442 ↗

(On Diff #100921)

You also need to add support for 'r' to llvm-lto2. Please also add an llvm-lto2 based test that verifies that the linkage has changed.

[dmikulin]

Addressed review comments.
Made changes and added a test to llvm-lto2.

[dmikulin]

Changed the mapping from string --> binding to Symbol* --> binding.

[pcc]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

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;

[dmikulin]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

But __wrap_ symbols are different: they need to be on the list, but they are not being replaced.

[pcc]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

They only need to be marked as IsUsedInRegularObj. And that's taken care of for you by addUndefined.

[dmikulin]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

Map it to NULL?

[davide]

lld/ELF/Config.h

102 ↗

(On Diff #101247)

This is the only field in Config that has a comment. Not sure how I feel about it, I'll defer it to Rui.

lld/ELF/SymbolTable.cpp

191–194 ↗

(On Diff #101247)

Is this bit needed? I don't see a test for it.

195 ↗

(On Diff #101247)

Can this ever be null? If so, you should check for it.
If not, you may as well inline this in the single use.

lld/ELF/SymbolTable.h

42–43 ↗

(On Diff #101247)

Unsorted.

llvm/lib/LTO/LTO.cpp

407–410 ↗

(On Diff #101247)

This comment is now stale after your change.

[pcc]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

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

[dmikulin]

lld/ELF/SymbolTable.cpp

203–231 ↗

(On Diff #101247)

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.

[dmikulin]

lld/ELF/Config.h

102 ↗

(On Diff #101247)

Removed the comment. Now it's next to the RemappedSymbol structure.

lld/ELF/SymbolTable.cpp

191–194 ↗

(On Diff #101247)

It was moved here from SymbolTable::alias, there is a test case for it already.

195 ↗

(On Diff #101247)

It can't be null, I just find it easier to read, that's all.

[dmikulin]

Changed the way -defsym and -wrap are handled.

[pcc]

lld/ELF/Driver.cpp

972 ↗

(On Diff #101280)

This comment and the one below aren't really accurate any more because this code is used in the non-LTO case as well.

lld/ELF/LTO.cpp

139 ↗

(On Diff #101280)

I think this can just be Config->RenamedSymbols.count(Sym).

lld/ELF/SymbolTable.cpp

160 ↗

(On Diff #101280)

Is this function necessary? All callers pass a non-null SymbolBody, so they can just query the binding directly.

167–169 ↗

(On Diff #101280)

Move this comment to applySymbolRenames.

172 ↗

(On Diff #101280)

I'd minimise the diff here and rename this variable back.

[dmikulin]

Fixed comments.
Rebased the patch.

[davide]

lld/ELF/SymbolTable.cpp

167 ↗

(On Diff #101289)

You're setting IsUsedInRegularObj from what I can see, here, for all the symbols passed with --wrap.
Can you check this is really NFC? IIRC it could impact the way symbols are added (or not) to the symbol table.

184 ↗

(On Diff #101289)

Same here.

[• rafael]

This LGTM. I am not a big fan of keeping a copy or the original binding, but it sure saves a lot of plumbing.

Please just wait to see if Davide and Peter have something to add.

[pcc]

LGTM as well

lld/test/ELF/lto/wrap-1.ll

13 ↗

(On Diff #101289)

Please update this comment. I think you also want to test 'r' for __real_bar.

[davide]

I'm fine with this once Peter's comments are addressed. I'm not a fan of having to carry around the original binding as well, but I think it's tolerable.

[ruiu]

lld/ELF/Config.h

107 ↗

(On Diff #101289)

Please run clang-format-diff. It should be formatted as <Symbol *, RenamedSymbol>

lld/ELF/Driver.cpp

973 ↗

(On Diff #101289)

I prefer a more concrete comment. Handle -wrap option.

977 ↗

(On Diff #101289)

Ditto. Handle --defsym=sym=alias option.

lld/ELF/SymbolTable.cpp

166 ↗

(On Diff #101289)

Add a blank line before a comment.

183 ↗

(On Diff #101289)

Add a blank line before a comment.

189 ↗

(On Diff #101289)

It is not obvious why you need this function from the source code. You need to add a comment.

190 ↗

(On Diff #101289)

What is RSI? I found just KV (short for key-value) makes sense in most cases.

194 ↗

(On Diff #101289)

Ditto

[dmikulin]

lld/ELF/SymbolTable.cpp

190 ↗

(On Diff #101289)

RSI is supposed to be Renamed Symbol Iterator, but KV is ok with me too

lld/test/ELF/lto/wrap-1.ll

13 ↗

(On Diff #101289)

Resolutions file is driven by input file symbol tables. Since __real_bar is not defined in an object, it does not show up there. I added a definition and a test for the 'r' flag.

[davide]

There were some comments pending by Rui. You addressed them, but you still need to wait for an explicit LGTM before committing.
I don't think it's worth reverting this patch, but this is not okay and you should consider it next time.

lld/trunk/ELF/Config.h

 // can be found in version script if it is used for link.
 struct VersionDefinition {
   llvm::StringRef Name;
   uint16_t Id = 0;
   std::vector<SymbolVersion> Globals;
   size_t NameOff = 0; // Offset in the string table
 };
 
+// Structure for mapping renamed symbols
+struct RenamedSymbol {
+  Symbol *Target;
+  uint8_t OrigBinding;
+};
+
 // This struct contains the global configuration for the linker.
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {
   InputFile *FirstElf = nullptr;
   uint8_t OSABI = 0;
   llvm::CachePruningPolicy ThinLTOCachePolicy;
struct Configuration {
   std::vector<VersionDefinition> VersionDefinitions;
   std::vector<llvm::StringRef> AuxiliaryList;
   std::vector<llvm::StringRef> SearchPaths;
   std::vector<llvm::StringRef> SymbolOrderingFile;
   std::vector<llvm::StringRef> Undefined;
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<SymbolVersion> VersionScriptLocals;
   std::vector<uint8_t> BuildIdVector;
+  llvm::MapVector<Symbol *, RenamedSymbol> RenamedSymbols;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;
   bool ColorDiagnostics = false;
   bool CompressDebugSections;
   bool DefineCommon;
   bool Demangle = true;

lld/trunk/ELF/Driver.cpp

template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   // Return if there were name resolution errors.
   if (ErrorCount)
     return;
 
   Symtab.scanUndefinedFlags();
   Symtab.scanShlibUndefined();
   Symtab.scanVersionScript();
 
+  // Create wrapped symbols for -wrap option.
+  for (auto *Arg : Args.filtered(OPT_wrap))
+    Symtab.addSymbolWrap(Arg->getValue());
+
+  // Create alias symbols for -defsym option.
+  for (std::pair<StringRef, StringRef> &Def : getDefsym(Args))
+    Symtab.addSymbolAlias(Def.first, Def.second);
+
   Symtab.addCombinedLTOObject();
   if (ErrorCount)
     return;
 
   // Some symbols (such as __ehdr_start) are defined lazily only when there
   // are undefined symbols for them, so we add these to trigger that logic.
   for (StringRef Sym : Script->Opt.ReferencedSymbols)
     Symtab.addUndefined(Sym);
 
-  for (auto *Arg : Args.filtered(OPT_wrap))
-    Symtab.wrap(Arg->getValue());
-
-  // Handle --defsym=sym=alias option.
-  for (std::pair<StringRef, StringRef> &Def : getDefsym(Args))
-    Symtab.alias(Def.first, Def.second);
+  // Apply symbol renames for -wrap and -defsym
+  Symtab.applySymbolRenames();
 
   // Now that we have a complete list of input files.
   // Beyond this point, no new files are added.
   // Aggregate all input sections into one place.
   for (elf::ObjectFile<ELFT> *F : Symtab.getObjectFiles())
     for (InputSectionBase *S : F->getSections())
       if (S && S != &InputSection::Discarded)
         InputSections.push_back(S);

lld/trunk/ELF/LTO.cpp

for (const lto::InputFile::Symbol &ObjSym : Obj.symbols()) {
     // Once IRObjectFile is fixed to report only one symbol this hack can
     // be removed.
     R.Prevailing = !ObjSym.isUndefined() && B->File == &F;
 
     R.VisibleToRegularObj =
         Sym->IsUsedInRegularObj || (R.Prevailing && Sym->includeInDynsym());
     if (R.Prevailing)
       undefine(Sym);
+    R.LinkerRedefined = Config->RenamedSymbols.count(Sym);
   }
   checkError(LTOObj->add(std::move(F.Obj), Resols));
 }
 
 // Merge all the bitcode files we have seen, codegen the result
 // and return the resulting ObjectFile(s).
 std::vector<InputFile *> BitcodeCompiler::compile() {
   std::vector<InputFile *> Ret;

lld/trunk/ELF/SymbolTable.h

 // add*() functions, which are called by input files as they are parsed. There
 // is one add* function per symbol type.
 template <class ELFT> class SymbolTable {
   typedef typename ELFT::Sym Elf_Sym;
 
 public:
   void addFile(InputFile *File);
   void addCombinedLTOObject();
+  void addSymbolAlias(StringRef Alias, StringRef Name);
+  void addSymbolWrap(StringRef Name);
+  void applySymbolRenames();
 
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
   ArrayRef<ObjectFile<ELFT> *> getObjectFiles() const { return ObjectFiles; }
   ArrayRef<BinaryFile *> getBinaryFiles() const { return BinaryFiles; }
   ArrayRef<SharedFile<ELFT> *> getSharedFiles() const { return SharedFiles; }
 
   DefinedRegular *addAbsolute(StringRef Name,
                               uint8_t Visibility = llvm::ELF::STV_HIDDEN,
public:
   void scanUndefinedFlags();
   void scanShlibUndefined();
   void scanVersionScript();
 
   SymbolBody *find(StringRef Name);
   SymbolBody *findInCurrentDSO(StringRef Name);
 
   void trace(StringRef Name);
-  void wrap(StringRef Name);
-  void alias(StringRef Alias, StringRef Name);
 
 private:
   std::vector<SymbolBody *> findByVersion(SymbolVersion Ver);
   std::vector<SymbolBody *> findAllByVersion(SymbolVersion Ver);
 
   llvm::StringMap<std::vector<SymbolBody *>> &getDemangledSyms();
   void handleAnonymousVersion();
   void assignExactVersion(SymbolVersion Ver, uint16_t VersionId,

lld/trunk/ELF/SymbolTable.cpp

 // Set a flag for --trace-symbol so that we can print out a log message
 // if a new symbol with the same name is inserted into the symbol table.
 template <class ELFT> void SymbolTable<ELFT>::trace(StringRef Name) {
   Symtab.insert({CachedHashStringRef(Name), {-1, true}});
 }
 
 // Rename SYM as __wrap_SYM. The original symbol is preserved as __real_SYM.
 // Used to implement --wrap.
-template <class ELFT> void SymbolTable<ELFT>::wrap(StringRef Name) {
+template <class ELFT> void SymbolTable<ELFT>::addSymbolWrap(StringRef Name) {
   SymbolBody *B = find(Name);
   if (!B)
     return;
   Symbol *Sym = B->symbol();
   Symbol *Real = addUndefined(Saver.save("__real_" + Name));
   Symbol *Wrap = addUndefined(Saver.save("__wrap_" + Name));
 
-  // We rename symbols by replacing the old symbol's SymbolBody with the new
-  // symbol's SymbolBody. This causes all SymbolBody pointers referring to the
-  // old symbol to instead refer to the new symbol.
-  memcpy(Real->Body.buffer, Sym->Body.buffer, sizeof(Sym->Body));
-  memcpy(Sym->Body.buffer, Wrap->Body.buffer, sizeof(Wrap->Body));
+  // Tell LTO not to eliminate this symbol
+  Wrap->IsUsedInRegularObj = true;
+
+  Config->RenamedSymbols[Real] = RenamedSymbol{Sym, Real->Binding};
+  Config->RenamedSymbols[Sym] = RenamedSymbol{Wrap, Sym->Binding};
 }
 
 // Creates alias for symbol. Used to implement --defsym=ALIAS=SYM.
-template <class ELFT>
-void SymbolTable<ELFT>::alias(StringRef Alias, StringRef Name) {
+template <class ELFT> void SymbolTable<ELFT>::addSymbolAlias(StringRef Alias,
+                                                             StringRef Name) {
   SymbolBody *B = find(Name);
   if (!B) {
     error("-defsym: undefined symbol: " + Name);
     return;
   }
   Symbol *Sym = B->symbol();
   Symbol *AliasSym = addUndefined(Alias);
-  memcpy(AliasSym->Body.buffer, Sym->Body.buffer, sizeof(AliasSym->Body));
+
+  // Tell LTO not to eliminate this symbol
+  Sym->IsUsedInRegularObj = true;
+  Config->RenamedSymbols[AliasSym] = RenamedSymbol{Sym, AliasSym->Binding};
+}
+
+// Apply symbol renames created by -wrap and -defsym. The renames are created
+// before LTO in addSymbolWrap() and addSymbolAlias() to have a chance to inform
+// LTO (if LTO is running) not to include these symbols in IPO. Now that the
+// symbols are finalized, we can perform the replacement.
+template <class ELFT> void SymbolTable<ELFT>::applySymbolRenames() {
+  for (auto &KV : Config->RenamedSymbols) {
+    Symbol *Sym = KV.first;
+    Symbol *Rename = KV.second.Target;
+    Sym->Binding = KV.second.OrigBinding;
+
+    // We rename symbols by replacing the old symbol's SymbolBody with the new
+    // symbol's SymbolBody. This causes all SymbolBody pointers referring to the
+    // old symbol to instead refer to the new symbol.
+    memcpy(Sym->Body.buffer, Rename->Body.buffer, sizeof(Sym->Body));
+  }
 }
 
 static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VA == STV_DEFAULT)
     return VB;
   if (VB == STV_DEFAULT)
     return VA;
   return std::min(VA, VB);

lld/trunk/test/ELF/lto/Inputs/defsym-bar.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @this_is_bar1()
+declare void @this_is_bar2()
+declare void @this_is_bar3()
+
+define hidden void @bar1() {
+  call void @this_is_bar1()
+  ret void
+}
+
+define hidden void @bar2() {
+  call void @this_is_bar2()
+  ret void
+}
+
+define hidden void @bar3() {
+  call void @this_is_bar3()
+  ret void
+}

lld/trunk/test/ELF/lto/Inputs/wrap-bar.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define hidden void @bar() {
+  ret void
+}
+
+define hidden void @__real_bar() {
+  ret void
+}
+
+define hidden void @__wrap_bar() {
+  ret void
+}

lld/trunk/test/ELF/lto/defsym.ll

+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: llvm-as %S/Inputs/defsym-bar.ll -o %t1.o
+; RUN: ld.lld %t.o %t1.o -shared -o %t.so -defsym=bar2=bar3
+; RUN: llvm-objdump -d %t.so | FileCheck %s
+
+; Call to bar2() should not be inlined and should be routed to bar3()
+; Symbol bar3 should not be eliminated
+
+; CHECK:      foo:
+; CHECK-NEXT: pushq %rax
+; CHECK-NEXT: callq
+; CHECK-NEXT: callq{{.*}}<bar3>
+; CHECK-NEXT: callq
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar1()
+declare void @bar2()
+declare void @bar3()
+
+define void @foo() {
+  call void @bar1()
+  call void @bar2()
+  call void @bar3()
+  ret void
+}

lld/trunk/test/ELF/lto/wrap-1.ll

+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -o %t.out -wrap=bar -save-temps
+; RUN: llvm-readobj -t %t.out | FileCheck %s
+; RUN: cat %t.out.resolution.txt | FileCheck -check-prefix=RESOLS %s
+
+; CHECK:      Name: __wrap_bar
+; CHECK-NEXT: Value:
+; CHECK-NEXT: Size:
+; CHECK-NEXT: Binding: Global
+; CHECK-NEXT: Type: Function
+
+; Make sure that the 'r' (linker redefined) bit is set for bar and __wrap_bar
+; in the resolutions file.
+; RESOLS: ,bar,r
+; RESOLS: ,__wrap_bar,px
+; RESOLS: ,__real_bar,pxr
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+
+define void @_start() {
+  call void @bar()
+  ret void
+}
+
+define void @__wrap_bar() {
+  ret void
+}
+
+define void @__real_bar() {
+  ret void
+}

lld/trunk/test/ELF/lto/wrap-2.ll

+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: llvm-as %S/Inputs/wrap-bar.ll -o %t1.o
+; RUN: ld.lld %t.o %t1.o -shared -o %t.so -wrap=bar
+; RUN: llvm-objdump -d %t.so | FileCheck %s
+; RUN: llvm-readobj -t %t.so | FileCheck -check-prefix=BIND %s
+
+; Make sure that calls in foo() are not eliminated and that bar is
+; routed to __wrap_bar and __real_bar is routed to bar.
+
+; CHECK:      foo:
+; CHECK-NEXT: pushq %rax
+; CHECK-NEXT: callq{{.*}}<__wrap_bar>
+; CHECK-NEXT: callq{{.*}}<bar>
+
+; Check that bar and __wrap_bar retain their original binding.
+; BIND:      Name: bar
+; BIND-NEXT: Value:
+; BIND-NEXT: Size:
+; BIND-NEXT: Binding: Local
+; BIND:      Name: __wrap_bar
+; BIND-NEXT: Value:
+; BIND-NEXT: Size:
+; BIND-NEXT: Binding: Local
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+declare void @__real_bar()
+
+define void @foo() {
+  call void @bar()
+  call void @__real_bar()
+  ret void
+}

llvm/trunk/include/llvm/LTO/LTO.h

private:
 
   mutable bool CalledGetMaxTasks = false;
 };
 
 /// The resolution for a symbol. The linker must provide a SymbolResolution for
 /// each global symbol based on its internal resolution of that symbol.
 struct SymbolResolution {
   SymbolResolution()
-      : Prevailing(0), FinalDefinitionInLinkageUnit(0), VisibleToRegularObj(0) {
-  }
+      : Prevailing(0), FinalDefinitionInLinkageUnit(0), VisibleToRegularObj(0),
+        LinkerRedefined(0) {}
+
   /// The linker has chosen this definition of the symbol.
   unsigned Prevailing : 1;
 
   /// The definition of this symbol is unpreemptable at runtime and is known to
   /// be in this linkage unit.
   unsigned FinalDefinitionInLinkageUnit : 1;
 
   /// The definition of this symbol is visible outside of the LTO unit.
   unsigned VisibleToRegularObj : 1;
+
+  /// Linker redefined version of the symbol which appeared in -wrap or -defsym
+  /// linker option.
+  unsigned LinkerRedefined : 1;
 };
 
 } // namespace lto
 } // namespace llvm
 
 #endif

llvm/trunk/lib/LTO/LTO.cpp

 // Add the given symbol to the GlobalResolutions map, and resolve its partition.
 void LTO::addSymbolToGlobalRes(const InputFile::Symbol &Sym,
                                SymbolResolution Res, unsigned Partition) {
   auto &GlobalRes = GlobalResolutions[Sym.getName()];
   GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr();
   if (Res.Prevailing)
     GlobalRes.IRName = Sym.getIRName();
 
-  // Set the partition to external if we know it is used elsewhere, e.g.
-  // it is visible to a regular object, is referenced from llvm.compiler_used,
-  // or was already recorded as being referenced from a different partition.
-  if (Res.VisibleToRegularObj || Sym.isUsed() ||
+  // Set the partition to external if we know it is re-defined by the linker
+  // with -defsym or -wrap options, used elsewhere, e.g. it is visible to a
+  // regular object, is referenced from llvm.compiler_used, or was already
+  // recorded as being referenced from a different partition.
+  if (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
       (GlobalRes.Partition != GlobalResolution::Unknown &&
        GlobalRes.Partition != Partition)) {
     GlobalRes.Partition = GlobalResolution::External;
   } else
     // First recorded reference, save the current partition.
     GlobalRes.Partition = Partition;
 
   // Flag as visible outside of ThinLTO if visible from a regular object or
for (const InputFile::Symbol &Sym : Input->symbols()) {
 
     OS << "-r=" << Path << ',' << Sym.getName() << ',';
     if (Res.Prevailing)
       OS << 'p';
     if (Res.FinalDefinitionInLinkageUnit)
       OS << 'l';
     if (Res.VisibleToRegularObj)
       OS << 'x';
+    if (Res.LinkerRedefined)
+      OS << 'r';
     OS << '\n';
   }
   OS.flush();
   assert(ResI == Res.end());
 }
 
 Error LTO::add(std::unique_ptr<InputFile> Input,
                ArrayRef<SymbolResolution> Res) {
for (const InputFile::Symbol &Sym : Syms) {
     ModuleSymbolTable::Symbol Msym = *MsymI++;
     Skip();
 
     if (GlobalValue *GV = Msym.dyn_cast<GlobalValue *>()) {
       if (Res.Prevailing) {
         if (Sym.isUndefined())
           continue;
         Keep.push_back(GV);
+        // For symbols re-defined with linker -wrap and -defsym options,
+        // set the linkage to weak to inhibit IPO. The linkage will be
+        // restored by the linker.
+        if (Res.LinkerRedefined)
+          GV->setLinkage(GlobalValue::WeakAnyLinkage);
+
         GlobalValue::LinkageTypes OriginalLinkage = GV->getLinkage();
         if (GlobalValue::isLinkOnceLinkage(OriginalLinkage))
           GV->setLinkage(GlobalValue::getWeakLinkage(
               GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
       } else if (isa<GlobalObject>(GV) &&
                  (GV->hasLinkOnceODRLinkage() || GV->hasWeakODRLinkage() ||
                   GV->hasAvailableExternallyLinkage()) &&
                  !AliasedGlobals.count(cast<GlobalObject>(GV))) {

llvm/trunk/test/LTO/Resolution/X86/linker-redef.ll

+; RUN: llvm-as %s -o %t.o
+; RUN: llvm-lto2 run -o %t1.o %t.o -r %t.o,bar,pr
+; RUN: llvm-readobj -t %t1.o.0 | FileCheck %s
+
+; CHECK: Name: bar
+; CHECK-NEXT: Value:
+; CHECK-NEXT: Size:
+; CHECK-NEXT: Binding: Weak
+; CHECK-NEXT: Type: Function
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}

llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp

for (std::string R : SymbolResolutions) {
     SymbolResolution Res;
     for (char C : Rest) {
       if (C == 'p')
         Res.Prevailing = true;
       else if (C == 'l')
         Res.FinalDefinitionInLinkageUnit = true;
       else if (C == 'x')
         Res.VisibleToRegularObj = true;
+      else if (C == 'r')
+        Res.LinkerRedefined = true;
       else {
         llvm::errs() << "invalid character " << C << " in resolution: " << R
                      << '\n';
         return 1;
       }
     }
     CommandLineResolutions[{FileName, SymbolName}].push_back(Res);
   }