[llvm] cc0b964 - [LLD][ThinLTO] Handle GUID collision in import global processing
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 25 12:38:03 PDT 2019
Author: Teresa Johnson
Date: 2019-10-25T12:36:01-07:00
New Revision: cc0b9647b76178bc3869bbfff80535ad86366472
URL: https://github.com/llvm/llvm-project/commit/cc0b9647b76178bc3869bbfff80535ad86366472
DIFF: https://github.com/llvm/llvm-project/commit/cc0b9647b76178bc3869bbfff80535ad86366472.diff
LOG: [LLD][ThinLTO] Handle GUID collision in import global processing
Summary:
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.
Patch by Joel Klinghed (the_jk at opera.com)
Reviewers: evgeny777, tejohnson
Reviewed By: tejohnson
Subscribers: tejohnson, dblaikie, MaskRay, mehdi_amini, inglorion, hiraditya, steven_wu, dexonsmith, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D67322
Added:
llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
llvm/test/ThinLTO/X86/guid_collision.ll
Modified:
llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 76b4635ad501..3109224dc2ce 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -239,11 +239,17 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
// 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)) {
+ // We can have more than one local with the same GUID, in the case of
+ // same-named locals in
diff erent but same-named source files that were
+ // compiled in their respective directories (so the source file name
+ // and resulting GUID is the same). Find the one in this module.
+ auto* GVS = dyn_cast<GlobalVarSummary>(
+ ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier()));
+ // At this stage "maybe" is "definitely"
+ if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
+ V->addAttribute("thinlto-internalize");
+ }
}
bool DoPromote = false;
diff --git a/llvm/test/ThinLTO/X86/Inputs/guid_collision.ll b/llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
new file mode 100644
index 000000000000..885761cc494a
--- /dev/null
+++ b/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
+
diff --git a/llvm/test/ThinLTO/X86/guid_collision.ll b/llvm/test/ThinLTO/X86/guid_collision.ll
new file mode 100644
index 000000000000..4d1add861b6e
--- /dev/null
+++ b/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
More information about the llvm-commits
mailing list