Merge pull request swiftlang#27138 from eeckstein/fix-stacknesting

StackNesting: fix a corner case crash related to unreachable blocks

Author: eeckstein

include/swift/SILOptimizer/Utils/StackNesting.h

@@ -155,10 +155,13 @@ class StackNesting {
     return bitNumberForAlloc(AllocInst);
   }
 
+  /// Insert deallocations at block boundaries.
+  Changes insertDeallocsAtBlockBoundaries();
+
   /// Modifies the SIL to end up with a correct stack nesting.
   ///
   /// Returns the status of what changes were made.
-  Changes adaptDeallocs();
+  bool adaptDeallocs();
 };
 
 } // end namespace swift

lib/SILOptimizer/Utils/StackNesting.cpp

@@ -24,6 +24,7 @@ void StackNesting::setup(SILFunction *F) {
 
   // We use pointers to BlockInfo structs. Therefore it's important that the
   // BlockInfos vector is never re-allocated.
+  BlockInfos.clear();
   BlockInfos.reserve(F->size());
 
   // Start with the function entry block and add blocks while walking down along
@@ -231,36 +232,26 @@ bool StackNesting::insertDeallocs(const BitVector &AliveBefore,
   return changesMade;
 }
 
-StackNesting::Changes StackNesting::adaptDeallocs() {
-
-  bool InstChanged = false;
-  bool CFGChanged = false;
-  BitVector Bits(StackLocs.size());
-
-  // Visit all blocks. Actually the order doesn't matter, but let's to it in
-  // the same order as in solve().
+// Insert deallocations at block boundaries.
+// This can be necessary for unreachable blocks. Example:
+//
+//   %1 = alloc_stack
+//   %2 = alloc_stack
+//   cond_br %c, bb2, bb3
+// bb2: <--- need to insert a dealloc_stack %2 at the begin of bb2
+//   dealloc_stack %1
+//   unreachable
+// bb3:
+//   dealloc_stack %2
+//   dealloc_stack %1
+StackNesting::Changes StackNesting::insertDeallocsAtBlockBoundaries() {
+  Changes changes = Changes::None;
   for (const BlockInfo &BI : reversed(BlockInfos)) {
-    // Collect the alive-bits (at the block exit) from the successor blocks.
-    Bits = BI.AliveStackLocsAtExit;
-
-    // Insert deallocations at block boundaries.
-    // This can be necessary for unreachable blocks. Example:
-    //
-    //   %1 = alloc_stack
-    //   %2 = alloc_stack
-    //   cond_br %c, bb2, bb3
-    // bb2:   <--- need to insert a dealloc_stack %2 at the begin of bb2
-    //   dealloc_stack %1
-    //   unreachable
-    // bb3:
-    //   dealloc_stack %2
-    //   dealloc_stack %1
-    //
     for (unsigned SuccIdx = 0, NumSuccs = BI.Successors.size();
-         SuccIdx < NumSuccs; ++ SuccIdx) {
-      BlockInfo *SuccBI = BI.Successors[SuccIdx];
+         SuccIdx < NumSuccs; ++SuccIdx) {
 
-      if (SuccBI->AliveStackLocsAtEntry == Bits)
+      BlockInfo *SuccBI = BI.Successors[SuccIdx];
+      if (SuccBI->AliveStackLocsAtEntry == BI.AliveStackLocsAtExit)
         continue;
 
       // Insert deallocations for all locations which are alive at the end of
@@ -271,11 +262,26 @@ StackNesting::Changes StackNesting::adaptDeallocs() {
         // block, we have to insert a new block where we can add the
         // deallocations.
         InsertionBlock = splitEdge(BI.Block->getTerminator(), SuccIdx);
-        CFGChanged = true;
+        changes = Changes::CFG;
+      }
+      if (insertDeallocs(BI.AliveStackLocsAtExit, SuccBI->AliveStackLocsAtEntry,
+                         &InsertionBlock->front(), None)) {
+        if (changes == Changes::None)
+          changes = Changes::Instructions;
       }
-      InstChanged |= insertDeallocs(Bits, SuccBI->AliveStackLocsAtEntry,
-                                    &InsertionBlock->front(), None);
     }
+  }
+  return changes;
+}
+
+bool StackNesting::adaptDeallocs() {
+  bool InstChanged = false;
+  BitVector Bits(StackLocs.size());
+
+  // Visit all blocks. Actually the order doesn't matter, but let's to it in
+  // the same order as in solve().
+  for (const BlockInfo &BI : reversed(BlockInfos)) {
+    Bits = BI.AliveStackLocsAtExit;
 
     // Insert/remove deallocations inside blocks.
     for (SILInstruction *StackInst : reversed(BI.StackInsts)) {
@@ -310,19 +316,29 @@ StackNesting::Changes StackNesting::adaptDeallocs() {
     }
     assert(Bits == BI.AliveStackLocsAtEntry && "dataflow didn't converge");
   }
-  if (CFGChanged)
-    return Changes::CFG;
-  if (InstChanged)
-    return Changes::Instructions;
-  return Changes::None;
+  return InstChanged;
 }
 
 StackNesting::Changes StackNesting::correctStackNesting(SILFunction *F) {
   setup(F);
-  if (solve()) {
-    return adaptDeallocs();
+  if (!solve())
+    return Changes::None;
+
+  // Insert deallocs at block boundaries. This might be necessary in CFG sub
+  // graphs which don't  reach a function exit, but only an unreachable.
+  Changes changes = insertDeallocsAtBlockBoundaries();
+  if (changes != Changes::None) {
+    // Those inserted deallocs make it necessary to re-compute the analysis.
+    setup(F);
+    solve();
+  }
+  // Do the real work: extend lifetimes by moving deallocs.
+  if (adaptDeallocs()) {
+    if (changes == Changes::None)
+      changes = Changes::Instructions;
   }
-  return Changes::None;
+
+  return changes;
 }
 
 void StackNesting::dump() const {

test/SILOptimizer/allocbox_to_stack.sil

@@ -1086,9 +1086,39 @@ bb2:
 bb3:
   strong_release %1 : ${ var Int }
   unreachable
+}
+
+
+// Verify that we don't crash on this one:
+//
+// CHECK-LABEL: sil @unreachable_with_two_predecessors
+sil @unreachable_with_two_predecessors : $@convention(thin) (Int) -> () {
+bb0(%0 : $Int):
+  cond_br undef, bb1, bb4
 
+bb1:
+  %4 = alloc_stack $Bool
+  %1 = alloc_box ${ var Int }
+  %1a = project_box %1 : ${ var Int }, 0
+  store %0 to %1a : $*Int
+  cond_br undef, bb3, bb2
+
+bb2:
+  dealloc_stack %4 : $*Bool
+  br bb4
+
+bb3:
+  %9 = load %1a : $*Int
+  strong_release %1 : ${ var Int }
+  dealloc_stack %4 : $*Bool
+  %13 = tuple ()
+  return %13 : $()
+
+bb4:
+  unreachable
 }
 
+
 // Verify the case in which a box used by a partial apply which is then used by a try_apply.
 //
 // <rdar://problem/42815224> Swift CI: 2. Swift Source Compatibility Suite (master): Kitura project fails with compiler crash