IR: Fix a race condition in type id clients of ModuleSummaryIndex.

[pcc]

Summary

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

[mehdi_amini]

llvm/include/llvm/IR/ModuleSummaryIndex.h

530

Document.

Also, it seems unusual to do this, I don't think I saw this in the past.

711

Can we use "find()" instead of "get()" and return a pointer?

[pcc]

• Use pointer

llvm/include/llvm/IR/ModuleSummaryIndex.h

530

Removed

[mehdi_amini]

LGTM.

llvm/include/llvm/IR/ModuleSummaryIndex.h

private:
 
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
 
   /// Mapping from type identifiers to summary information for that type
   /// 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.
   std::map<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
 
   // YAML I/O support.
   friend yaml::MappingTraits<ModuleSummaryIndex>;
 
public:
   bool hasExportedFunctions(const Module &M) const {
     return ModulePathStringTable.count(M.getModuleIdentifier());
   }
 
   const std::map<std::string, TypeIdSummary> &typeIds() const {
     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(),
   /// but if there was only one module or this was the first module we might
   /// not invoke mergeFrom.
   void removeEmptySummaryEntries();
 
   /// Collect for the given module the list of function it defines

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

 
 /// Export the given type identifier so that ThinLTO backends may import it.
 /// Type identifiers are exported by adding coarse-grained information about how
 /// to test the type identifier to the summary, and creating symbols in the
 /// object file (aliases and absolute symbols) containing fine-grained
 /// 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) {
     GlobalAlias *GA =
         GlobalAlias::create(Int8Ty, 0, GlobalValue::ExternalLinkage,
                             "__typeid_" + TypeId + "_" + Name, C, &M);
     GA->setVisibility(GlobalValue::HiddenVisibility);
   };
void LowerTypeTestsModule::exportTypeId(StringRef TypeId,
 
   if (TIL.TheKind == TypeTestResolution::Inline)
     ExportGlobal("inline_bits",
                  ConstantExpr::getIntToPtr(TIL.InlineBits, Int8PtrTy));
 }
 
 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;
 
   auto ImportGlobal = [&](StringRef Name, unsigned AbsWidth) {
     Constant *C =
         M.getOrInsertGlobal(("__typeid_" + TypeId + "_" + Name).str(), Int8Ty);
     auto *GV = dyn_cast<GlobalVariable>(C);

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

for (DevirtCallSite Call : DevirtCalls) {
                                                    &NumUnsafeUses);
     }
 
     CI->eraseFromParent();
   }
 }
 
 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
     // call site will cast it to the correct type.
     auto *SingleImpl = M.getOrInsertFunction(
         Res.SingleImplName, Type::getVoidTy(M.getContext()), nullptr);
 
     // This is the import phase so we should not be exporting anything.
for (auto &S : CallSlots) {
     // Search each of the members of the type identifier for the virtual
     // function implementation at offset S.first.ByteOffset, and add to
     // TargetsForSlot.
     std::vector<VirtualCallTarget> TargetsForSlot;
     if (tryFindVirtualCallTargets(TargetsForSlot, TypeIdMap[S.first.TypeID],
                                   S.first.ByteOffset)) {
       WholeProgramDevirtResolution *Res = nullptr;
       if (Action == PassSummaryAction::Export && isa<MDString>(S.first.TypeID))
-        Res =
-            &Summary
-                 ->getTypeIdSummary(cast<MDString>(S.first.TypeID)->getString())
+        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))
         DidVirtualConstProp = true;
 
       // Collect functions devirtualized at least for one call site for stats.
       if (RemarksEnabled)
         for (const auto &T : TargetsForSlot)