[PATCH] D35064: [lib/LTO] Fix the interaction between linker redefined symbols and ThinLTO

[Davide Italiano via Phabricator via llvm-commits]

davide updated this revision to Diff 105477.
davide added a comment.

Actually, I now think I understand this slightly better.
As this was handling only the update of linkage to `available_externally`, I think the easiest solution is that of moving the early return later and in case the original and the linkage in the summary don't match, and the one in the summary is weak, just switch to it.
I think this is safe, but maybe we might want to do the switch only for some kind of (original) linkages, maybe.


https://reviews.llvm.org/D35064

Files:
  lib/LTO/LTO.cpp
  lib/Transforms/IPO/FunctionImport.cpp
  test/LTO/Resolution/X86/linker-redef-thin.ll


Index: test/LTO/Resolution/X86/linker-redef-thin.ll
===================================================================
--- /dev/null
+++ test/LTO/Resolution/X86/linker-redef-thin.ll
@@ -0,0 +1,16 @@
+; RUN: opt -module-summary %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
+}
Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -537,15 +537,24 @@
   };
 
   auto updateLinkage = [&](GlobalValue &GV) {
-    if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
-      return;
     // See if the global summary analysis computed a new resolved linkage.
     const auto &GS = DefinedGlobals.find(GV.getGUID());
     if (GS == DefinedGlobals.end())
       return;
     auto NewLinkage = GS->second->linkage();
     if (NewLinkage == GV.getLinkage())
       return;
+
+    // Switch the linkage to weakany for linker redefined symbols
+    // (via --wrap or --defsym).
+    if (NewLinkage == GlobalValue::WeakAnyLinkage) {
+      GV.setLinkage(NewLinkage);
+      return;
+    }
+
+    // Handle switch to available_externally.
+    if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
+      return;
     // Check for a non-prevailing def that has interposable linkage
     // (e.g. non-odr weak or linkonce). In that case we can't simply
     // convert to available_externally, since it would lose the
Index: lib/LTO/LTO.cpp
===================================================================
--- lib/LTO/LTO.cpp
+++ lib/LTO/LTO.cpp
@@ -665,6 +665,15 @@
         auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
             Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
         ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
+
+        // For linker redefined symbols (via --wrap or --defsym) we want to
+        // switch the linkage to `weak` to prevent IPOs from happening.
+        // Find the summary in the module for this very GV and record the new
+        // linkage so that we can switch it when we import the GV.
+        auto S = ThinLTO.CombinedIndex.findSummaryInModule(
+            GUID, BM.getModuleIdentifier());
+        if (S && Res.LinkerRedefined)
+          S->setLinkage(GlobalValue::WeakAnyLinkage);
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35064.105477.patch
Type: text/x-patch
Size: 2667 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170706/6a770804/attachment-0001.bin>