commit cbec0269baeaa572bcfb62fdac7dea708d94c1e9
author Reid Kleckner <rnk@google.com> Mon Apr 02 20:00:39 2018 +0000
committer Reid Kleckner <rnk@google.com> Mon Apr 02 20:00:39 2018 +0000
tree 98af7e672bbecd6da6c94975f3269620ca3f9090
parent 227d4399bf32dc87a8307af6f1c86cd5bdde5e08

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.

llvm-svn: 329007

clang/lib/AST/VTableBuilder.cpp

diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index a95791c..aab2ca7 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -2367,8 +2367,6 @@
 
 class VFTableBuilder {
 public:
-  typedef MicrosoftVTableContext::MethodVFTableLocation MethodVFTableLocation;
-
   typedef llvm::DenseMap<GlobalDecl, MethodVFTableLocation>
     MethodVFTableLocationsTy;
 
@@ -3544,10 +3542,9 @@
   }
 }
 
-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 @@
   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 @@
   computeVTableRelatedInformation(RD);
 
   assert(VFPtrLocations.count(RD) && "Couldn't find vfptr locations");
-  return VFPtrLocations[RD];
+  return *VFPtrLocations[RD];
 }
 
 const VTableLayout &
@@ -3733,7 +3730,7 @@
   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");