diff --git a/include/swift/SIL/InstructionUtils.h b/include/swift/SIL/InstructionUtils.h index a1ec3ea2fbbc4..12722d64ac3c0 100644 --- a/include/swift/SIL/InstructionUtils.h +++ b/include/swift/SIL/InstructionUtils.h @@ -247,6 +247,13 @@ lookUpFunctionInWitnessTable(WitnessMethodInst *wmi, SILModule::LinkingMode link /// struct-with-deinit drops the deinit. bool shouldExpand(SILModule &module, SILType ty); +/// Returns true if `arg` is mutated. +/// if `ignoreDestroys` is true, `destroy_addr` instructions are ignored. +/// `defaultIsMutating` specifies the state of instructions which are not explicitly handled. +/// For historical reasons this utility is implemented in SILVerifier.cpp. +bool isIndirectArgumentMutated(SILFunctionArgument *arg, bool ignoreDestroys = false, + bool defaultIsMutating = false); + } // end namespace swift #endif diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 655d00c0325dd..823f3ab7d9166 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -35,6 +35,7 @@ #include "swift/SIL/DebugUtils.h" #include "swift/SIL/Dominance.h" #include "swift/SIL/DynamicCasts.h" +#include "swift/SIL/InstructionUtils.h" #include "swift/SIL/MemAccessUtils.h" #include "swift/SIL/OwnershipLiveness.h" #include "swift/SIL/OwnershipUtils.h" @@ -572,6 +573,11 @@ void verifyKeyPathComponent(SILModule &M, /// open_existential_addr. We should expand it as needed. struct ImmutableAddressUseVerifier { SmallVector worklist; + bool ignoreDestroys; + bool defaultIsMutating; + + ImmutableAddressUseVerifier(bool ignoreDestroys = false, bool defaultIsMutating = false) + : ignoreDestroys(ignoreDestroys), defaultIsMutating(defaultIsMutating) {} bool isConsumingOrMutatingArgumentConvention(SILArgumentConvention conv) { switch (conv) { @@ -704,10 +710,9 @@ struct ImmutableAddressUseVerifier { } } - // Otherwise this is a builtin that we are not expecting to see, so bail - // and assert. - llvm::errs() << "Unhandled, unexpected builtin instruction: " << *inst; - llvm_unreachable("invoking standard assertion failure"); + // Otherwise this is a builtin that we are not expecting to see. + if (defaultIsMutating) + return true; break; } case SILInstructionKind::MarkDependenceInst: @@ -775,7 +780,9 @@ struct ImmutableAddressUseVerifier { else break; case SILInstructionKind::DestroyAddrInst: - return true; + if (!ignoreDestroys) + return true; + break; case SILInstructionKind::UpcastInst: case SILInstructionKind::UncheckedAddrCastInst: { if (isAddrCastToNonConsuming(cast(inst))) { @@ -841,9 +848,7 @@ struct ImmutableAddressUseVerifier { } break; } - llvm::errs() << "Unhandled, unexpected instruction: " << *inst; - llvm_unreachable("invoking standard assertion failure"); - break; + return true; } case SILInstructionKind::TuplePackElementAddrInst: { if (&cast(inst)->getOperandRef( @@ -865,8 +870,8 @@ struct ImmutableAddressUseVerifier { return false; } default: - llvm::errs() << "Unhandled, unexpected instruction: " << *inst; - llvm_unreachable("invoking standard assertion failure"); + if (defaultIsMutating) + return true; break; } } @@ -7385,6 +7390,11 @@ class SILVerifier : public SILVerifierBase { #undef require #undef requireObjectType +bool swift::isIndirectArgumentMutated(SILFunctionArgument *arg, bool ignoreDestroys, + bool defaultIsMutating) { + return ImmutableAddressUseVerifier(ignoreDestroys, defaultIsMutating).isMutatingOrConsuming(arg); +} + //===----------------------------------------------------------------------===// // Out of Line Verifier Run Functions //===----------------------------------------------------------------------===// diff --git a/lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp b/lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp index 0f9d449925eb5..268170041ec25 100644 --- a/lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp +++ b/lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp @@ -88,6 +88,15 @@ bool FunctionSignatureTransform::OwnedToGuaranteedAnalyzeParameters() { continue; } + // Make sure that an @in argument is not mutated otherwise than destroyed, + // because an @in_guaranteed argument must not be mutated. + if (A.hasConvention(SILArgumentConvention::Indirect_In) && + // With opaque values, @in arguments can have non-address types. + A.Arg->getType().isAddress() && + isIndirectArgumentMutated(A.Arg, /*ignoreDestroys=*/ true, /*defaultIsMutating=*/true)) { + continue; + } + // See if we can find a ref count equivalent strong_release or release_value // at the end of this function if our argument is an @owned parameter. // See if we can find a destroy_addr at the end of this function if our diff --git a/test/SILOptimizer/functionsigopts_crash.swift b/test/SILOptimizer/functionsigopts_crash.swift index 60f87eb25f358..f499fc72d4cae 100644 --- a/test/SILOptimizer/functionsigopts_crash.swift +++ b/test/SILOptimizer/functionsigopts_crash.swift @@ -35,3 +35,21 @@ func testit(_ p: P) { public func callit(s: S) { testit(s) } + +// Check that FSO does not trigger a verifier error caused by a mutated @in argument which is +// converted to an @in_guaranteed argument (which must not be mutated). + +public protocol IP { + associatedtype Element + + init(iterator: consuming Iterator) where Iterator: IteratorProtocol, Iterator.Element == Element +} + +extension Array: IP { + public init(iterator: consuming Iterator) where Iterator: IteratorProtocol, Iterator.Element == Element { + self.init() + while let next = iterator.next() { + append(next) + } + } +}