[PATCH] D67322: [LLD][ThinLTO] Handle GUID collision in import global processing

[Joel Klinghed via Phabricator via llvm-commits]

the_jk created this revision.
the_jk added a reviewer: evgeny777.
Herald added subscribers: llvm-commits, dexonsmith, steven_wu, hiraditya, inglorion, mehdi_amini.
Herald added a project: LLVM.

If there are a GUID collision between two globals checking the
summarylist from the import index to make assumption can be dangerous.

      

Do not assume that a GlobalValue that has a GlobalVarSummary
actually is a GlobalVariable as it can be another GlobalValue with
the same GUID that the summary is connected to.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67322

Files:
  llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
  llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
  llvm/test/ThinLTO/X86/guid_collision.ll


Index: llvm/test/ThinLTO/X86/guid_collision.ll
===================================================================
--- /dev/null
+++ llvm/test/ThinLTO/X86/guid_collision.ll
@@ -0,0 +1,17 @@
+; Make sure LTO succeeds even if %t.bc contains a GlobalVariable F and
+; %t2.bc cointains a Function F with the same GUID.
+;
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/guid_collision.ll -o %t2.bc
+; RUN: llvm-lto2 run %t.bc %t2.bc -o %t.out \
+; RUN: -r=%t.bc,dummy,px -r=%t2.bc,dummy2,px
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; The source for the GUID for this symbol will be -:F
+source_filename = "-"
+ at F = internal constant i8 0
+
+; Needed to give llvm-lto2 something to do
+ at dummy = global i32 0
Index: llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
===================================================================
--- /dev/null
+++ llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
@@ -0,0 +1,12 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; The source for the GUID for this symbol will be -:F
+source_filename = "-"
+define internal fastcc i64 @F() {
+  ret i64 0
+}
+
+; Needed to give llvm-lto2 something to do
+ at dummy2 = global i32 0
+
Index: llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -239,11 +239,20 @@
   // propagateConstants hasn't been run. We can't internalize GV
   // in such case.
   if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {
-    const auto &SL = VI.getSummaryList();
-    auto *GVS = SL.empty() ? nullptr : dyn_cast<GlobalVarSummary>(SL[0].get());
-    // At this stage "maybe" is "definitely"
-    if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
-      cast<GlobalVariable>(&GV)->addAttribute("thinlto-internalize");
+    if (GlobalVariable *V = dyn_cast<GlobalVariable>(&GV)) {
+      GlobalVarSummary* GVS = nullptr;
+      for (auto &S : VI.getSummaryList()) {
+        GVS = dyn_cast<GlobalVarSummary>(S->getBaseObject());
+        if (GVS) {
+          if (GVS->modulePath() == M.getModuleIdentifier())
+            break;
+          GVS = nullptr;
+        }
+      }
+      // At this stage "maybe" is "definitely"
+      if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
+        V->addAttribute("thinlto-internalize");
+    }
   }
 
   bool DoPromote = false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67322.219252.patch
Type: text/x-patch
Size: 2595 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190907/8f5f637b/attachment.bin>