[ownership] Add new pass OwnershipVerifierTextualErrorDumper and use it in ownership verifier FileCheck tests instead of SILVerifier.

This will make it easier for me with a few further refactors to make the
ownership verifier testing mode emit per function error numbers instead of the
global error number that it is emitting now.

The reason why this is necessary is that today, the verification by
-sil-verify-all causes the errors to be emitted. That verification is done on a
per value level, rather than a per function level, so it is hard to get per
function error numbers without doing unprincipled things like propagating around
state saying what the current function being verified is.

This pass instead will let me make the error counter be per ErrorBuilder which
are created per function.

One thing to be aware of is that this /will/ cause SILValue::verifyOwnership to
not emit any output when the testing flag is enabled. This is to ensure I only
do not get duplicate textual error messages from the SILVerifier.

Author: gottesmm

include/swift/SIL/SILFunction.h

@@ -1052,6 +1052,13 @@ class SILFunction
   /// invariants.
   void verify(bool SingleFunction = true) const;
 
+  /// Run the SIL ownership verifier to check for ownership invariant failures.
+  ///
+  /// NOTE: The ownership verifier is always run when performing normal IR
+  /// verification, so this verification can be viewed as a subset of
+  /// SILFunction::verify.
+  void verifyOwnership(DeadEndBlocks *deadEndBlocks = nullptr) const;
+
   /// Verify that all non-cond-br critical edges have been split.
   ///
   /// This is a fast subset of the checks performed in the SILVerifier.

include/swift/SILOptimizer/PassManager/Passes.def

@@ -314,6 +314,8 @@ PASS(UsePrespecialized, "use-prespecialized",
      "Use Pre-Specialized Functions")
 PASS(OwnershipDumper, "ownership-dumper",
      "Print Ownership information for Testing")
+PASS(OwnershipVerifierTextualErrorDumper, "ownership-verifier-textual-error-dumper",
+     "Run ownership verification on all functions, emitting FileCheck-able textual errors instead of asserting")
 PASS(SemanticARCOpts, "semantic-arc-opts",
      "Semantic ARC Optimization")
 PASS(SimplifyUnreachableContainingBlocks, "simplify-unreachable-containing-blocks",

lib/SIL/Verifier/SILOwnershipVerifier.cpp

@@ -882,12 +882,41 @@ void SILInstruction::verifyOperandOwnership() const {
   }
 }
 
+static void verifySILValueHelper(const SILFunction *f, SILValue value,
+                                 DeadEndBlocks *deadEndBlocks) {
+  assert(!isa<SILUndef>(value) &&
+         "We assume we are always passed arguments or instruction results");
+
+  // If the given function has unqualified ownership or we have been asked by
+  // the user not to verify this function, there is nothing to verify.
+  if (!f->hasOwnership() || !f->shouldVerifyOwnership())
+    return;
+
+  using BehaviorKind = LinearLifetimeChecker::ErrorBehaviorKind;
+  Optional<LinearLifetimeChecker::ErrorBuilder> errorBuilder;
+  if (IsSILOwnershipVerifierTestingEnabled) {
+    errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndReturnFalse);
+  } else {
+    errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndAssert);
+  }
+
+  SmallPtrSet<SILBasicBlock *, 32> liveBlocks;
+  if (deadEndBlocks) {
+    SILValueOwnershipChecker(*deadEndBlocks, value, *errorBuilder, liveBlocks)
+        .check();
+  } else {
+    DeadEndBlocks deadEndBlocks(f);
+    SILValueOwnershipChecker(deadEndBlocks, value, *errorBuilder, liveBlocks)
+        .check();
+  }
+}
+
 void SILValue::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
   if (DisableOwnershipVerification)
     return;
 
   // Do not validate SILUndef values.
-  if (isa<SILUndef>(Value))
+  if (isa<SILUndef>(*this))
     return;
 
 #ifdef NDEBUG
@@ -898,8 +927,8 @@ void SILValue::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
   // that if we run into something that we do not understand, we do not assert
   // in user code even tohugh we aren't going to actually verify (the default
   // behavior when -sil-verify-all is disabled).
-  auto *Mod = Value->getModule();
-  if (!Mod || !Mod->getOptions().VerifyAll)
+  auto *mod = Value->getModule();
+  if (!mod || !mod->getOptions().VerifyAll)
     return;
 #endif
 
@@ -911,31 +940,51 @@ void SILValue::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
     }
   }
 
+  // If we are testing the verifier, bail so we only print errors once when
+  // performing a full verification a function at a time by the
+  // OwnershipVerifierStateDumper pass, instead of additionally in the
+  // SILBuilder and in the actual SIL verifier that may be run by sil-opt.
+  if (IsSILOwnershipVerifierTestingEnabled)
+    return;
+
   // Since we do not have SILUndef, we now know that getFunction() should return
   // a real function. Assert in case this assumption is no longer true.
-  SILFunction *f = (*this)->getFunction();
+  auto *f = (*this)->getFunction();
   assert(f && "Instructions and arguments should have a function");
+  verifySILValueHelper(f, *this, deadEndBlocks);
+}
+
+void SILFunction::verifyOwnership(DeadEndBlocks *deadEndBlocks) const {
+  if (DisableOwnershipVerification)
+    return;
+
+#ifdef NDEBUG
+  // When compiling without asserts enabled, only verify ownership if
+  // -sil-verify-all is set.
+  //
+  // NOTE: We purposely return if we do can not look up a module here to ensure
+  // that if we run into something that we do not understand, we do not assert
+  // in user code even tohugh we aren't going to actually verify (the default
+  // behavior when -sil-verify-all is disabled).
+  auto *mod = getParent();
+  if (!mod || !mod->getOptions().VerifyAll)
+    return;
+#endif
 
   // If the given function has unqualified ownership or we have been asked by
   // the user not to verify this function, there is nothing to verify.
-  if (!f->hasOwnership() || !f->shouldVerifyOwnership())
+  if (!hasOwnership() || !shouldVerifyOwnership())
     return;
 
-  using BehaviorKind = LinearLifetimeChecker::ErrorBehaviorKind;
-  Optional<LinearLifetimeChecker::ErrorBuilder> errorBuilder;
-  if (IsSILOwnershipVerifierTestingEnabled) {
-    errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndReturnFalse);
-  } else {
-    errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndAssert);
-  }
+  for (auto &block : *this) {
+    for (auto *arg : block.getArguments()) {
+      verifySILValueHelper(this, arg, deadEndBlocks);
+    }
 
-  SmallPtrSet<SILBasicBlock *, 32> liveBlocks;
-  if (deadEndBlocks) {
-    SILValueOwnershipChecker(*deadEndBlocks, *this, *errorBuilder, liveBlocks)
-        .check();
-  } else {
-    DeadEndBlocks deadEndBlocks(f);
-    SILValueOwnershipChecker(deadEndBlocks, *this, *errorBuilder, liveBlocks)
-        .check();
+    for (auto &inst : block) {
+      for (auto result : inst.getResults()) {
+        verifySILValueHelper(this, result, deadEndBlocks);
+      }
+    }
   }
 }

lib/SILOptimizer/UtilityPasses/CMakeLists.txt

@@ -31,4 +31,5 @@ silopt_register_sources(
   SimplifyUnreachableContainingBlocks.cpp
   StripDebugInfo.cpp
   OwnershipDumper.cpp
+  OwnershipVerifierTextualErrorDumper.cpp
 )

lib/SILOptimizer/UtilityPasses/OwnershipVerifierTextualErrorDumper.cpp

@@ -0,0 +1,49 @@
+//===--- OwnershipVerifierStateDumper.cpp ---------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+///
+/// This is a simple utility pass that verifies the ownership of all SILValue in
+/// a module with a special flag set that causes the verifier to emit textual
+/// errors instead of asserting. This is done one function at a time so that we
+/// can number the errors as we emit them so in FileCheck tests, we can be 100%
+/// sure we exactly matched the number of errors.
+///
+//===----------------------------------------------------------------------===//
+
+#include "swift/SIL/BasicBlockUtils.h"
+#include "swift/SIL/SILFunction.h"
+#include "swift/SIL/SILInstruction.h"
+#include "swift/SILOptimizer/PassManager/Passes.h"
+#include "swift/SILOptimizer/PassManager/Transforms.h"
+
+using namespace swift;
+
+//===----------------------------------------------------------------------===//
+//                            Top Level Entrypoint
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class OwnershipVerifierTextualErrorDumper : public SILFunctionTransform {
+  void run() override {
+    SILFunction *f = getFunction();
+    DeadEndBlocks deadEndBlocks(f);
+    f->verifyOwnership(&deadEndBlocks);
+  }
+};
+
+} // end anonymous namespace
+
+SILTransform *swift::createOwnershipVerifierTextualErrorDumper() {
+  return new OwnershipVerifierTextualErrorDumper();
+}

test/SIL/ownership-verifier/arguments.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -module-name Swift -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -module-name Swift -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 // This is a test that verifies ownership behavior around arguments that should

test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null %s 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 -o /dev/null %s 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 // This file tests that when we emit an error, it is a true negative. This is

test/SIL/ownership-verifier/disable_verifier_using_semantic_tag.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 // This test makes sure that the ownership verifier can be disabled on specific

test/SIL/ownership-verifier/interior_pointer.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 sil_stage canonical

test/SIL/ownership-verifier/leaks.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -module-name Swift -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -module-name Swift -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 // This file is meant to contain dataflow tests that are true leaks. It is

test/SIL/ownership-verifier/over_consume.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null 2>&1  %s | %FileCheck %s
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 -o /dev/null %s 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 sil_stage canonical

test/SIL/ownership-verifier/subobject_borrowing.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null 2>&1  %s | %FileCheck %s
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 -o /dev/null %s 2>&1 | %FileCheck %s
 // REQUIRES: asserts
 
 sil_stage canonical