[PATCH] D43929: [RewriteStatepoints] Fix stale parse points

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban updated this revision to Diff 136948.
yrouban added a comment.

done the minor corrections as Anna suggested.
Could anyone with appropriate rights merge the patch?


https://reviews.llvm.org/D43929

Files:
  lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  test/Transforms/RewriteStatepointsForGC/unreachable-regression.ll


Index: test/Transforms/RewriteStatepointsForGC/unreachable-regression.ll
===================================================================
--- test/Transforms/RewriteStatepointsForGC/unreachable-regression.ll
+++ test/Transforms/RewriteStatepointsForGC/unreachable-regression.ll
@@ -0,0 +1,34 @@
+; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
+; RUN: opt -S -passes=rewrite-statepoints-for-gc < %s | FileCheck %s
+;
+; Regression test:
+;   After the rewritable callsite collection if any callsite was found
+;   in a block that was reported unreachanble by DominanceTree then
+;   removeUnreachableBlocks() was called. But it is stronger than
+;   DominatorTree::isReachableFromEntry(), i.e. removeUnreachableBlocks
+;   can remove some blocks for which isReachableFromEntry() returns true.
+;   This resulted in stale pointers to the collected but removed
+;   callsites. Such stale pointers caused crash when accessed.
+declare void @f(i8 addrspace(1)* %obj)
+
+define void @test(i8 addrspace(1)* %arg) gc "statepoint-example" {
+; CHECK-LABEL: test(
+; CHECK-NEXT: @f
+ call void @f(i8 addrspace(1)* %arg) #1
+ br i1 true, label %not_zero, label %zero
+
+not_zero:
+ ret void
+
+; This block is reachable but removed by removeUnreachableBlocks()
+zero:
+; CHECK-NOT: @f
+ call void @f(i8 addrspace(1)* %arg) #1
+ ret void
+
+unreach:
+ call void @f(i8 addrspace(1)* %arg) #1
+ ret void
+}
+
+attributes #1 = { norecurse noimplicitfloat }
Index: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===================================================================
--- lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2529,30 +2529,31 @@
     return false;
   };
 
+
+  // Delete any unreachable statepoints so that we don't have unrewritten
+  // statepoints surviving this pass.  This makes testing easier and the
+  // resulting IR less confusing to human readers.
+  DeferredDominance DD(DT);
+  bool MadeChange = removeUnreachableBlocks(F, nullptr, &DD);
+  DD.flush();
+
   // Gather all the statepoints which need rewritten.  Be careful to only
   // consider those in reachable code since we need to ask dominance queries
   // when rewriting.  We'll delete the unreachable ones in a moment.
   SmallVector<CallSite, 64> ParsePointNeeded;
-  bool HasUnreachableStatepoint = false;
   for (Instruction &I : instructions(F)) {
     // TODO: only the ones with the flag set!
     if (NeedsRewrite(I)) {
-      if (DT.isReachableFromEntry(I.getParent()))
-        ParsePointNeeded.push_back(CallSite(&I));
-      else
-        HasUnreachableStatepoint = true;
+      // NOTE removeUnreachableBlocks() is stronger than
+      // DominatorTree::isReachableFromEntry(). In other words
+      // removeUnreachableBlocks can remove some blocks for which
+      // isReachableFromEntry() returns true.
+      assert(DT.isReachableFromEntry(I.getParent()) &&
+            "no unreachable blocks expected");
+      ParsePointNeeded.push_back(CallSite(&I));
     }
   }
 
-  bool MadeChange = false;
-
-  // Delete any unreachable statepoints so that we don't have unrewritten
-  // statepoints surviving this pass.  This makes testing easier and the
-  // resulting IR less confusing to human readers.  Rather than be fancy, we
-  // just reuse a utility function which removes the unreachable blocks.
-  if (HasUnreachableStatepoint)
-    MadeChange |= removeUnreachableBlocks(F);
-
   // Return early if no work to do.
   if (ParsePointNeeded.empty())
     return MadeChange;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43929.136948.patch
Type: text/x-patch
Size: 3553 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180305/0692f5e2/attachment.bin>