[WinEH] Avoid holding references to BlockColor (DenseMap) entries while inserting new elements

[andrew.w.kaylor]

Summary

I ran into a case where the spec2006/483 test was crashing during compilation because the WinEHPrepare pass was inserting a new element into a DenseMap while holding a reference to an existing element and the insertion caused the map to be reallocated. Near as I can tell, what WinEHPrepare was doing would have been legal with an std::map but isn't guaranteed to work with DenseMap.

[rnk]

lgtm

lib/CodeGen/WinEHPrepare.cpp

1205–1207 ↗

(On Diff #81167)

This can be shortened to something like "Insert NewBlock before looking up PHIBlock to avoid dangling references.". Anyone familiar with DenseMap should know how the iterator invalidation guarantees differ from std::map.

llvm/trunk/lib/CodeGen/WinEHPrepare.cpp

if (auto *CatchRet =
       BranchInst *Goto = cast<BranchInst>(IncomingBlock->getTerminator());
       Goto->removeFromParent();
       CatchRet->removeFromParent();
       IncomingBlock->getInstList().push_back(CatchRet);
       NewBlock->getInstList().push_back(Goto);
       Goto->setSuccessor(0, PHIBlock);
       CatchRet->setSuccessor(NewBlock);
       // Update the color mapping for the newly split edge.
+      // Grab a reference to the ColorVector to be inserted before getting the
+      // reference to the vector we are copying because inserting the new
+      // element in BlockColors might cause the map to be reallocated.
+      ColorVector &ColorsForNewBlock = BlockColors[NewBlock];
       ColorVector &ColorsForPHIBlock = BlockColors[PHIBlock];
-      BlockColors[NewBlock] = ColorsForPHIBlock;
+      ColorsForNewBlock = ColorsForPHIBlock;
       for (BasicBlock *FuncletPad : ColorsForPHIBlock)
         FuncletBlocks[FuncletPad].push_back(NewBlock);
       // Treat the new block as incoming for load insertion.
       IncomingBlock = NewBlock;
     }
     Value *&Load = Loads[IncomingBlock];
     // Insert the load into the predecessor block
     if (!Load)