[PATCH] D31226: IR: Fix a race condition in type id clients of ModuleSummaryIndex.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc updated this revision to Diff 92582.
pcc marked an inline comment as done.
pcc added a comment.

- Use pointer


https://reviews.llvm.org/D31226

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


Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===================================================================
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1196,9 +1196,14 @@
 }
 
 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 @@
                                   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))
Index: llvm/lib/Transforms/IPO/LowerTypeTests.cpp
===================================================================
--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -265,7 +265,7 @@
   /// 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 @@
 /// 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 @@
 
 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;
Index: llvm/include/llvm/IR/ModuleSummaryIndex.h
===================================================================
--- llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -691,10 +691,21 @@
     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(),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31226.92582.patch
Type: text/x-patch
Size: 4278 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170322/385987c1/attachment.bin>