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 ↗

(On Diff #92576)

Document.

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

711 ↗

(On Diff #92576)

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

[pcc]

• Use pointer

llvm/include/llvm/IR/ModuleSummaryIndex.h

530 ↗

(On Diff #92576)

Removed

[mehdi_amini]

LGTM.

llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h

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 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(),
   /// 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/trunk/lib/Transforms/IPO/LowerTypeTests.cpp

class LowerTypeTestsModule {
   };
   DenseMap<Metadata *, TypeIdUserInfo> TypeIdUsers;
 
   /// This structure describes how to lower type tests for a particular type
   /// identifier. It is either built directly from the global analysis (during
   /// 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;
 
     /// ByteArray, Inline, AllOnes: log2 of the required global alignment
     /// relative to the start address.
     Constant *AlignLog2;
 

 
 /// 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 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;
 
   auto ImportGlobal = [&](StringRef Name, unsigned AbsWidth) {
     Constant *C =
         M.getOrInsertGlobal(("__typeid_" + TypeId + "_" + Name).str(), Int8Ty);
     auto *GV = dyn_cast<GlobalVariable>(C);

llvm/trunk/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());
+  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
     // 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)