Closure cycles were originally enforced "conservatively". Real code
does, however, use recursive local functions. And it's surprisingly
easy to create false exclusivity violations in those cases.
This is safe as long as we can rely on SILGen to keep captured
variables in boxes if the capture escapes in any closure that may call
other closures that capture the same variable.
Fixes https://github.com/apple/swift/issues/61404
Dynamic exclusivity checking bug with nested functions. #61404
Fixes rdar://102056143 (assertion failure due to exclusivity checker -
AccessEnforcementSelection should handle recursive local captures)
SWIFT_STDLIB_SINGLE_THREADED_RUNTIME is too much of a blunt instrument here.
It covers both the Concurrency runtime and the rest of the runtime, but we'd
like to be able to have e.g. a single-threaded Concurrency runtime while
the rest of the runtime is still thread safe (for instance).
So: rename it to SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY and make it just
control the Concurrency runtime, then add a SWIFT_STDLIB_THREADING_PACKAGE
setting at the CMake/build-script level, which defines
SWIFT_STDLIB_THREADING_xxx where xxx depends on the chosen threading package.
This is especially useful on systems where there may be a choice of threading
package that you could use.
rdar://90776105
SWIFT_STDLIB_SINGLE_THREADED_RUNTIME is too much of a blunt instrument here.
It covers both the Concurrency runtime and the rest of the runtime, but we'd
like to be able to have e.g. a single-threaded Concurrency runtime while
the rest of the runtime is still thread safe (for instance).
So: rename it to SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY and make it just
control the Concurrency runtime, then add a SWIFT_STDLIB_THREADING_PACKAGE
setting at the CMake/build-script level, which defines
SWIFT_STDLIB_THREADING_xxx where xxx depends on the chosen threading package.
This is especially useful on systems where there may be a choice of threading
package that you could use.
rdar://90776105
The back-deployed concurrency libraries and older Swift runtimes are
not compatible, so turn off runtime exclusivity checking when running on
older systems. Fixes rdar://84274148.
I also removed the -verify-sil-ownership flag in favor of a disable flag
-disable-sil-ownership-verifier. I used this on only two tests that still need
work to get them to pass with ownership, but whose problems are well understood,
small corner cases. I am going to fix them in follow on commits. I detail them
below:
1. SILOptimizer/definite_init_inout_super_init.swift. This is a test case where
DI is supposed to error. The only problem is that we crash before we error since
the code emitting by SILGen to trigger this error does not pass ownership
invariants. I have spoken with JoeG about this and he suggested that I fix this
earlier in the compiler. Since we do not run the ownership verifier without
asserts enabled, this should not affect compiler users. Given that it has
triggered DI errors previously I think it is safe to disable ownership here.
2. PrintAsObjC/extensions.swift. In this case, the signature generated by type
lowering for one of the thunks here uses an unsafe +0 return value instead of
doing an autorelease return. The ownership checker rightly flags this leak. This
is going to require either an AST level change or a change to TypeLowering. I
think it is safe to turn this off since it is such a corner case that it was
found by a test that has nothing to do with it.
rdar://43398898
The problem here is that without this patch we emit code like this:
bb0(%0 : @owned $T):
%1 = partial_apply %foo(%0)
%2 = mark_dependence %1 on %0
Since a partial_apply consumes the object, the mark_dependence is a use after
free (especially if one has any further uses of %0 after the mark_dependence).
So what I did was I copied the value before creating the partial_apply. So
now we get this:
bb0(%0 : @owned $T):
%1 = copy_value %0
%2 = partial_apply %foo(%1)
%3 = mark_dependence %2 on %0
...
destroy_value %0
This ensures that one can actually have uses after the mark_dependence of both
operands.
This enables ownership verification to be enabled on
Interpreter/enforce_exclusive_access.
rdar://48521061
This is in preparation to make the code here more target agnostic for
porting to the Windows threading primitives. This is used pretty
extensively in the tests, so disabling tests would lose a chunk of
coverage.
A directly applied noescape closure that captures a local requires
dynamic enforcement if that local escaped prior to the direct
application. Previously this was only statically enforced, which could
miss conflicts.
It's unlikely that an actual conflict could occur, but can happen in
cases like this:
func noescapeConflict() {
var localVal = 0
let nestedModify = { localVal = 3 }
_ = {
takeInoutAndPerform(&localVal, closure: nestedModify)
}()
}
Code isn't normally written like this, but the general pattern of
passing an escaping closure as an argument to a noescape closure may
arise when using withoutActuallyEscaping. Even in that case, it's
highly unlikely that a real conflict would exist.
For this case to be handled correcly, we must also never allow a
closure passed to withoutActuallyEscaping to be SILGen's as a
non-escaping closure. Doing so would result in a non-escaping closure
to be passed as an argument to the withoutActuallyEscaping trailing
closure, which is also non-escaping. That directly violates the
Non-Escaping Recursion Restriction, but there would be no way to
diagnose the violation, creating a hole in the exclusivity model.
Narrow the list of address producers that verification can "peek through"
to avoid trigger an assertion in the previous commit with
-enable-verify-exclusivity.
My previous attempt to strengthen this verification ignored the fact that a
projection or addressor that returns UnsafePointer can have nested access after
being passed as an @inout argument. After inlining, this caused the verifier to
assert.
Instead, handle access to an UnsafePointer as a valid but unidentified access. I
was trying to avoid this because an UnsafePointer could refer to a global or
class property, which we can never consider "Unidentified". However, this is
reasonably safe because it can only result from a _nested_ access. If a global
or class property is being addressed, it must already have its own dynamic
access within the addressor, and that access will be exposed to the
optimizer. If a non-public KeyPath is being addressed, a keypath instruction
must already exist somewhere in the module which is exposed to the optimizer.
Fixes <rdar://problem/41660554> Swift CI (macOS release master, OSS): SIL verification failed: Unknown formal access pattern: storage.
Modify IRGen to emit builtin access markers with an error flag in
Swift 3 mode.
KeyPath enforcement is required by user code in Swift 4+ mode, but is
implemented within the standard library. A [builtin] flag marks the
special case for access generated by Builtins so that they are
always enforced as an error regardless of the language mode.
This is necessary for Swift 4.2 because the standard library continues
to build in Swift 3 mode. Once the standard library build migrates,
this is all irrelevant.
This does not actually affect existing Swift 3 code, since the KeyPath
feature wasn't introduced until Swift 4.
<rdar://problem/40115738> [Exclusivity] Enforce Keypath access as an error, not a warning in 4.2.
Add dynamic enforcement of exclusive access when a KeyPath directly accesses a final
stored property on an instance of a class. For read-only projections, this begins and ends
the access immediately. For mutable projections, this uses the ClassHolder to perform
a long-term access that lasts as long as the lifetime of the ClassHolder.
rdar://problem/31972680
This converts the instances of the pattern for which we have a proper
substitution in lit. This will make it easier to replace it
appropriately with Windows equivalents.
In AccessEnforcementSelection, treat passing a projection from a box to
a partial_apply as an escape to force using dynamic enforcement on the box.
This will now correctly use dynamic enforcement on variables that are taken
as inout and also captured by storage address in a closure:
var x = ...
x.mutatingMethod { ... use x ...}
but does pessimize some of our existing enforcement into dynamic since
access enforcement selection.
Ideally we would distinguish between escaping via an nonescaping closures
(which can only conflict with accesses that are in progress) and
escaping via escaping closures (which can conflict for any reachable code
after the escape)
