Hello,
I've been looking into swiftlang/swift#91665, which is a dead swift_bridgeObjectRelease that gets
left behind in a hot String to Span loop, and I ended up with a fix that depends on a semantic
assumption I can't find written down anywhere, so I figured I should ask instead of guessing.
The short version of what goes wrong is that an escape walk gives up, and once it gives up
AliasAnalysis.getApplyEffect (AliasAnalysis.swift:418) falls back to the global effects of the
callee, which say "may write", and that stops redundant load elimination from forwarding the store
that would let the release fold away. The place it gives up is walkDownCallee
(EscapeUtils.swift:647), where the precise "indirect arguments cannot escape" early out gets
skipped whenever apply.isAddressable(operand:) is true, which was added in #80628. That skip is
correct as far as it goes, because for a small String the address really is captured into the
returned UTF8Span, so the escape answer really is "it escaped".
The thing is that the caller is asking a narrower question than the one being answered. It doesn't
need to know where the address went, it only needs to know if this call can write through it. If a
borrow can't be used to write, then that's answerable locally, with no annotations and without
needing to know which field of the result the address landed in, and it also works for types that
users define themselves, which none of the other approaches I tried do.
So the rule I'd like to rely on is that a value whose lifetime is a scoped dependence on the address
of an @in_guaranteed argument can be used to read that storage but never to write it. My question
is whether that's actually guaranteed, or if it's just how everything happens to be written today.
The reason I'm not comfortable assuming it is that I checked and a stock compiler doesn't currently
rely on it. Here's a ~Escapable view built from a borrowing parameter that takes the address out
and writes through it anyway:
@_addressableForDependencies
public struct MyBox { var a: Int; var p: Payload }
public struct EvilView: ~Escapable, ~Copyable {
var baseBits: Int
@_lifetime(borrow box)
init(box: borrowing MyBox) {
self.baseBits = Int(bitPattern: UnsafeRawPointer(Builtin.addressOfBorrow(box)))
}
@inline(never) public func evilWrite(_ v: Int) {
UnsafeMutableRawPointer(bitPattern: baseBits)!.storeBytes(of: v, as: Int.self)
}
}
@inline(never) public func caller(_ p: Payload, _ n: Int) -> Int {
var b = MyBox(a: n, p: p)
b.a = n
do { let v = b.view(); v.evilWrite(99) } // view() is borrowing, @_lifetime(borrow self)
return b.a
}
At -O on a 6.5-dev snapshot this prints 99, and with the rule applied it prints 3. I'm assuming
the program is undefined behaviour anyway, since SE-0176 says that exclusivity conflicts through
unsafe pointers "simply have undefined behavior", but since applying the rule would be a behaviour
change and not just writing down something the compiler already does, I'd rather have that confirmed
than assume it.
The other thing I found out is that the parameter convention is doing all of the work
here and nothing else is. The same change without gating on @in_guaranteed is just a miscompile.
If you give it an @inout source, so something shaped like a MutableSpan producer where the
dependent value is allowed to write, a store gets forwarded across the mutating call and the same
kind of program prints 3 instead of 99. That version passed 1335 SILOptimizer tests before I caught
it by actually running a program. It's easy to get wrong because the dependence itself doesn't warn
you at all:
(@in_guaranteed String) -> @lifetime(borrow address_for_deps 0) @owned UTF8Span
(@inout Array<UInt8>) -> @lifetime(borrow 0) @owned MutableSpan<UInt8>
ParsedLifetimeDependenceKind does tell Borrow and Inout apart (LifetimeDependence.h:43), but
LifetimeDependenceKind collapses both of them into Scope (:50), so by the time the optimizer
sees it the SIL dependence carries nothing about reading or writing and the convention is the only
thing left that does. Also, isCompatibleWithOwnership (LifetimeDependence.cpp:995) accepts
borrow on an InOut parameter when it's parsing a .swiftinterface, so keying off the attribute
spelling would have been the wrong thing to do and keying off the convention isn't.
So what I'm actually asking is whether the rule is sound as stated, meaning does a scoped borrow
dependence extend the formal access scope of the source across the lifetime of the dependent value,
so that writing through that value is an exclusivity violation and therefore undefined behaviour. I
also don't know if @in_guaranteed is the right thing to check, or if there's a case I haven't
thought of where a scoped dependence on a guaranteed argument can legitimately give you something
you can write to. I've assumed mark_dependence [nonescaping] isn't relevant here since it
constrains lifetime and not mutation, so I didn't require it, but I'd like to know if that reading
is wrong. And even if the rule does hold, I don't know if it's acceptable for the optimizer to
depend on it right now or if that should wait, because SE-0465 describes @_lifetime as "an
illustrative placeholder … until the language gains an official way to express such constraints",
and I'd rather not bake semantics into the optimizer before the design settles.
On a separate note, and this one has nothing to do with whether any of the above is a good idea, the
begin_apply arm of walkDownCallee never calls apply.isAddressable(operand:) at all. If a
coroutine can yield a value that depends on an addressable argument, that looks like it might
already be unsound today, independently of this change. Happy to open a separate issue for it if
that's the right read.
I can share the patch and the reproducers if that's useful. It's about 25 lines in total, a visitor
hook that defaults to false, a call to it in the addressable arm of walkDownCallee, and one
implementor in the memory effects visitor, so only that client is affected and stack promotion,
the escape to instruction queries and the effects derivation all keep the answers they give today.
Let me know.
All the line numbers above are against main as of 89c950d1.