diff --git a/include/swift/SIL/SILFunction.h b/include/swift/SIL/SILFunction.h index 6213188af2fba..7ffe87bc892d3 100644 --- a/include/swift/SIL/SILFunction.h +++ b/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. diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def index 55f12b2d7d8d5..eb13c359e8062 100644 --- a/include/swift/SILOptimizer/PassManager/Passes.def +++ b/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", diff --git a/lib/SIL/Verifier/SILOwnershipVerifier.cpp b/lib/SIL/Verifier/SILOwnershipVerifier.cpp index bbce868ac244b..817381749f2dc 100644 --- a/lib/SIL/Verifier/SILOwnershipVerifier.cpp +++ b/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(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 errorBuilder; + if (IsSILOwnershipVerifierTestingEnabled) { + errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndReturnFalse); + } else { + errorBuilder.emplace(*f, BehaviorKind::PrintMessageAndAssert); + } + + SmallPtrSet 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(Value)) + if (isa(*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 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 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); + } + } } } diff --git a/lib/SILOptimizer/UtilityPasses/CMakeLists.txt b/lib/SILOptimizer/UtilityPasses/CMakeLists.txt index b7472977050d7..24f268320b8da 100644 --- a/lib/SILOptimizer/UtilityPasses/CMakeLists.txt +++ b/lib/SILOptimizer/UtilityPasses/CMakeLists.txt @@ -31,4 +31,5 @@ silopt_register_sources( SimplifyUnreachableContainingBlocks.cpp StripDebugInfo.cpp OwnershipDumper.cpp + OwnershipVerifierTextualErrorDumper.cpp ) diff --git a/lib/SILOptimizer/UtilityPasses/OwnershipVerifierTextualErrorDumper.cpp b/lib/SILOptimizer/UtilityPasses/OwnershipVerifierTextualErrorDumper.cpp new file mode 100644 index 0000000000000..167b8e584cc96 --- /dev/null +++ b/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(); +} diff --git a/test/SIL/ownership-verifier/arguments.sil b/test/SIL/ownership-verifier/arguments.sil index 0978148817bc1..0d2c02327791a 100644 --- a/test/SIL/ownership-verifier/arguments.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil b/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil index 5f738b0adc60c..2aa91496230c3 100644 --- a/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/disable_verifier_using_semantic_tag.sil b/test/SIL/ownership-verifier/disable_verifier_using_semantic_tag.sil index 6bc788272470a..b7ba3d3564f60 100644 --- a/test/SIL/ownership-verifier/disable_verifier_using_semantic_tag.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/interior_pointer.sil b/test/SIL/ownership-verifier/interior_pointer.sil index b82eb54e782f3..4ce8fd58249f5 100644 --- a/test/SIL/ownership-verifier/interior_pointer.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/leaks.sil b/test/SIL/ownership-verifier/leaks.sil index a3d45849f5c91..e680576e80e19 100644 --- a/test/SIL/ownership-verifier/leaks.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/over_consume.sil b/test/SIL/ownership-verifier/over_consume.sil index 556800a619eca..d59ade297caf3 100644 --- a/test/SIL/ownership-verifier/over_consume.sil +++ b/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 diff --git a/test/SIL/ownership-verifier/subobject_borrowing.sil b/test/SIL/ownership-verifier/subobject_borrowing.sil index 29dae9141bc95..040e7230a0fd7 100644 --- a/test/SIL/ownership-verifier/subobject_borrowing.sil +++ b/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