From 1b5b740e233c7bb162d43317cfd5e0c722db0152 Mon Sep 17 00:00:00 2001
From: Davide Italiano <davide@freebsd.org>
Date: Fri, 21 Apr 2017 04:25:00 +0000
Subject: [PATCH] [PartialInliner] Fix crash when inlining functions with
 unreachable blocks.

CodeExtractor looks up the dominator node corresponding to return blocks
when splitting them. If one of these blocks is unreachable, there's no
node in the Dom and CodeExtractor crashes because it doesn't check
for domtree node validity.
In theory, we could add just a check for skipping null DTNodes in
`splitReturnBlock` but the fix I propose here is slightly different. To the
best of my knowledge, unreachable blocks are irrelevant for the algorithm,
therefore we can just skip them when building the candidate set in the
constructor.

Differential Revision:  https://reviews.llvm.org/D32335

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@300946 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Utils/CodeExtractor.cpp        | 25 ++++++------
 .../CodeExtractor/unreachable-block.ll        | 38 +++++++++++++++++++
 2 files changed, 51 insertions(+), 12 deletions(-)
 create mode 100644 test/Transforms/CodeExtractor/unreachable-block.ll

diff --git a/lib/Transforms/Utils/CodeExtractor.cpp b/lib/Transforms/Utils/CodeExtractor.cpp
index 92b6c0b84db..5a1b8926072 100644
--- a/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/lib/Transforms/Utils/CodeExtractor.cpp
@@ -74,24 +74,25 @@ bool CodeExtractor::isBlockValidForExtraction(const BasicBlock &BB) {
 
 /// \brief Build a set of blocks to extract if the input blocks are viable.
 static SetVector<BasicBlock *>
-buildExtractionBlockSet(ArrayRef<BasicBlock *> BBs) {
-  auto BBBegin = BBs.begin();
-  auto BBEnd = BBs.end();
-  assert(BBBegin != BBEnd && "The set of blocks to extract must be non-empty");
-
+buildExtractionBlockSet(ArrayRef<BasicBlock *> BBs, DominatorTree *DT) {
+  assert(!BBs.empty() && "The set of blocks to extract must be non-empty");
   SetVector<BasicBlock *> Result;
 
   // Loop over the blocks, adding them to our set-vector, and aborting with an
   // empty set if we encounter invalid blocks.
-  do {
-    if (!Result.insert(*BBBegin))
-      llvm_unreachable("Repeated basic blocks in extraction input");
+  for (BasicBlock *BB : BBs) {
+
+    // If this block is dead, don't process it.
+    if (DT && !DT->isReachableFromEntry(BB))
+      continue;
 
-    if (!CodeExtractor::isBlockValidForExtraction(**BBBegin)) {
+    if (!Result.insert(BB))
+      llvm_unreachable("Repeated basic blocks in extraction input");
+    if (!CodeExtractor::isBlockValidForExtraction(*BB)) {
       Result.clear();
       return Result;
     }
-  } while (++BBBegin != BBEnd);
+  }
 
 #ifndef NDEBUG
   for (SetVector<BasicBlock *>::iterator I = std::next(Result.begin()),
@@ -111,13 +112,13 @@ CodeExtractor::CodeExtractor(ArrayRef<BasicBlock *> BBs, DominatorTree *DT,
                              bool AggregateArgs, BlockFrequencyInfo *BFI,
                              BranchProbabilityInfo *BPI)
     : DT(DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-      BPI(BPI), Blocks(buildExtractionBlockSet(BBs)), NumExitBlocks(~0U) {}
+      BPI(BPI), Blocks(buildExtractionBlockSet(BBs, DT)), NumExitBlocks(~0U) {}
 
 CodeExtractor::CodeExtractor(DominatorTree &DT, Loop &L, bool AggregateArgs,
                              BlockFrequencyInfo *BFI,
                              BranchProbabilityInfo *BPI)
     : DT(&DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-      BPI(BPI), Blocks(buildExtractionBlockSet(L.getBlocks())),
+      BPI(BPI), Blocks(buildExtractionBlockSet(L.getBlocks(), &DT)),
       NumExitBlocks(~0U) {}
 
 /// definedInRegion - Return true if the specified value is defined in the
diff --git a/test/Transforms/CodeExtractor/unreachable-block.ll b/test/Transforms/CodeExtractor/unreachable-block.ll
new file mode 100644
index 00000000000..d20a35718e6
--- /dev/null
+++ b/test/Transforms/CodeExtractor/unreachable-block.ll
@@ -0,0 +1,38 @@
+; RUN: opt -S -partial-inliner %s | FileCheck %s
+
+; CHECK-LABEL: define void @dipsy(
+; CHECK-NEXT:   call void @tinkywinky.1_ontrue()
+; CHECK-NEXT:   call void @patatuccio()
+; CHECK-NEXT:   ret void
+; CHECK-NEXT: }
+
+; CHECK-LABEL: define internal void @tinkywinky.1_ontrue() {
+; CHECK-NEXT: newFuncRoot:
+; CHECK-NEXT:   br label %ontrue
+; CHECK: .exitStub:
+; CHECK-NEXT:   ret void
+; CHECK: ontrue:
+; CHECK-NEXT:   call void @patatino()
+; CHECK-NEXT:   br label %onfalse
+; CHECK: onfalse:
+; CHECK-NEXT:   br label %.exitStub
+; CHECK-NEXT: }
+
+declare void @patatino()
+declare void @patatuccio()
+
+define fastcc void @tinkywinky() {
+  br i1 true, label %ontrue, label %onfalse
+ontrue:
+  call void @patatino()
+  br label %onfalse
+onfalse:
+  call void @patatuccio()
+  ret void
+cantreachme:
+  ret void
+}
+define void @dipsy() {
+  call fastcc void @tinkywinky()
+  ret void
+}
-- 
2.40.0