From d9b5a4621f63d69becda1a5d1cbf4b956b51dab3 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sat, 22 Feb 2025 07:56:57 -1000 Subject: [PATCH] Enable strict memory safety in the Distributed module --- stdlib/public/Distributed/CMakeLists.txt | 1 + .../public/Distributed/DistributedActor.swift | 4 +- .../Distributed/DistributedActorSystem.swift | 46 +++++++++---------- .../Distributed/DistributedAssertions.swift | 18 ++++---- .../DistributedDefaultExecutor.swift | 6 +-- .../Distributed/DistributedMetadata.swift | 12 ++--- .../LocalTestingDistributedActorSystem.swift | 11 +++-- 7 files changed, 50 insertions(+), 48 deletions(-) diff --git a/stdlib/public/Distributed/CMakeLists.txt b/stdlib/public/Distributed/CMakeLists.txt index 0a3a821da56..ae3f0fdad12 100644 --- a/stdlib/public/Distributed/CMakeLists.txt +++ b/stdlib/public/Distributed/CMakeLists.txt @@ -52,6 +52,7 @@ add_swift_target_library(swiftDistributed ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES} IS SWIFT_COMPILE_FLAGS ${SWIFT_STANDARD_LIBRARY_SWIFT_FLAGS} -parse-stdlib + -strict-memory-safety LINK_FLAGS "${SWIFT_RUNTIME_SWIFT_LINK_FLAGS}" diff --git a/stdlib/public/Distributed/DistributedActor.swift b/stdlib/public/Distributed/DistributedActor.swift index fd53d71e71a..a2df218e2e4 100644 --- a/stdlib/public/Distributed/DistributedActor.swift +++ b/stdlib/public/Distributed/DistributedActor.swift @@ -406,10 +406,10 @@ extension DistributedActor { @_implements(Actor, unownedExecutor) public nonisolated var __actorUnownedExecutor: UnownedSerialExecutor { if #available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *) { - return unownedExecutor + return unsafe unownedExecutor } else { // On older platforms, all distributed actors are default actors. - return UnownedSerialExecutor(Builtin.buildDefaultActorExecutorRef(self)) + return unsafe UnownedSerialExecutor(Builtin.buildDefaultActorExecutorRef(self)) } } diff --git a/stdlib/public/Distributed/DistributedActorSystem.swift b/stdlib/public/Distributed/DistributedActorSystem.swift index df02708b1be..f6f30fd50d3 100644 --- a/stdlib/public/Distributed/DistributedActorSystem.swift +++ b/stdlib/public/Distributed/DistributedActorSystem.swift @@ -438,8 +438,8 @@ extension DistributedActorSystem { // Gen the generic environment (if any) associated with the target. let genericEnv = - targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in - _getGenericEnvironmentOfDistributedTarget( + unsafe targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in + unsafe _getGenericEnvironmentOfDistributedTarget( targetNameUTF8.baseAddress!, UInt(targetNameUTF8.endIndex)) } @@ -449,11 +449,11 @@ extension DistributedActorSystem { var numWitnessTables: Int = 0 defer { - substitutionsBuffer?.deallocate() - witnessTablesBuffer?.deallocate() + unsafe substitutionsBuffer?.deallocate() + unsafe witnessTablesBuffer?.deallocate() } - if let genericEnv = genericEnv { + if let genericEnv = unsafe genericEnv { let subs = try invocationDecoder.decodeGenericSubstitutions() if subs.isEmpty { throw ExecuteDistributedTargetError( @@ -461,14 +461,14 @@ extension DistributedActorSystem { errorCode: .missingGenericSubstitutions) } - substitutionsBuffer = .allocate(capacity: subs.count) + unsafe substitutionsBuffer = .allocate(capacity: subs.count) for (offset, substitution) in subs.enumerated() { - let element = substitutionsBuffer?.advanced(by: offset) - element?.initialize(to: substitution) + let element = unsafe substitutionsBuffer?.advanced(by: offset) + unsafe element?.initialize(to: substitution) } - (witnessTablesBuffer, numWitnessTables) = _getWitnessTablesFor(environment: genericEnv, + unsafe (witnessTablesBuffer, numWitnessTables) = unsafe _getWitnessTablesFor(environment: genericEnv, genericArguments: substitutionsBuffer!) if numWitnessTables < 0 { throw ExecuteDistributedTargetError( @@ -478,8 +478,8 @@ extension DistributedActorSystem { } let paramCount = - targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in - __getParameterCount( + unsafe targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in + unsafe __getParameterCount( targetNameUTF8.baseAddress!, UInt(targetNameUTF8.endIndex)) } @@ -497,12 +497,12 @@ extension DistributedActorSystem { // Prepare buffer for the parameter types to be decoded into: let argumentTypesBuffer = UnsafeMutableBufferPointer.allocate(capacity: Int(paramCount)) defer { - argumentTypesBuffer.deallocate() + unsafe argumentTypesBuffer.deallocate() } // Demangle and write all parameter types into the prepared buffer - let decodedNum = targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in - __getParameterTypeInfo( + let decodedNum = unsafe targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in + unsafe __getParameterTypeInfo( targetNameUTF8.baseAddress!, UInt(targetNameUTF8.endIndex), genericEnv, @@ -525,7 +525,7 @@ extension DistributedActorSystem { var argumentTypes: [Any.Type] = [] do { argumentTypes.reserveCapacity(Int(decodedNum)) - for argumentType in argumentTypesBuffer { + for unsafe argumentType in unsafe argumentTypesBuffer { argumentTypes.append(argumentType) } } @@ -536,8 +536,8 @@ extension DistributedActorSystem { } let maybeReturnTypeFromTypeInfo = - targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in - __getReturnTypeInfo( + unsafe targetNameUTF8.withUnsafeBufferPointer { targetNameUTF8 in + unsafe __getReturnTypeInfo( /*targetName:*/targetNameUTF8.baseAddress!, /*targetLength:*/UInt(targetNameUTF8.endIndex), /*genericEnv:*/genericEnv, @@ -549,7 +549,7 @@ extension DistributedActorSystem { errorCode: .typeDeserializationFailure) } - guard let resultBuffer = _openExistential(returnTypeFromTypeInfo, do: doAllocateReturnTypeBuffer) else { + guard let resultBuffer = _openExistential(returnTypeFromTypeInfo, do: unsafe doAllocateReturnTypeBuffer) else { throw ExecuteDistributedTargetError( message: "Failed to allocate buffer for distributed target return type", errorCode: .typeDeserializationFailure) @@ -560,16 +560,16 @@ extension DistributedActorSystem { var executeDistributedTargetHasThrown = true func doDestroyReturnTypeBuffer(_: R.Type) { - let buf = resultBuffer.assumingMemoryBound(to: R.self) + let buf = unsafe resultBuffer.assumingMemoryBound(to: R.self) if !executeDistributedTargetHasThrown { // since the _execute function has NOT thrown, // there must be a value in the result buffer that we must deinitialize - buf.deinitialize(count: 1) + unsafe buf.deinitialize(count: 1) } // otherwise, the _execute has thrown and not populated the result buffer // finally, deallocate the buffer - buf.deallocate() + unsafe buf.deallocate() } defer { @@ -581,7 +581,7 @@ extension DistributedActorSystem { // let errorType = try invocationDecoder.decodeErrorType() // TODO(distributed): decide how to use when typed throws are done // Execute the target! - try await _executeDistributedTarget( + try unsafe await _executeDistributedTarget( on: actor, /*targetNameData:*/targetName, /*targetNameLength:*/UInt(targetName.count), @@ -599,7 +599,7 @@ extension DistributedActorSystem { if returnType == Void.self { try await handler.onReturnVoid() } else { - try await self.invokeHandlerOnReturn( + try unsafe await self.invokeHandlerOnReturn( handler: handler, resultBuffer: resultBuffer, metatype: returnType diff --git a/stdlib/public/Distributed/DistributedAssertions.swift b/stdlib/public/Distributed/DistributedAssertions.swift index f4ac7c90552..881741904c8 100644 --- a/stdlib/public/Distributed/DistributedAssertions.swift +++ b/stdlib/public/Distributed/DistributedAssertions.swift @@ -51,11 +51,11 @@ extension DistributedActor { return } - let unownedExecutor = self.unownedExecutor - let expectationCheck = _taskIsCurrentExecutor(unownedExecutor._executor) + let unownedExecutor = unsafe self.unownedExecutor + let expectationCheck = unsafe _taskIsCurrentExecutor(unownedExecutor._executor) precondition(expectationCheck, - "Incorrect actor executor assumption; Expected '\(self.unownedExecutor)' executor. \(message())", + unsafe "Incorrect actor executor assumption; Expected '\(unsafe self.unownedExecutor)' executor. \(message())", file: file, line: line) } } @@ -99,9 +99,9 @@ extension DistributedActor { return } - let unownedExecutor = self.unownedExecutor - guard _taskIsCurrentExecutor(unownedExecutor._executor) else { - let msg = "Incorrect actor executor assumption; Expected '\(unownedExecutor)' executor. \(message())" + let unownedExecutor = unsafe self.unownedExecutor + guard unsafe _taskIsCurrentExecutor(unownedExecutor._executor) else { + let msg = unsafe "Incorrect actor executor assumption; Expected '\(unsafe unownedExecutor)' executor. \(message())" /// TODO: implement the logic in-place perhaps rather than delegating to precondition()? assertionFailure(msg, file: file, line: line) // short-cut so we get the exact same failure reporting semantics return @@ -165,8 +165,8 @@ extension DistributedActor { fatalError("Cannot assume to be 'isolated \(Self.self)' since distributed actor '\(self)' is a remote actor reference.") } - let unownedExecutor = self.unownedExecutor - guard _taskIsCurrentExecutor(unownedExecutor._executor) else { + let unownedExecutor = unsafe self.unownedExecutor + guard unsafe _taskIsCurrentExecutor(unownedExecutor._executor) else { // TODO: offer information which executor we actually got when fatalError("Incorrect actor executor assumption; Expected same executor as \(self).", file: file, line: line) } @@ -174,7 +174,7 @@ extension DistributedActor { // To do the unsafe cast, we have to pretend it's @escaping. return try withoutActuallyEscaping(operation) { (_ fn: @escaping YesActor) throws -> T in - let rawFn = unsafeBitCast(fn, to: NoActor.self) + let rawFn = unsafe unsafeBitCast(fn, to: NoActor.self) return try rawFn(self) } } diff --git a/stdlib/public/Distributed/DistributedDefaultExecutor.swift b/stdlib/public/Distributed/DistributedDefaultExecutor.swift index abe0aca19c8..14e27adfe8b 100644 --- a/stdlib/public/Distributed/DistributedDefaultExecutor.swift +++ b/stdlib/public/Distributed/DistributedDefaultExecutor.swift @@ -18,7 +18,7 @@ import _Concurrency internal final class DistributedRemoteActorReferenceExecutor: SerialExecutor { static let _shared: DistributedRemoteActorReferenceExecutor = DistributedRemoteActorReferenceExecutor() static var sharedUnownedExecutor: UnownedSerialExecutor { - UnownedSerialExecutor(ordinary: _shared) + unsafe UnownedSerialExecutor(ordinary: _shared) } internal init() {} @@ -38,7 +38,7 @@ internal final class DistributedRemoteActorReferenceExecutor: SerialExecutor { #endif // !SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY public func asUnownedSerialExecutor() -> UnownedSerialExecutor { - UnownedSerialExecutor(ordinary: self) + unsafe UnownedSerialExecutor(ordinary: self) } } @@ -57,5 +57,5 @@ internal final class DistributedRemoteActorReferenceExecutor: SerialExecutor { public func buildDefaultDistributedRemoteActorExecutor( _ actor: Act ) -> UnownedSerialExecutor where Act: DistributedActor { - return DistributedRemoteActorReferenceExecutor.sharedUnownedExecutor + return unsafe DistributedRemoteActorReferenceExecutor.sharedUnownedExecutor } diff --git a/stdlib/public/Distributed/DistributedMetadata.swift b/stdlib/public/Distributed/DistributedMetadata.swift index b42a4ed4457..b3d3db230dd 100644 --- a/stdlib/public/Distributed/DistributedMetadata.swift +++ b/stdlib/public/Distributed/DistributedMetadata.swift @@ -19,8 +19,8 @@ import Swift public // SPI Distributed func _getParameterCount(mangledMethodName name: String) -> Int32 { let nameUTF8 = Array(name.utf8) - return nameUTF8.withUnsafeBufferPointer { nameUTF8 in - return __getParameterCount( + return unsafe nameUTF8.withUnsafeBufferPointer { nameUTF8 in + return unsafe __getParameterCount( nameUTF8.baseAddress!, UInt(nameUTF8.endIndex)) } } @@ -47,8 +47,8 @@ func _getParameterTypeInfo( into typesBuffer: Builtin.RawPointer, length typesLength: Int ) -> Int32 { let nameUTF8 = Array(name.utf8) - return nameUTF8.withUnsafeBufferPointer { nameUTF8 in - return __getParameterTypeInfo( + return unsafe nameUTF8.withUnsafeBufferPointer { nameUTF8 in + return unsafe __getParameterTypeInfo( nameUTF8.baseAddress!, UInt(nameUTF8.endIndex), genericEnv, genericArguments, typesBuffer, typesLength) } @@ -75,8 +75,8 @@ func _getReturnTypeInfo( genericArguments: UnsafeRawPointer? ) -> Any.Type? { let nameUTF8 = Array(name.utf8) - return nameUTF8.withUnsafeBufferPointer { nameUTF8 in - return __getReturnTypeInfo(nameUTF8.baseAddress!, UInt(nameUTF8.endIndex), + return unsafe nameUTF8.withUnsafeBufferPointer { nameUTF8 in + return unsafe __getReturnTypeInfo(nameUTF8.baseAddress!, UInt(nameUTF8.endIndex), genericEnv, genericArguments) } } diff --git a/stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift b/stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift index 33c208bff38..cab15d5677e 100644 --- a/stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift +++ b/stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift @@ -236,6 +236,7 @@ public struct LocalTestingDistributedActorSystemError: DistributedActorSystemErr // === lock ---------------------------------------------------------------- @available(SwiftStdlib 5.7, *) +@safe fileprivate class _Lock { #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS) private let underlying: UnsafeMutablePointer @@ -252,7 +253,7 @@ fileprivate class _Lock { init() { #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS) self.underlying = UnsafeMutablePointer.allocate(capacity: 1) - self.underlying.initialize(to: os_unfair_lock()) + unsafe self.underlying.initialize(to: os_unfair_lock()) #elseif os(Windows) self.underlying = UnsafeMutablePointer.allocate(capacity: 1) InitializeSRWLock(self.underlying) @@ -280,8 +281,8 @@ fileprivate class _Lock { #endif #if !os(WASI) - self.underlying.deinitialize(count: 1) - self.underlying.deallocate() + unsafe self.underlying.deinitialize(count: 1) + unsafe self.underlying.deallocate() #endif } @@ -289,7 +290,7 @@ fileprivate class _Lock { @discardableResult func withLock(_ body: () -> T) -> T { #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS) - os_unfair_lock_lock(self.underlying) + unsafe os_unfair_lock_lock(self.underlying) #elseif os(Windows) AcquireSRWLockExclusive(self.underlying) #elseif os(WASI) @@ -302,7 +303,7 @@ fileprivate class _Lock { defer { #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS) - os_unfair_lock_unlock(self.underlying) + unsafe os_unfair_lock_unlock(self.underlying) #elseif os(Windows) ReleaseSRWLockExclusive(self.underlying) #elseif os(WASI)