[llvm] r298531 - IR: Fix a race condition in type id clients of ModuleSummaryIndex.

[Peter Collingbourne via llvm-commits]

Author: pcc
Date: Wed Mar 22 13:04:39 2017
New Revision: 298531

URL: http://llvm.org/viewvc/llvm-project?rev=298531&view=rev
Log:
IR: Fix a race condition in type id clients of ModuleSummaryIndex.

Add a const version of the getTypeIdSummary accessor that avoids
mutating the TypeIdMap.

Differential Revision: https://reviews.llvm.org/D31226

Modified:
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
    llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=298531&r1=298530&r2=298531&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Wed Mar 22 13:04:39 2017
@@ -691,10 +691,21 @@ public:
     return TypeIdMap;
   }
 
-  TypeIdSummary &getTypeIdSummary(StringRef TypeId) {
+  /// This accessor should only be used when exporting because it can mutate the
+  /// map.
+  TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
     return TypeIdMap[TypeId];
   }
 
+  /// This returns either a pointer to the type id summary (if present in the
+  /// summary map) or null (if not present). This may be used when importing.
+  const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
+    auto I = TypeIdMap.find(TypeId);
+    if (I == TypeIdMap.end())
+      return nullptr;
+    return &I->second;
+  }
+
   /// Remove entries in the GlobalValueMap that have empty summaries due to the
   /// eager nature of map entry creation during VST parsing. These would
   /// also be suppressed during combined index generation in mergeFrom(),

Modified: llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp?rev=298531&r1=298530&r2=298531&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp Wed Mar 22 13:04:39 2017
@@ -265,7 +265,7 @@ class LowerTypeTestsModule {
   /// regular LTO or the regular LTO phase of ThinLTO), or indirectly using type
   /// identifier summaries and external symbol references (in ThinLTO backends).
   struct TypeIdLowering {
-    TypeTestResolution::Kind TheKind;
+    TypeTestResolution::Kind TheKind = TypeTestResolution::Unsat;
 
     /// All except Unsat: the start address within the combined global.
     Constant *OffsetedGlobal;
@@ -700,7 +700,7 @@ void LowerTypeTestsModule::buildBitSetsF
 /// information about the type identifier.
 void LowerTypeTestsModule::exportTypeId(StringRef TypeId,
                                         const TypeIdLowering &TIL) {
-  TypeTestResolution &TTRes = Summary->getTypeIdSummary(TypeId).TTRes;
+  TypeTestResolution &TTRes = Summary->getOrInsertTypeIdSummary(TypeId).TTRes;
   TTRes.TheKind = TIL.TheKind;
 
   auto ExportGlobal = [&](StringRef Name, Constant *C) {
@@ -738,7 +738,10 @@ void LowerTypeTestsModule::exportTypeId(
 
 LowerTypeTestsModule::TypeIdLowering
 LowerTypeTestsModule::importTypeId(StringRef TypeId) {
-  TypeTestResolution &TTRes = Summary->getTypeIdSummary(TypeId).TTRes;
+  const TypeIdSummary *TidSummary = Summary->getTypeIdSummary(TypeId);
+  if (!TidSummary)
+    return {}; // Unsat: no globals match this type id.
+  const TypeTestResolution &TTRes = TidSummary->TTRes;
 
   TypeIdLowering TIL;
   TIL.TheKind = TTRes.TheKind;

Modified: llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp?rev=298531&r1=298530&r2=298531&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp Wed Mar 22 13:04:39 2017
@@ -1196,9 +1196,14 @@ void DevirtModule::scanTypeCheckedLoadUs
 }
 
 void DevirtModule::importResolution(VTableSlot Slot, VTableSlotInfo &SlotInfo) {
-  const WholeProgramDevirtResolution &Res =
-      Summary->getTypeIdSummary(cast<MDString>(Slot.TypeID)->getString())
-          .WPDRes[Slot.ByteOffset];
+  const TypeIdSummary *TidSummary =
+      Summary->getTypeIdSummary(cast<MDString>(Slot.TypeID)->getString());
+  if (!TidSummary)
+    return;
+  auto ResI = TidSummary->WPDRes.find(Slot.ByteOffset);
+  if (ResI == TidSummary->WPDRes.end())
+    return;
+  const WholeProgramDevirtResolution &Res = ResI->second;
 
   if (Res.TheKind == WholeProgramDevirtResolution::SingleImpl) {
     // The type of the function in the declaration is irrelevant because every
@@ -1354,10 +1359,10 @@ bool DevirtModule::run() {
                                   S.first.ByteOffset)) {
       WholeProgramDevirtResolution *Res = nullptr;
       if (Action == PassSummaryAction::Export && isa<MDString>(S.first.TypeID))
-        Res =
-            &Summary
-                 ->getTypeIdSummary(cast<MDString>(S.first.TypeID)->getString())
-                 .WPDRes[S.first.ByteOffset];
+        Res = &Summary
+                   ->getOrInsertTypeIdSummary(
+                       cast<MDString>(S.first.TypeID)->getString())
+                   .WPDRes[S.first.ByteOffset];
 
       if (!trySingleImplDevirt(TargetsForSlot, S.second, Res) &&
           tryVirtualConstProp(TargetsForSlot, S.second, Res, S.first))