[PATCH] D23015: [ThinLTO/gold] Conservatively handle unknown GVs when internalizing

[Teresa Johnson via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: mehdi_amini, twoh.
tejohnson added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

When IR linking in a module, the value symbol table may append a unique
identifier to disambiguate with an existing GV in the dest module.
This can happen when linking in the original ThinLTO backend module
to an empty dest module in the following case. If there is a referenced
alias to a referenced but preempted weak global, the IRLinker will
first create a declaration in the dest module for the weak global. Then
it will create a local copy of the global's definition, and the name
will have a unique identifier appended by the symbol table since it
conflicts with the declaration already there.

Rather than try to be too clever in this case and attempt to find and
strip off the unique id, simply return true conservatively from the
MustPreserveGV lambda.

This fixes PR28760.

https://reviews.llvm.org/D23015

Files:
  lib/Transforms/IPO/FunctionImport.cpp
  test/tools/gold/X86/Inputs/thinlto_alias2.ll
  test/tools/gold/X86/thinlto_alias2.ll

Index: test/tools/gold/X86/thinlto_alias2.ll
===================================================================
--- /dev/null
+++ test/tools/gold/X86/thinlto_alias2.ll
@@ -0,0 +1,22 @@
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto_alias2.ll -o %t2.o
+
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     -shared \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=-import-instr-limit=0 \
+; RUN:     --plugin-opt=save-temps \
+; RUN:     -o %t3.o %t.o %t2.o
+; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck %s
+
+$c1 = comdat any
+
+define weak_odr i32 @f1(i8*) unnamed_addr comdat($c1) {
+  ret i32 42
+}
+
+ at a1 = alias i32 (i8*), i32 (i8*)* @f1
+
+; CHECK: @a2 = alias i32 (i8*), i32 (i8*)* @f1.1.llvm.0
+; CHECK: declare i32 @f1(i8*) unnamed_addr
+; CHECK: define hidden i32 @f1.1.llvm.0(i8* nocapture readnone) unnamed_addr #0 comdat($c2) {
Index: test/tools/gold/X86/Inputs/thinlto_alias2.ll
===================================================================
--- /dev/null
+++ test/tools/gold/X86/Inputs/thinlto_alias2.ll
@@ -0,0 +1,8 @@
+$c2 = comdat any
+
+define weak_odr i32 @f1(i8*) unnamed_addr comdat($c2) {
+  ret i32 41
+}
+
+ at r2 = global i32(i8*)* @f1
+ at a2 = alias i32(i8*), i32(i8*)* @f1
Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -543,7 +543,14 @@
         // recorded in the index using the original name.
         // FIXME: This may not be needed once PR27866 is fixed.
         const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigName));
-        assert(GS != DefinedGlobals.end());
+        // If we don't find the original name, then conservatively return
+        // true, indicating that it must be preserved. One example where
+        // we might not find it is when the linked in local copy for an alias
+        // conflicts with the declaration created for the aliased preempted
+        // weak value, and the ValueSymbolTable appends a unique identifier
+        // to distinguish them.
+        if (GS == DefinedGlobals.end())
+          return true;
         Linkage = GS->second->linkage();
       } else {
         Linkage = GS->second->linkage();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23015.66320.patch
Type: text/x-patch
Size: 2323 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160801/187806ed/attachment.bin>