Merge pull request swiftlang#31539 from gottesmm/pr-88ca78b6938c98c9da3c1f7a88d81fac297d0ee2

[ownership] Add new pass OwnershipVerifierTextualErrorDumper and use it in ownership verifier FileCheck tests instead of 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