Teach exclusivity access marker verifier to handle UnsafePointer. #17935
Conversation
|
@swift-ci test. |
|
|
||
| // The compiler intrinsic _projectXXXKeyPath returns a tuple of | ||
| // UnsafePointer and Optional AnyObject. After inlining, the unsafe | ||
| // pointer may appear to orinate from anywhere, including a phi. |
There was a problem hiding this comment.
Oops thanks. Restarting the tests ;)
|
@swift-ci test. |
|
Build failed |
|
Build failed |
lib/SIL/MemAccessUtils.cpp
Outdated
| assert(isa<BuiltinRawPointerType>(SEI->getType().getASTType())); | ||
| auto &C = SEI->getModule().getASTContext(); | ||
| auto *decl = SEI->getStructDecl(); | ||
| return (decl == C.getUnsafeMutablePointerDecl() |
There was a problem hiding this comment.
The return value here doesn't need () around it.
| @@ -306,7 +306,7 @@ ExclusiveAccessTestSuite.test("SequentialKeyPathWritesDontOverlap") { | |||
|
|
|||
| c[keyPath: getF] = 7 | |||
| c[keyPath: getF] = 8 // no-trap | |||
| c[keyPath: getF] += c[keyPath: getF] + 1 // no-trap | |||
| c[keyPath: getF] += c[keyPath: getF] // no-trap | |||
There was a problem hiding this comment.
Question. Was this filecheck test changed for a reason.
There was a problem hiding this comment.
Cleanup in a closely related test case.
|
@swift-ci test. |
|
Build failed |
|
Build failed |
|
LGTM |
|
@swift-ci smoke test and merge. |
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.
|
@swift-ci smoke test and merge. |
Narrow the list of address producers that verification can "peek through" to avoid trigger an assertion in the previous commit with -enable-verify-exclusivity.
|
@swift-ci smoke test. |
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.