[PATCH] D140837: [AAPointerInfo] fix assertion at the pass-through use of a pointer

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds updated this revision to Diff 485870.
sameerds added a comment.

Actually fix the root cause. The previous change was cleaned up too hastily and completely missed the point of the fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140837/new/

https://reviews.llvm.org/D140837

Files:
  llvm/lib/Transforms/IPO/AttributorAttributes.cpp


Index: llvm/lib/Transforms/IPO/AttributorAttributes.cpp
===================================================================
--- llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -1434,11 +1434,20 @@
   DenseMap<Value *, OffsetInfo> OffsetInfoMap;
   OffsetInfoMap[&AssociatedValue].insert(0);
 
-  auto HandlePassthroughUser = [&](Value *Usr, const OffsetInfo &PtrOI,
-                                   bool &Follow) {
+  auto HandlePassthroughUser = [&](Value *Usr, Value *CurPtr, bool &Follow) {
+    // One does not simply walk into a map and assign a reference to a possibly
+    // new location. That can cause an invalidation before the assignment
+    // happens, like so:
+    //
+    //   OffsetInfoMap[Usr] = OffsetInfoMap[CurPtr]; /* bad idea! */
+    //
+    // The RHS is a reference that may be invalidated by an insertion caused by
+    // the LHS. So we ensure that the side-effect of the LHS happens first.
+    auto &UsrOI = OffsetInfoMap[Usr];
+    auto &PtrOI = OffsetInfoMap[CurPtr];
     assert(!PtrOI.isUnassigned() &&
            "Cannot pass through if the input Ptr was not visited!");
-    OffsetInfoMap[Usr] = PtrOI;
+    UsrOI = PtrOI;
     Follow = true;
     return true;
   };
@@ -1460,7 +1469,7 @@
 
     if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Usr)) {
       if (CE->isCast())
-        return HandlePassthroughUser(Usr, OffsetInfoMap[CurPtr], Follow);
+        return HandlePassthroughUser(Usr, CurPtr, Follow);
       if (CE->isCompare())
         return true;
       if (!isa<GEPOperator>(CE)) {
@@ -1490,7 +1499,7 @@
     if (isa<PtrToIntInst>(Usr))
       return false;
     if (isa<CastInst>(Usr) || isa<SelectInst>(Usr) || isa<ReturnInst>(Usr))
-      return HandlePassthroughUser(Usr, OffsetInfoMap[CurPtr], Follow);
+      return HandlePassthroughUser(Usr, CurPtr, Follow);
 
     // For PHIs we need to take care of the recurrence explicitly as the value
     // might change while we iterate through a loop. For now, we give up if
@@ -1558,7 +1567,7 @@
         if (IsFirstPHIUser || BaseOI == UsrOI) {
           LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI is invariant " << *CurPtr
                             << " in " << *Usr << "\n");
-          return HandlePassthroughUser(Usr, PtrOI, Follow);
+          return HandlePassthroughUser(Usr, CurPtr, Follow);
         }
 
         LLVM_DEBUG(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140837.485870.patch
Type: text/x-patch
Size: 2402 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230102/eec819bc/attachment.bin>