Merge pull request #7443 from gottesmm/ownership_unreachable_code

Author: swift-ci

include/swift/SIL/SILBasicBlock.h

@@ -76,9 +76,6 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
   /// This method unlinks 'self' from the containing SILFunction and deletes it.
   void eraseFromParent();
 
-  /// Returns true if this BB is the entry BB of its parent.
-  bool isEntry() const;
-
   //===--------------------------------------------------------------------===//
   // SILInstruction List Inspection and Manipulation
   //===--------------------------------------------------------------------===//
@@ -314,6 +311,17 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
     return const_cast<SILBasicBlock *>(this)->getSinglePredecessorBlock();
   }
 
+  //===--------------------------------------------------------------------===//
+  // Utility
+  //===--------------------------------------------------------------------===//
+
+  /// Returns true if this BB is the entry BB of its parent.
+  bool isEntry() const;
+
+  /// Returns true if this block ends in an unreachable or an apply of a
+  /// no-return apply or builtin.
+  bool isNoReturn() const;
+
   //===--------------------------------------------------------------------===//
   // Debugging
   //===--------------------------------------------------------------------===//

include/swift/SIL/SILInstruction.h

@@ -4889,7 +4889,7 @@ class CondBranchInst : public TermInst {
 
     auto Operands = getTrueOperands();
     return Operands.front().getOperandNumber() <= OpIndex &&
-           Operands.back().getOperandNumber() <= OpIndex;
+           OpIndex <= Operands.back().getOperandNumber();
   }
 
   /// Is \p OpIndex an operand associated with the false case?
@@ -4901,7 +4901,7 @@ class CondBranchInst : public TermInst {
 
     auto Operands = getFalseOperands();
     return Operands.front().getOperandNumber() <= OpIndex &&
-           Operands.back().getOperandNumber() <= OpIndex;
+           OpIndex <= Operands.back().getOperandNumber();
   }
 
   /// Returns the argument on the cond_br terminator that will be passed to

include/swift/SIL/SILValue.h

@@ -28,12 +28,15 @@
 namespace swift {
 
 class DominanceInfo;
+class PostOrderFunctionInfo;
+class ReversePostOrderInfo;
 class Operand;
 class SILBasicBlock;
 class SILFunction;
 class SILInstruction;
 class SILLocation;
 class SILModule;
+class TransitivelyUnreachableBlocksInfo;
 class ValueBaseUseIterator;
 class ValueUseIterator;
 
@@ -278,7 +281,8 @@ class SILValue {
   ValueOwnershipKind getOwnershipKind() const;
 
   /// Verify that this SILValue and its uses respects ownership invariants.
-  void verifyOwnership(SILModule &Mod) const;
+  void verifyOwnership(SILModule &Mod,
+                       TransitivelyUnreachableBlocksInfo *TUB = nullptr) const;
 };
 
 /// A formal SIL reference to a value, suitable for use as a stored

lib/SIL/SILBasicBlock.cpp

@@ -305,3 +305,22 @@ SILBasicBlock::getFunctionArguments() const {
   };
   return makeTransformArrayRef(getArguments(), F);
 }
+
+/// Returns true if this block ends in an unreachable or an apply of a
+/// no-return apply or builtin.
+bool SILBasicBlock::isNoReturn() const {
+  if (isa<UnreachableInst>(getTerminator()))
+    return true;
+
+  auto Iter = prev_or_begin(getTerminator()->getIterator(), begin());
+  FullApplySite FAS = FullApplySite::isa(const_cast<SILInstruction *>(&*Iter));
+  if (FAS && FAS.isCalleeNoReturn()) {
+    return true;
+  }
+
+  if (auto *BI = dyn_cast<BuiltinInst>(&*Iter)) {
+    return BI->getModule().isNoReturnBuiltinOrIntrinsic(BI->getName());
+  }
+
+  return false;
+}

lib/SIL/SILOwnershipVerifier.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "sil-ownership-verifier"
 
+#include "TransitivelyUnreachableBlocks.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/AnyFunctionRef.h"
 #include "swift/AST/Decl.h"
@@ -38,6 +39,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
+#include <algorithm>
 
 using namespace swift;
 
@@ -1043,6 +1045,10 @@ class SILValueOwnershipChecker {
   /// The module that we are in.
   SILModule &Mod;
 
+  /// A cache of unreachable function blocks that we use to determine if we can
+  /// ignore "leaks".
+  const TransitivelyUnreachableBlocksInfo &TUB;
+
   /// The value whose ownership we will check.
   SILValue Value;
 
@@ -1065,7 +1071,10 @@ class SILValueOwnershipChecker {
   llvm::SmallPtrSet<SILBasicBlock *, 8> SuccessorBlocksThatMustBeVisited;
 
 public:
-  SILValueOwnershipChecker(SILModule &M, SILValue V) : Mod(M), Value(V) {}
+  SILValueOwnershipChecker(SILModule &M,
+                           const TransitivelyUnreachableBlocksInfo &TUB,
+                           SILValue V)
+      : Mod(M), TUB(TUB), Value(V) {}
 
   ~SILValueOwnershipChecker() = default;
   SILValueOwnershipChecker(SILValueOwnershipChecker &) = delete;
@@ -1084,6 +1093,7 @@ class SILValueOwnershipChecker {
 private:
   bool checkUses();
   void checkDataflow();
+  void checkDataflowEndConditions();
   void
   gatherUsers(llvm::SmallVectorImpl<SILInstruction *> &LifetimeEndingUsers,
               llvm::SmallVectorImpl<SILInstruction *> &NonLifetimeEndingUsers);
@@ -1106,6 +1116,8 @@ class SILValueOwnershipChecker {
                                     SILBasicBlock *UserBlock);
 
   bool checkValueWithoutLifetimeEndingUses();
+
+  bool checkFunctionArgWithoutLifetimeEndingUses(SILFunctionArgument *Arg);
 };
 
 } // end anonymous namespace
@@ -1248,8 +1260,8 @@ void SILValueOwnershipChecker::uniqueNonLifetimeEndingUsers(
   }
 }
 
-static bool checkFunctionArgWithoutLifetimeEndingUses(SILFunctionArgument *Arg,
-                                                      SILModule &Mod) {
+bool SILValueOwnershipChecker::checkFunctionArgWithoutLifetimeEndingUses(
+    SILFunctionArgument *Arg) {
   switch (Arg->getOwnershipKind()) {
   case ValueOwnershipKind::Guaranteed:
   case ValueOwnershipKind::Unowned:
@@ -1262,6 +1274,9 @@ static bool checkFunctionArgWithoutLifetimeEndingUses(SILFunctionArgument *Arg,
     break;
   }
 
+  if (TUB.isUnreachable(Arg->getParent()))
+    return true;
+
   llvm::errs() << "Function: '" << Arg->getFunction()->getName() << "'\n"
                << "    Owned function parameter without life "
                   "ending uses!\n"
@@ -1274,7 +1289,7 @@ static bool checkFunctionArgWithoutLifetimeEndingUses(SILFunctionArgument *Arg,
 bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses() {
   DEBUG(llvm::dbgs() << "    No lifetime ending users?! Bailing early.\n");
   if (auto *Arg = dyn_cast<SILFunctionArgument>(Value)) {
-    if (checkFunctionArgWithoutLifetimeEndingUses(Arg, Mod)) {
+    if (checkFunctionArgWithoutLifetimeEndingUses(Arg)) {
       return true;
     }
   }
@@ -1290,6 +1305,17 @@ bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses() {
   if (Value.getOwnershipKind() == ValueOwnershipKind::Unowned)
     return true;
 
+  if (auto *ParentBlock = Value->getParentBlock()) {
+    if (TUB.isUnreachable(ParentBlock)) {
+      DEBUG(llvm::dbgs() << "    Ignoring transitively unreachable value "
+                         << "without users!\n"
+                         << "    Function: '" << Value->getFunction()->getName()
+                         << "'\n"
+                         << "    Value: " << *Value << '\n');
+      return true;
+    }
+  }
+
   if (!isValueAddressOrTrivial(Value, Mod)) {
     llvm::errs() << "Function: '" << Value->getFunction()->getName() << "'\n"
                  << "Non trivial values, non address values, and non "
@@ -1425,6 +1451,12 @@ bool SILValueOwnershipChecker::checkUses() {
       llvm_unreachable("triggering standard assertion failure routine");
     }
 
+    // If this user is in the same block as the value, do not visit
+    // predecessors. We must be extra tolerant here since we allow for
+    // unreachable code.
+    if (UserBlock == Value->getParentBlock())
+      continue;
+
     // Then for each predecessor of this block...
     for (auto *Pred : UserBlock->getPredecessorBlocks()) {
       // If this block is not a block that we have already put on the list, add
@@ -1491,15 +1523,23 @@ void SILValueOwnershipChecker::checkDataflow() {
     // users.
     BlocksWithNonLifetimeEndingUses.erase(BB);
 
-    // Ok, now we know that we do not have an overconsume. So now we need to
-    // update our state for our successors to make sure by the end of the
-    // traversal we visit them.
+    // Ok, now we know that we do not have an overconsume. If this block does
+    // not end in a no return function, we need to update our state for our
+    // successors to make sure by the end of the traversal we visit them.
+    //
+    // We must consider such no-return blocks since we may be running during
+    // SILGen before NoReturn folding has run.
     for (SILBasicBlock *SuccBlock : BB->getSuccessorBlocks()) {
       // If we already visited the successor, there is nothing to do since we
       // already visited the successor.
       if (VisitedBlocks.count(SuccBlock))
         continue;
 
+      // Then check if the successor is a transitively unreachable block. In
+      // such a case, we ignore it since we are going to leak along that path.
+      if (TUB.isUnreachable(SuccBlock))
+        continue;
+
       // Otherwise, add the successor to our SuccessorBlocksThatMustBeVisited
       // set to ensure that we assert if we do not visit it by the end of the
       // algorithm.
@@ -1530,6 +1570,7 @@ void SILValueOwnershipChecker::checkDataflow() {
       if (VisitedBlocks.count(PredBlock)) {
         continue;
       }
+
       VisitedBlocks.insert(PredBlock);
       Worklist.push_back(PredBlock);
     }
@@ -1601,8 +1642,15 @@ void SILInstruction::verifyOperandOwnership() const {
 #endif
 }
 
-void SILValue::verifyOwnership(SILModule &Mod) const {
+void SILValue::verifyOwnership(SILModule &Mod,
+                               TransitivelyUnreachableBlocksInfo *TUB) const {
 #ifndef NDEBUG
-  SILValueOwnershipChecker(Mod, *this).check();
+  if (TUB) {
+    SILValueOwnershipChecker(Mod, *TUB, *this).check();
+  } else {
+    PostOrderFunctionInfo NewPOFI((*this)->getFunction());
+    TransitivelyUnreachableBlocksInfo TUB(NewPOFI);
+    SILValueOwnershipChecker(Mod, TUB, *this).check();
+  }
 #endif
 }

lib/SIL/SILVerifier.cpp

@@ -10,31 +10,33 @@
 //
 //===----------------------------------------------------------------------===//
 
-#define DEBUG_TYPE "silverifier"
-#include "swift/SIL/SILDebugScope.h"
-#include "swift/SIL/SILFunction.h"
-#include "swift/SIL/SILModule.h"
-#include "swift/SIL/SILOpenedArchetypesTracker.h"
-#include "swift/SIL/SILVisitor.h"
-#include "swift/SIL/SILVTable.h"
-#include "swift/SIL/Dominance.h"
-#include "swift/SIL/DynamicCasts.h"
-#include "swift/AST/AnyFunctionRef.h"
+#define DEBUG_TYPE "sil-verifier"
+#include "TransitivelyUnreachableBlocks.h"
 #include "swift/AST/ASTContext.h"
+#include "swift/AST/AnyFunctionRef.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/Types.h"
+#include "swift/Basic/Range.h"
+#include "swift/ClangImporter/ClangModule.h"
+#include "swift/SIL/Dominance.h"
+#include "swift/SIL/DynamicCasts.h"
+#include "swift/SIL/PostOrder.h"
 #include "swift/SIL/PrettyStackTrace.h"
+#include "swift/SIL/SILDebugScope.h"
+#include "swift/SIL/SILFunction.h"
+#include "swift/SIL/SILModule.h"
+#include "swift/SIL/SILOpenedArchetypesTracker.h"
+#include "swift/SIL/SILVTable.h"
+#include "swift/SIL/SILVisitor.h"
 #include "swift/SIL/TypeLowering.h"
-#include "swift/ClangImporter/ClangModule.h"
-#include "swift/Basic/Range.h"
-#include "llvm/Support/Debug.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
 using namespace swift;
 
 using Lowering::AbstractionPattern;
@@ -108,6 +110,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   SILOpenedArchetypesTracker OpenedArchetypes;
   const SILInstruction *CurInstruction = nullptr;
   DominanceInfo *Dominance = nullptr;
+  llvm::Optional<PostOrderFunctionInfo> PostOrderInfo;
+  llvm::Optional<TransitivelyUnreachableBlocksInfo> UnreachableBlockInfo;
   bool SingleFunction = true;
 
   SILVerifier(const SILVerifier&) = delete;
@@ -413,6 +417,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       : M(F.getModule().getSwiftModule()), F(F),
         fnConv(F.getLoweredFunctionType(), F.getModule()),
         TC(F.getModule().Types), OpenedArchetypes(F), Dominance(nullptr),
+        PostOrderInfo(), UnreachableBlockInfo(),
         SingleFunction(SingleFunction) {
     if (F.isExternalDeclaration())
       return;
@@ -425,7 +430,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "Basic blocks must end with a terminator instruction");
     }
 
-    Dominance = new DominanceInfo(const_cast<SILFunction*>(&F));
+    Dominance = new DominanceInfo(const_cast<SILFunction *>(&F));
+    if (isSILOwnershipEnabled()) {
+      PostOrderInfo.emplace(const_cast<SILFunction *>(&F));
+      UnreachableBlockInfo.emplace(PostOrderInfo.getValue());
+    }
 
     auto *DebugScope = F.getDebugScope();
     require(DebugScope, "All SIL functions must have a debug scope");
@@ -476,7 +485,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     // ownership.
     if (!F->hasQualifiedOwnership())
       return;
-    SILValue(V).verifyOwnership(F->getModule());
+    SILValue(V).verifyOwnership(F->getModule(),
+                                &UnreachableBlockInfo.getValue());
   }
 
   void checkSILInstruction(SILInstruction *I) {