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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc created this revision.

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


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,12 @@
 }
 
 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());
+  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 +1357,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
@@ -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,7 @@
 
 LowerTypeTestsModule::TypeIdLowering
 LowerTypeTestsModule::importTypeId(StringRef TypeId) {
-  TypeTestResolution &TTRes = Summary->getTypeIdSummary(TypeId).TTRes;
+  const TypeTestResolution &TTRes = Summary->getTypeIdSummary(TypeId).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
@@ -527,6 +527,7 @@
   /// identifier.
   // FIXME: Add bitcode read/write support for this field.
   std::map<std::string, TypeIdSummary> TypeIdMap;
+  TypeIdSummary DefaultInitTypeIdSummary;
 
   /// Mapping from original ID to GUID. If original ID can map to multiple
   /// GUIDs, it will be mapped to 0.
@@ -691,10 +692,23 @@
     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 a reference to either the type id summary (if present in the
+  /// summary map) or a default initialized summary (if not present). A default
+  /// initialized summary has the correct semantics if no globals are associated
+  /// with the type id. This may should be used when importing.
+  const TypeIdSummary &getTypeIdSummary(StringRef TypeId) const {
+    auto I = TypeIdMap.find(TypeId);
+    if (I == TypeIdMap.end())
+      return DefaultInitTypeIdSummary;
+    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.92576.patch
Type: text/x-patch
Size: 4168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170322/37cd1dcb/attachment.bin>