Fix some DenseMap use-after-rehash bugs and hoist MethodVFTableLocation

This re-lands r328845 with fixes for crbug.com/827810.

The initial motiviation was to hoist MethodVFTableLocation to global
scope so it could be forward declared.

In this patch, I noticed that MicrosoftVTableContext uses some risky
patterns. It has methods that return references to data stored in
DenseMaps. I've made some of them return by value for trivial structs
and I've moved some things into separate allocations.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@329007 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Mangle.h b/include/clang/AST/Mangle.h
index 7a45d88c23c8fb7112b59bf6ee19345e314fc886..71c78dcfabdd13627ec82c2f2cf72c67700a5b54 100644 (file)
--- a/include/clang/AST/Mangle.h
+++ b/include/clang/AST/Mangle.h
@@ -30,6 +30,7 @@ namespace clang {
   class CXXDestructorDecl;
   class CXXMethodDecl;
   class FunctionDecl;
+  struct MethodVFTableLocation;
   class NamedDecl;
   class ObjCMethodDecl;
   class StringLiteral;
@@ -201,7 +202,8 @@ public:
                                                    raw_ostream &Out) = 0;
 
   virtual void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
-                                        raw_ostream &) = 0;
+                                        const MethodVFTableLocation &ML,
+                                        raw_ostream &Out) = 0;
 
   virtual void mangleCXXVirtualDisplacementMap(const CXXRecordDecl *SrcRD,
                                                const CXXRecordDecl *DstRD,

diff --git a/include/clang/AST/VTableBuilder.h b/include/clang/AST/VTableBuilder.h
index b0b71e473516dc7499b56aaf54de5fd42314a2d8..8ac4ffa9718d4da1227fa4cb71c1269b345382fa 100644 (file)
--- a/include/clang/AST/VTableBuilder.h
+++ b/include/clang/AST/VTableBuilder.h
@@ -479,41 +479,42 @@ struct VirtualBaseInfo {
   VPtrInfoVector VBPtrPaths;
 };
 
+struct MethodVFTableLocation {
+  /// If nonzero, holds the vbtable index of the virtual base with the vfptr.
+  uint64_t VBTableIndex;
+
+  /// If nonnull, holds the last vbase which contains the vfptr that the
+  /// method definition is adjusted to.
+  const CXXRecordDecl *VBase;
+
+  /// This is the offset of the vfptr from the start of the last vbase, or the
+  /// complete type if there are no virtual bases.
+  CharUnits VFPtrOffset;
+
+  /// Method's index in the vftable.
+  uint64_t Index;
+
+  MethodVFTableLocation()
+      : VBTableIndex(0), VBase(nullptr), VFPtrOffset(CharUnits::Zero()),
+        Index(0) {}
+
+  MethodVFTableLocation(uint64_t VBTableIndex, const CXXRecordDecl *VBase,
+                        CharUnits VFPtrOffset, uint64_t Index)
+      : VBTableIndex(VBTableIndex), VBase(VBase), VFPtrOffset(VFPtrOffset),
+        Index(Index) {}
+
+  bool operator<(const MethodVFTableLocation &other) const {
+    if (VBTableIndex != other.VBTableIndex) {
+      assert(VBase != other.VBase);
+      return VBTableIndex < other.VBTableIndex;
+    }
+    return std::tie(VFPtrOffset, Index) <
+           std::tie(other.VFPtrOffset, other.Index);
+  }
+};
+
 class MicrosoftVTableContext : public VTableContextBase {
 public:
-  struct MethodVFTableLocation {
-    /// If nonzero, holds the vbtable index of the virtual base with the vfptr.
-    uint64_t VBTableIndex;
-
-    /// If nonnull, holds the last vbase which contains the vfptr that the
-    /// method definition is adjusted to.
-    const CXXRecordDecl *VBase;
-
-    /// This is the offset of the vfptr from the start of the last vbase, or the
-    /// complete type if there are no virtual bases.
-    CharUnits VFPtrOffset;
-
-    /// Method's index in the vftable.
-    uint64_t Index;
-
-    MethodVFTableLocation()
-        : VBTableIndex(0), VBase(nullptr), VFPtrOffset(CharUnits::Zero()),
-          Index(0) {}
-
-    MethodVFTableLocation(uint64_t VBTableIndex, const CXXRecordDecl *VBase,
-                          CharUnits VFPtrOffset, uint64_t Index)
-        : VBTableIndex(VBTableIndex), VBase(VBase),
-          VFPtrOffset(VFPtrOffset), Index(Index) {}
-
-    bool operator<(const MethodVFTableLocation &other) const {
-      if (VBTableIndex != other.VBTableIndex) {
-        assert(VBase != other.VBase);
-        return VBTableIndex < other.VBTableIndex;
-      }
-      return std::tie(VFPtrOffset, Index) <
-             std::tie(other.VFPtrOffset, other.Index);
-    }
-  };
 
 private:
   ASTContext &Context;
@@ -522,7 +523,7 @@ private:
     MethodVFTableLocationsTy;
   MethodVFTableLocationsTy MethodVFTableLocations;
 
-  typedef llvm::DenseMap<const CXXRecordDecl *, VPtrInfoVector>
+  typedef llvm::DenseMap<const CXXRecordDecl *, std::unique_ptr<VPtrInfoVector>>
       VFPtrLocationsMapTy;
   VFPtrLocationsMapTy VFPtrLocations;
 
@@ -559,7 +560,7 @@ public:
   const VTableLayout &getVFTableLayout(const CXXRecordDecl *RD,
                                        CharUnits VFPtrOffset);
 
-  const MethodVFTableLocation &getMethodVFTableLocation(GlobalDecl GD);
+  MethodVFTableLocation getMethodVFTableLocation(GlobalDecl GD);
 
   const ThunkInfoVectorTy *getThunkInfo(GlobalDecl GD) override {
     // Complete destructors don't have a slot in a vftable, so no thunks needed.

diff --git a/lib/AST/MicrosoftMangle.cpp b/lib/AST/MicrosoftMangle.cpp
index 785ee3a26e3097208d1456e1706fe664f8848764..9d49e0f1799f133b3c380f1c5a5a5e8f3537920c 100644 (file)
--- a/lib/AST/MicrosoftMangle.cpp
+++ b/lib/AST/MicrosoftMangle.cpp
@@ -135,7 +135,8 @@ public:
   bool shouldMangleStringLiteral(const StringLiteral *SL) override;
   void mangleCXXName(const NamedDecl *D, raw_ostream &Out) override;
   void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
-                                raw_ostream &) override;
+                                const MethodVFTableLocation &ML,
+                                raw_ostream &Out) override;
   void mangleThunk(const CXXMethodDecl *MD, const ThunkInfo &Thunk,
                    raw_ostream &) override;
   void mangleCXXDtorThunk(const CXXDestructorDecl *DD, CXXDtorType Type,
@@ -297,9 +298,8 @@ public:
   void mangleMemberDataPointer(const CXXRecordDecl *RD, const ValueDecl *VD);
   void mangleMemberFunctionPointer(const CXXRecordDecl *RD,
                                    const CXXMethodDecl *MD);
-  void mangleVirtualMemPtrThunk(
-      const CXXMethodDecl *MD,
-      const MicrosoftVTableContext::MethodVFTableLocation &ML);
+  void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
+                                const MethodVFTableLocation &ML);
   void mangleNumber(int64_t Number);
   void mangleTagTypeKind(TagTypeKind TK);
   void mangleArtificalTagType(TagTypeKind TK, StringRef UnqualifiedName,
@@ -607,7 +607,7 @@ MicrosoftCXXNameMangler::mangleMemberFunctionPointer(const CXXRecordDecl *RD,
     if (MD->isVirtual()) {
       MicrosoftVTableContext *VTContext =
           cast<MicrosoftVTableContext>(getASTContext().getVTableContext());
-      const MicrosoftVTableContext::MethodVFTableLocation &ML =
+      MethodVFTableLocation ML =
           VTContext->getMethodVFTableLocation(GlobalDecl(MD));
       mangleVirtualMemPtrThunk(MD, ML);
       NVOffset = ML.VFPtrOffset.getQuantity();
@@ -644,8 +644,7 @@ MicrosoftCXXNameMangler::mangleMemberFunctionPointer(const CXXRecordDecl *RD,
 }
 
 void MicrosoftCXXNameMangler::mangleVirtualMemPtrThunk(
-    const CXXMethodDecl *MD,
-    const MicrosoftVTableContext::MethodVFTableLocation &ML) {
+    const CXXMethodDecl *MD, const MethodVFTableLocation &ML) {
   // Get the vftable offset.
   CharUnits PointerWidth = getASTContext().toCharUnitsFromBits(
       getASTContext().getTargetInfo().getPointerWidth(0));
@@ -2775,14 +2774,9 @@ static void mangleThunkThisAdjustment(const CXXMethodDecl *MD,
   }
 }
 
-void
-MicrosoftMangleContextImpl::mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
-                                                     raw_ostream &Out) {
-  MicrosoftVTableContext *VTContext =
-      cast<MicrosoftVTableContext>(getASTContext().getVTableContext());
-  const MicrosoftVTableContext::MethodVFTableLocation &ML =
-      VTContext->getMethodVFTableLocation(GlobalDecl(MD));
-
+void MicrosoftMangleContextImpl::mangleVirtualMemPtrThunk(
+    const CXXMethodDecl *MD, const MethodVFTableLocation &ML,
+    raw_ostream &Out) {
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO);
   Mangler.getStream() << '?';

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index a95791ce884f86a06d1c2de9d517a437819e9250..aab2ca7aecbe073a441a44730e1a883f389a0870 100644 (file)
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -2367,8 +2367,6 @@ namespace {
 
 class VFTableBuilder {
 public:
-  typedef MicrosoftVTableContext::MethodVFTableLocation MethodVFTableLocation;
-
   typedef llvm::DenseMap<GlobalDecl, MethodVFTableLocation>
     MethodVFTableLocationsTy;
 
@@ -3544,10 +3542,9 @@ static void computeFullPathsForVFTables(ASTContext &Context,
   }
 }
 
-static bool
-vfptrIsEarlierInMDC(const ASTRecordLayout &Layout,
-                    const MicrosoftVTableContext::MethodVFTableLocation &LHS,
-                    const MicrosoftVTableContext::MethodVFTableLocation &RHS) {
+static bool vfptrIsEarlierInMDC(const ASTRecordLayout &Layout,
+                                const MethodVFTableLocation &LHS,
+                                const MethodVFTableLocation &RHS) {
   CharUnits L = LHS.VFPtrOffset;
   CharUnits R = RHS.VFPtrOffset;
   if (LHS.VBase)
@@ -3568,14 +3565,14 @@ void MicrosoftVTableContext::computeVTableRelatedInformation(
   const VTableLayout::AddressPointsMapTy EmptyAddressPointsMap;
 
   {
-    VPtrInfoVector VFPtrs;
-    computeVTablePaths(/*ForVBTables=*/false, RD, VFPtrs);
-    computeFullPathsForVFTables(Context, RD, VFPtrs);
+    auto VFPtrs = llvm::make_unique<VPtrInfoVector>();
+    computeVTablePaths(/*ForVBTables=*/false, RD, *VFPtrs);
+    computeFullPathsForVFTables(Context, RD, *VFPtrs);
     VFPtrLocations[RD] = std::move(VFPtrs);
   }
 
   MethodVFTableLocationsTy NewMethodLocations;
-  for (const std::unique_ptr<VPtrInfo> &VFPtr : VFPtrLocations[RD]) {
+  for (const std::unique_ptr<VPtrInfo> &VFPtr : *VFPtrLocations[RD]) {
     VFTableBuilder Builder(*this, RD, *VFPtr);
 
     VFTableIdTy id(RD, VFPtr->FullOffsetInMDC);
@@ -3720,7 +3717,7 @@ MicrosoftVTableContext::getVFPtrOffsets(const CXXRecordDecl *RD) {
   computeVTableRelatedInformation(RD);
 
   assert(VFPtrLocations.count(RD) && "Couldn't find vfptr locations");
-  return VFPtrLocations[RD];
+  return *VFPtrLocations[RD];
 }
 
 const VTableLayout &
@@ -3733,7 +3730,7 @@ MicrosoftVTableContext::getVFTableLayout(const CXXRecordDecl *RD,
   return *VFTableLayouts[id];
 }
 
-const MicrosoftVTableContext::MethodVFTableLocation &
+MethodVFTableLocation
 MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
   assert(cast<CXXMethodDecl>(GD.getDecl())->isVirtual() &&
          "Only use this method for virtual methods or dtors");

diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp
index 42c0210e1df4547c8919cf75c8429ec5d8fc654f..b6d336a2e8be2fd0ca59f49c5f44b47d73842c31 100644 (file)
--- a/lib/CodeGen/CGDebugInfo.cpp
+++ b/lib/CodeGen/CGDebugInfo.cpp
@@ -1416,7 +1416,7 @@ llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
       // deleting dtor.
       const auto *DD = dyn_cast<CXXDestructorDecl>(Method);
       GlobalDecl GD = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl(Method);
-      MicrosoftVTableContext::MethodVFTableLocation ML =
+      MethodVFTableLocation ML =
           CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
       VIndex = ML.Index;
 

diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp
index 542e1bf5fa37db39675e8d913e13ac1bcac3fb43..d5b168d0691b4f28c645ae306d9926f8fe0e0e36 100644 (file)
--- a/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -230,7 +230,7 @@ public:
   getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override {
     MD = MD->getCanonicalDecl();
     if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD)) {
-      MicrosoftVTableContext::MethodVFTableLocation ML =
+      MethodVFTableLocation ML =
           CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
       // The vbases might be ordered differently in the final overrider object
       // and the complete object, so the "this" argument may sometimes point to
@@ -616,9 +616,8 @@ private:
   const VBTableGlobals &enumerateVBTables(const CXXRecordDecl *RD);
 
   /// \brief Generate a thunk for calling a virtual member function MD.
-  llvm::Function *EmitVirtualMemPtrThunk(
-      const CXXMethodDecl *MD,
-      const MicrosoftVTableContext::MethodVFTableLocation &ML);
+  llvm::Function *EmitVirtualMemPtrThunk(const CXXMethodDecl *MD,
+                                         const MethodVFTableLocation &ML);
 
 public:
   llvm::Type *ConvertMemberPointerType(const MemberPointerType *MPT) override;
@@ -1336,7 +1335,7 @@ MicrosoftCXXABI::getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD) {
     LookupGD = GlobalDecl(DD, Dtor_Deleting);
   }
 
-  MicrosoftVTableContext::MethodVFTableLocation ML =
+  MethodVFTableLocation ML =
       CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
   CharUnits Adjustment = ML.VFPtrOffset;
 
@@ -1385,7 +1384,7 @@ Address MicrosoftCXXABI::adjustThisArgumentForVirtualFunctionCall(
     // with the base one, so look up the deleting one instead.
     LookupGD = GlobalDecl(DD, Dtor_Deleting);
   }
-  MicrosoftVTableContext::MethodVFTableLocation ML =
+  MethodVFTableLocation ML =
       CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
 
   CharUnits StaticOffset = ML.VFPtrOffset;
@@ -1851,8 +1850,7 @@ CGCallee MicrosoftCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
   llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty, MethodDecl->getParent());
 
   MicrosoftVTableContext &VFTContext = CGM.getMicrosoftVTableContext();
-  MicrosoftVTableContext::MethodVFTableLocation ML =
-      VFTContext.getMethodVFTableLocation(GD);
+  MethodVFTableLocation ML = VFTContext.getMethodVFTableLocation(GD);
 
   // Compute the identity of the most derived class whose virtual table is
   // located at the MethodVFTableLocation ML.
@@ -1937,16 +1935,16 @@ MicrosoftCXXABI::enumerateVBTables(const CXXRecordDecl *RD) {
   return VBGlobals;
 }
 
-llvm::Function *MicrosoftCXXABI::EmitVirtualMemPtrThunk(
-    const CXXMethodDecl *MD,
-    const MicrosoftVTableContext::MethodVFTableLocation &ML) {
+llvm::Function *
+MicrosoftCXXABI::EmitVirtualMemPtrThunk(const CXXMethodDecl *MD,
+                                        const MethodVFTableLocation &ML) {
   assert(!isa<CXXConstructorDecl>(MD) && !isa<CXXDestructorDecl>(MD) &&
          "can't form pointers to ctors or virtual dtors");
 
   // Calculate the mangled name.
   SmallString<256> ThunkName;
   llvm::raw_svector_ostream Out(ThunkName);
-  getMangleContext().mangleVirtualMemPtrThunk(MD, Out);
+  getMangleContext().mangleVirtualMemPtrThunk(MD, ML, Out);
 
   // If the thunk has been generated previously, just return it.
   if (llvm::GlobalValue *GV = CGM.getModule().getNamedValue(ThunkName))
@@ -2760,8 +2758,7 @@ MicrosoftCXXABI::EmitMemberFunctionPointer(const CXXMethodDecl *MD) {
     FirstField = CGM.GetAddrOfFunction(MD, Ty);
   } else {
     auto &VTableContext = CGM.getMicrosoftVTableContext();
-    MicrosoftVTableContext::MethodVFTableLocation ML =
-        VTableContext.getMethodVFTableLocation(MD);
+    MethodVFTableLocation ML = VTableContext.getMethodVFTableLocation(MD);
     FirstField = EmitVirtualMemPtrThunk(MD, ML);
     // Include the vfptr adjustment if the method is in a non-primary vftable.
     NonVirtualBaseAdjustment += ML.VFPtrOffset;