From 7a92bc3e61a3f1abab38f218a8e68ed7552c4b3f Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Fri, 2 Nov 2018 23:49:21 +0000 Subject: [PATCH] [LTO] Fix a crash caused by accessing an empty ValueInfo ModuleSummaryIndex::exportToDot crashes when linking the Linux kernel under ThinLTO using LLVMgold.so. This is due to the exportToDot function trying to get the GUID of an empty ValueInfo. The root cause related to the fact that we attempt to get the GUID of an aliasee via its OriginalGUID recorded in the aliasee summary, and that is not always possible. Specifically, we cannot do this mapping when the value is internal linkage and there were other internal linkage symbols with the same name. There are 2 fixes for the problem included here. 1) In all cases where we can currently print the dot file from the command line (which is only via save-temps), we have a valid AliaseeGUID in the AliasSummary. Use that when it is available, so that we can get the correct aliasee GUID whenever possible. 2) However, if we were to invoke exportToDot from the debugger right after it is built during the initial analysis step (i.e. the per-module summary), we won't have the AliaseeGUID field populated. In that case, we have a fallback fix that will simply print "@"+GUID when we aren't able to get the GUID from the OriginalGUID. It simply checks if the VI is valid or not before attempting to get the name. Additionally, since getAliaseeGUID will assert that the AliaseeGUID is non-zero, guard the earlier fix #1 by a new function hasAliaseeGUID(). Reviewers: pcc, tmroeder Subscribers: evgeny777, mehdi_amini, inglorion, dexonsmith, arphaman, llvm-commits Differential Revision: https://reviews.llvm.org/D53986 llvm-svn: 346055 --- llvm/include/llvm/IR/ModuleSummaryIndex.h | 1 + llvm/lib/IR/ModuleSummaryIndex.cpp | 40 +++++++++++++------ .../test/ThinLTO/X86/Inputs/alias_internal.ll | 8 ++++ llvm/test/ThinLTO/X86/alias_internal.ll | 21 ++++++++++ 4 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 llvm/test/ThinLTO/X86/Inputs/alias_internal.ll create mode 100644 llvm/test/ThinLTO/X86/alias_internal.ll diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h index 778907b05eb4f..8510afe60a17c 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -408,6 +408,7 @@ class AliasSummary : public GlobalValueSummary { return const_cast( static_cast(this)->getAliasee()); } + bool hasAliaseeGUID() const { return AliaseeGUID != 0; } const GlobalValue::GUID &getAliaseeGUID() const { assert(AliaseeGUID && "Unexpected missing aliasee GUID"); return AliaseeGUID; diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp index d43684135840c..e63407c3e75c1 100644 --- a/llvm/lib/IR/ModuleSummaryIndex.cpp +++ b/llvm/lib/IR/ModuleSummaryIndex.cpp @@ -198,9 +198,12 @@ static std::string getSummaryAttributes(GlobalValueSummary* GVS) { ", ffl: " + fflagsToString(FS->fflags()); } +static std::string getNodeVisualName(GlobalValue::GUID Id) { + return std::string("@") + std::to_string(Id); +} + static std::string getNodeVisualName(const ValueInfo &VI) { - return VI.name().empty() ? std::string("@") + std::to_string(VI.getGUID()) - : VI.name().str(); + return VI.name().empty() ? getNodeVisualName(VI.getGUID()) : VI.name().str(); } static std::string getNodeLabel(const ValueInfo &VI, GlobalValueSummary *GVS) { @@ -221,13 +224,19 @@ static std::string getNodeLabel(const ValueInfo &VI, GlobalValueSummary *GVS) { // specific module associated with it. Typically this is function // or variable defined in native object or library. static void defineExternalNode(raw_ostream &OS, const char *Pfx, - const ValueInfo &VI) { - auto StrId = std::to_string(VI.getGUID()); - OS << " " << StrId << " [label=\"" << getNodeVisualName(VI) - << "\"]; // defined externally\n"; + const ValueInfo &VI, GlobalValue::GUID Id) { + auto StrId = std::to_string(Id); + OS << " " << StrId << " [label=\""; + + if (VI) { + OS << getNodeVisualName(VI); + } else { + OS << getNodeVisualName(Id); + } + OS << "\"]; // defined externally\n"; } -void ModuleSummaryIndex::exportToDot(raw_ostream& OS) const { +void ModuleSummaryIndex::exportToDot(raw_ostream &OS) const { std::vector CrossModuleEdges; DenseMap> NodeMap; StringMap ModuleToDefinedGVS; @@ -311,10 +320,17 @@ void ModuleSummaryIndex::exportToDot(raw_ostream& OS) const { Draw(SummaryIt.first, R.getGUID(), -1); if (auto *AS = dyn_cast_or_null(SummaryIt.second)) { - auto AliaseeOrigId = AS->getAliasee().getOriginalName(); - auto AliaseeId = getGUIDFromOriginalID(AliaseeOrigId); - - Draw(SummaryIt.first, AliaseeId ? AliaseeId : AliaseeOrigId, -2); + GlobalValue::GUID AliaseeId; + if (AS->hasAliaseeGUID()) + AliaseeId = AS->getAliaseeGUID(); + else { + auto AliaseeOrigId = AS->getAliasee().getOriginalName(); + AliaseeId = getGUIDFromOriginalID(AliaseeOrigId); + if (!AliaseeId) + AliaseeId = AliaseeOrigId; + } + + Draw(SummaryIt.first, AliaseeId, -2); continue; } @@ -330,7 +346,7 @@ void ModuleSummaryIndex::exportToDot(raw_ostream& OS) const { for (auto &E : CrossModuleEdges) { auto &ModList = NodeMap[E.Dst]; if (ModList.empty()) { - defineExternalNode(OS, " ", getValueInfo(E.Dst)); + defineExternalNode(OS, " ", getValueInfo(E.Dst), E.Dst); // Add fake module to the list to draw an edge to an external node // in the loop below. ModList.push_back(-1); diff --git a/llvm/test/ThinLTO/X86/Inputs/alias_internal.ll b/llvm/test/ThinLTO/X86/Inputs/alias_internal.ll new file mode 100644 index 0000000000000..e55e40b1d0526 --- /dev/null +++ b/llvm/test/ThinLTO/X86/Inputs/alias_internal.ll @@ -0,0 +1,8 @@ +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define internal i32 @f(i8*) unnamed_addr { + ret i32 42 +} + +@a2 = weak alias i32 (i8*), i32 (i8*)* @f diff --git a/llvm/test/ThinLTO/X86/alias_internal.ll b/llvm/test/ThinLTO/X86/alias_internal.ll new file mode 100644 index 0000000000000..d6433f6981daf --- /dev/null +++ b/llvm/test/ThinLTO/X86/alias_internal.ll @@ -0,0 +1,21 @@ +; Test to make sure dot dumper can correctly handle aliases to multiple +; different internal aliasees with the same name. + +; RUN: opt -module-summary %s -o %t1.bc +; RUN: opt -module-summary %p/Inputs/alias_internal.ll -o %t2.bc +; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.out -save-temps \ +; RUN: -r %t1.bc,a1,plx \ +; RUN: -r %t2.bc,a2,plx + +; RUN: cat %t.out.index.dot | FileCheck %s +; CHECK-DAG: M0_12511626713252727690 -> M0_{{.*}} // alias +; CHECK-DAG: M1_8129049334585965161 -> M1_{{.*}} // alias + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define internal i32 @f(i8*) unnamed_addr { + ret i32 42 +} + +@a1 = weak alias i32 (i8*), i32 (i8*)* @f