[GC, RewriteStatepointsForGC] Style cleanup and bug fix

When doing style cleanup, I noticed a minor bug in this code.  If we have a pointer that we think is unused after a statepoint and thus doesn't need relocation, we store a null pointer into the alloca we're about to promote.  This helps turn a mistake in liveness analysis into an easily debuggable crash.  It turned out this code had never been updated to handle invoke statepoints.

There's no test for this.  Without a bug in liveness, it appears impossible to make this trigger in a way which is visible in the resulting IR.  We might store the null, but when promoting the alloca, there will be no uses and thus nothing to test against.  Suggestions on how to test are very welcome.

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

diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 5ce4267c1a0523165d07cb9f64bacabe418d4240..4f1e8a9fa243fd72038c7b34fa7399e0d96ae0e7 100644 (file)
--- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1538,21 +1538,41 @@ static void relocationViaAlloca(
 #ifndef NDEBUG
     // As a debuging aid, pretend that an unrelocated pointer becomes null at
     // the gc.statepoint.  This will turn some subtle GC problems into slightly
-    // easy to debug SEGVs
+    // easier to debug SEGVs
+    SmallVector<AllocaInst *, 64> ToClobber;
     for (auto Pair : allocaMap) {
-      Value *def = Pair.first;
-      Value *alloca = Pair.second;
+      Value *Def = Pair.first;
+      AllocaInst *Alloca = cast<AllocaInst>(Pair.second);
 
       // This value was relocated
-      if (visitedLiveValues.count(def)) {
+      if (visitedLiveValues.count(Def)) {
         continue;
       }
-
-      auto PT = cast<PointerType>(def->getType());
-      Constant *CPN = ConstantPointerNull::get(PT);
-      StoreInst *store = new StoreInst(CPN, alloca);
-      store->insertBefore(info.SafepointBounds.second);
+      ToClobber.push_back(Alloca);
     }
+
+    Instruction *Statepoint = info.SafepointBounds.first;
+    auto InsertClobbersAt = [&](Instruction *IP) {
+      for (auto *AI : ToClobber) {
+        auto AIType = cast<PointerType>(AI->getType());
+        auto PT = cast<PointerType>(AIType->getElementType());
+        Constant *CPN = ConstantPointerNull::get(PT);
+        StoreInst *store = new StoreInst(CPN, AI);
+        store->insertBefore(IP);
+      }
+    };
+
+    // Insert the clobbering stores.  These may get intermixed with the
+    // gc.results and gc.relocates, but that's fine.  
+    if (auto II = dyn_cast<InvokeInst>(Statepoint)) {
+      InsertClobbersAt(II->getNormalDest()->getFirstInsertionPt());
+      InsertClobbersAt(II->getUnwindDest()->getFirstInsertionPt());
+    } else if (auto CI = dyn_cast<CallInst>(Statepoint)) {
+      BasicBlock::iterator Next(CI);
+      Next++;
+      InsertClobbersAt(Next);
+    } else
+      llvm_unreachable("illegal statepoint instruction type?");
 #endif
   }
   // update use with load allocas and add store for gc_relocated