Runtime crash from consume inside _modify

[gor-gyolchanyan]

Description

There is a runtime crash caused by a consume expression inside a _modify accessor.

Reproduction

The issue can be reproduced by creating a Swift package with an executable target, pasting the following code snippet into its main.swift, then building and running the executable target.

example()

// An example function exercising the functionality of `ExecutionPath`.
func example() {
    var executionPath = ExecutionPath()
    print(executionPath.traceMessageList)
    // []
    executionPath.traceMessageList.append("outer") 
    // will change traceMessageList from [] to ["outer"]
    // did change traceMessageList from [] to ["outer"]
    print(executionPath.traceMessageList)
    //--------------------------//
    // Thread 1: EXC_BAD_ACCESS //
    //--------------------------//
    executionPath.traceMessageList.append("inner")
    print(executionPath.traceMessageList)
}

// An example type simplifying a more elaborate solution for the purpose of
// reproducing this issue.
struct ExecutionPath {

    init() { _traceMessageList = .init() }

    private var _traceMessageList: [String]

    // This is a representation of a property after being modified by an
    // accessor macro that transforms the original property with `willSet`
    // and/or `didSet` accessors into an observing proxy for an existing
    // property.
    var traceMessageList: [String] {
        get { _traceMessageList }

        _modify {
            // The following two variables have unique names generated by the
            // accessor macro to prevent them from interfering with the user
            // code that is copied into `do` statements below.
            let _priorTraceMessageList = _traceMessageList
            var _nextTraceMessageList = _priorTraceMessageList
            defer {
                do {
                    // The following variable has a name that is taken from the
                    // parameter of the `willSet` accessor if one was present,
                    // or "newValue" if not.
                    // The existing "newValue" is consumed into this variable
                    // in order to avoid the unnecessary reference counting
                    // overhead while renaming the variable.
                    let newValue = consume _nextTraceMessageList
                    // The body of the the `willSet` accessor is copied into the
                    // following `do` statement verbatim.
                    do {
                        // The copied code is wrapped into a `do` statement in
                        // order to avoid possible "invalid redeclaration"
                        // errors for "newValue".
                        print("will change traceMessageList from \(traceMessageList) to \(newValue)")
                    }
                    // The renamed "newValue" is put back into the original
                    // variable so that it can be committed into storage later.
                    _nextTraceMessageList = consume newValue
                }
                // The "newValue" is consumed when committing it into storage.
                //-----------------------------------------------------------//
                // NOTE: Removing this `consume` keyword resolves the crash. //
                //-----------------------------------------------------------//
                _traceMessageList = consume _nextTraceMessageList
                do {
                    // The following variable has a name that is taken from the
                    // parameter of the `didSet` accessor if one was present,
                    // or "oldValue" if not.
                    let oldValue = consume _priorTraceMessageList
                    // The body of the the `didSet` accessor is copied into the
                    // following `do` statement verbatim.
                    do {
                        // The copied code is wrapped into a `do` statement in
                        // order to avoid possible "invalid redeclaration"
                        // errors for "oldValue".
                        print("did change traceMessageList from \(oldValue) to \(traceMessageList)")
                    }
                    // The "oldValue" is consumed into oblivion explicitly in
                    // order to avoid possible "unused variable" warnings.
                    _ = consume oldValue
                }
            }
            yield &_nextTraceMessageList
        }
    }
}

Stack dump

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x20a3b0528)
    frame #0: 0x00000001afa290c8 libswiftCore.dylib`swift_retain + 44
    frame #1: 0x00000001af9dc630 libswiftCore.dylib`initializeBufferWithCopyOfBuffer value witness for Swift.Character + 32
    frame #2: 0x00000001af6773f4 libswiftCore.dylib`merged Swift.Array.description.getter : Swift.String + 696
    frame #3: 0x00000001af74016c libswiftCore.dylib`Swift._print_unlocked<τ_0_0, τ_0_1 where τ_0_1: Swift.TextOutputStream>(τ_0_0, inout τ_0_1) -> () + 976
    frame #4: 0x00000001af80d4ac libswiftCore.dylib`merged generic specialization <Swift._Stdout> of Swift._print<τ_0_0 where τ_0_0: Swift.TextOutputStream>(_: Swift.Array<Any>, separator: Swift.String, terminator: Swift.String, to: inout τ_0_0) -> () + 184
    frame #5: 0x00000001af80c744 libswiftCore.dylib`merged Swift.print(_: Any..., separator: Swift.String, terminator: Swift.String) -> () + 192
  * frame #6: 0x0000000100001514 example`example() at main.swift:11:5
    frame #7: 0x0000000100001358 example`main at main.swift:1:1
    frame #8: 0x000000019e530274 dyld`start + 2840

Expected behavior

The example function executes to completion and produces the following output:

[]
will change traceMessageList from [] to ["outer"]
did change traceMessageList from [] to ["outer"]
["outer"]
will change traceMessageList from ["outer"] to ["outer", "inner"]
did change traceMessageList from ["outer"] to ["outer", "inner"]
["outer", "inner"]

Environment

$ swift --version 
swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0

Additional information

Removing the consume keyword in the _traceMessageList = consume _nextTraceMessageList statement resolves the issue.

Based on this observation, it seems that the code generator may be emitting unconditional cleanup code after the yield statement for _nextTraceMessageList even when it has been explicitly consumed during assignment to _traceMessageList, causing the cleanup code to run on the same memory twice.

[jameesbrown]

Here is a reduced version:

struct S {
  var storage: [String] = []
  var list: [String] {
    _read {
      yield storage
    }
    _modify {
      var newValue = storage
      defer {
        storage = consume newValue
      }
      yield &newValue
    }
  }
}
var s = S()
s.list.append("a")
s.list.append("b")

Interestingly not using defer, i.e. just doing the consume after the yield, also prevents the crash.