Skip to content

Commit 99f9c31

Browse files
Xazax-hunGabor Horvath
authored andcommitted
[6.2][StrictMemorySafety] Check the safety of return types of calls
Explanation: There were some scenarios where we could call an unsafe function without marking the expression as unsafe. These affect mostly cases where the function's result is passed to another function or returned. This PR makes sure we always flag functions with unsafe return types, even if their result is not stored anywhere for later use. Issues: rdar://157237301 Original PRs: #83520 Risk: Low, worst case scenario the user has to add redundant unsafe keywords in strict memory safe mode. Testing: Added a compiler test. Reviewers: @DougGregor
1 parent fe48800 commit 99f9c31

40 files changed

+126
-98
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8505,7 +8505,7 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
85058505
// All other pointers are considered unsafe.
85068506
return true;
85078507
}
8508-
8508+
85098509
// Handle records recursively.
85108510
if (auto recordDecl = clangType->getAsTagDecl()) {
85118511
// If we reached this point the types is not imported as a shared reference,
@@ -8582,7 +8582,7 @@ ClangDeclExplicitSafety::evaluate(Evaluator &evaluator,
85828582
if (hasUnsafeType(evaluator, field->getType()))
85838583
return ExplicitSafety::Unsafe;
85848584
}
8585-
8585+
85868586
// Okay, call it safe.
85878587
return ExplicitSafety::Safe;
85888588
}

lib/Sema/TypeCheckUnsafe.cpp

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,12 @@
2121
#include "TypeCheckUnsafe.h"
2222

2323
#include "swift/AST/ASTContext.h"
24-
#include "swift/AST/Effects.h"
25-
#include "swift/AST/UnsafeUse.h"
2624
#include "swift/AST/DiagnosticsSema.h"
25+
#include "swift/AST/Effects.h"
2726
#include "swift/AST/PackConformance.h"
2827
#include "swift/AST/SourceFile.h"
28+
#include "swift/AST/Types.h"
29+
#include "swift/AST/UnsafeUse.h"
2930

3031
using namespace swift;
3132

@@ -243,19 +244,29 @@ bool swift::enumerateUnsafeUses(ConcreteDeclRef declRef,
243244
// If the type of this declaration involves unsafe types, diagnose that.
244245
ASTContext &ctx = decl->getASTContext();
245246
auto subs = declRef.getSubstitutions();
246-
if (!skipTypeCheck) {
247-
auto type = decl->getInterfaceType();
248-
if (subs) {
249-
if (auto *genericFnType = type->getAs<GenericFunctionType>())
250-
type = genericFnType->substGenericArgs(subs);
251-
else
252-
type = type.subst(subs);
253-
}
247+
auto type = decl->getInterfaceType();
248+
if (subs) {
249+
if (auto *genericFnType = type->getAs<GenericFunctionType>())
250+
type = genericFnType->substGenericArgs(subs);
251+
else
252+
type = type.subst(subs);
253+
}
254254

255+
if (skipTypeCheck) {
256+
// We check the arguements instead of the funcion type for function calls.
257+
// On the other hand, we still need to check the return type as we might not
258+
// have a declRef with the type of the return type. E.g. in
259+
// `return funcionCall()`.
260+
if (auto *fnTy = type->getAs<AnyFunctionType>())
261+
type = fnTy->getResult();
262+
else
263+
type = Type();
264+
}
265+
if (type) {
255266
bool shouldReturnTrue = false;
256267
diagnoseUnsafeType(ctx, loc, type, [&](Type unsafeType) {
257268
if (fn(UnsafeUse::forReferenceToUnsafe(
258-
decl, isCall && !isa<ParamDecl>(decl), unsafeType, loc)))
269+
decl, isCall && !isa<ParamDecl>(decl), unsafeType, loc)))
259270
shouldReturnTrue = true;
260271
});
261272

stdlib/public/Concurrency/CheckedContinuation.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public struct CheckedContinuation<T, E: Error>: Sendable {
162162
/// the caller. The task continues executing when its executor is
163163
/// able to reschedule it.
164164
public func resume(returning value: sending T) {
165-
if let c: UnsafeContinuation<T, E> = canary.takeContinuation() {
165+
if let c: UnsafeContinuation<T, E> = unsafe canary.takeContinuation() {
166166
unsafe c.resume(returning: value)
167167
} else {
168168
#if !$Embedded
@@ -186,7 +186,7 @@ public struct CheckedContinuation<T, E: Error>: Sendable {
186186
/// the caller. The task continues executing when its executor is
187187
/// able to reschedule it.
188188
public func resume(throwing error: __owned E) {
189-
if let c: UnsafeContinuation<T, E> = canary.takeContinuation() {
189+
if let c: UnsafeContinuation<T, E> = unsafe canary.takeContinuation() {
190190
unsafe c.resume(throwing: error)
191191
} else {
192192
#if !$Embedded

stdlib/public/Concurrency/Executor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,7 @@ internal func _task_serialExecutor_getExecutorRef<E>(_ executor: E) -> Builtin.E
888888
@_silgen_name("_task_taskExecutor_getTaskExecutorRef")
889889
internal func _task_taskExecutor_getTaskExecutorRef<E>(_ taskExecutor: E) -> Builtin.Executor
890890
where E: TaskExecutor {
891-
return taskExecutor.asUnownedTaskExecutor().executor
891+
return unsafe taskExecutor.asUnownedTaskExecutor().executor
892892
}
893893

894894
// Used by the concurrency runtime

stdlib/public/Concurrency/PartialAsyncTask.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public struct ExecutorJob: Sendable, ~Copyable {
318318
/// Returns the result of executing the closure.
319319
@available(StdlibDeploymentTarget 6.2, *)
320320
public func withUnsafeExecutorPrivateData<R, E>(body: (UnsafeMutableRawBufferPointer) throws(E) -> R) throws(E) -> R {
321-
let base = _jobGetExecutorPrivateData(self.context)
321+
let base = unsafe _jobGetExecutorPrivateData(self.context)
322322
let size = unsafe 2 * MemoryLayout<OpaquePointer>.stride
323323
return unsafe try body(UnsafeMutableRawBufferPointer(start: base,
324324
count: size))
@@ -470,21 +470,21 @@ extension ExecutorJob {
470470

471471
/// Allocate a specified number of bytes of uninitialized memory.
472472
public func allocate(capacity: Int) -> UnsafeMutableRawBufferPointer {
473-
let base = _jobAllocate(context, capacity)
473+
let base = unsafe _jobAllocate(context, capacity)
474474
return unsafe UnsafeMutableRawBufferPointer(start: base, count: capacity)
475475
}
476476

477477
/// Allocate uninitialized memory for a single instance of type `T`.
478478
public func allocate<T>(as type: T.Type) -> UnsafeMutablePointer<T> {
479-
let base = _jobAllocate(context, MemoryLayout<T>.size)
479+
let base = unsafe _jobAllocate(context, MemoryLayout<T>.size)
480480
return unsafe base.bindMemory(to: type, capacity: 1)
481481
}
482482

483483
/// Allocate uninitialized memory for the specified number of
484484
/// instances of type `T`.
485485
public func allocate<T>(capacity: Int, as type: T.Type)
486486
-> UnsafeMutableBufferPointer<T> {
487-
let base = _jobAllocate(context, MemoryLayout<T>.stride * capacity)
487+
let base = unsafe _jobAllocate(context, MemoryLayout<T>.stride * capacity)
488488
let typedBase = unsafe base.bindMemory(to: T.self, capacity: capacity)
489489
return unsafe UnsafeMutableBufferPointer<T>(start: typedBase, count: capacity)
490490
}

stdlib/public/Concurrency/Task+PriorityEscalation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func __withTaskPriorityEscalationHandler0<T, E>(
125125
onPriorityEscalated handler0: @Sendable (UInt8, UInt8) -> Void,
126126
isolation: isolated (any Actor)? = #isolation
127127
) async throws(E) -> T {
128-
let record = _taskAddPriorityEscalationHandler(handler: handler0)
128+
let record = unsafe _taskAddPriorityEscalationHandler(handler: handler0)
129129
defer { unsafe _taskRemovePriorityEscalationHandler(record: record) }
130130

131131
return try await operation()

stdlib/public/Concurrency/Task+TaskExecutor.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ public func withTaskExecutorPreference<T, Failure>(
146146
}
147147

148148
let taskExecutorBuiltin: Builtin.Executor =
149-
taskExecutor.asUnownedTaskExecutor().executor
149+
unsafe taskExecutor.asUnownedTaskExecutor().executor
150150

151-
let record = _pushTaskExecutorPreference(taskExecutorBuiltin)
151+
let record = unsafe _pushTaskExecutorPreference(taskExecutorBuiltin)
152152
defer {
153153
unsafe _popTaskExecutorPreference(record: record)
154154
}
@@ -177,9 +177,9 @@ public func _unsafeInheritExecutor_withTaskExecutorPreference<T: Sendable>(
177177
}
178178

179179
let taskExecutorBuiltin: Builtin.Executor =
180-
taskExecutor.asUnownedTaskExecutor().executor
180+
unsafe taskExecutor.asUnownedTaskExecutor().executor
181181

182-
let record = _pushTaskExecutorPreference(taskExecutorBuiltin)
182+
let record = unsafe _pushTaskExecutorPreference(taskExecutorBuiltin)
183183
defer {
184184
unsafe _popTaskExecutorPreference(record: record)
185185
}

stdlib/public/Concurrency/Task.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ internal func _getCurrentTaskName() -> UnsafePointer<UInt8>?
984984

985985
@available(SwiftStdlib 6.2, *)
986986
internal func _getCurrentTaskNameString() -> String? {
987-
if let stringPtr = _getCurrentTaskName() {
987+
if let stringPtr = unsafe _getCurrentTaskName() {
988988
unsafe String(cString: stringPtr)
989989
} else {
990990
nil

stdlib/public/Concurrency/TaskCancellation.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public func withTaskCancellationHandler<T>(
7777
) async rethrows -> T {
7878
// unconditionally add the cancellation record to the task.
7979
// if the task was already cancelled, it will be executed right away.
80-
let record = _taskAddCancellationHandler(handler: handler)
80+
let record = unsafe _taskAddCancellationHandler(handler: handler)
8181
defer { unsafe _taskRemoveCancellationHandler(record: record) }
8282

8383
return try await operation()
@@ -98,7 +98,7 @@ public func _unsafeInheritExecutor_withTaskCancellationHandler<T>(
9898
) async rethrows -> T {
9999
// unconditionally add the cancellation record to the task.
100100
// if the task was already cancelled, it will be executed right away.
101-
let record = _taskAddCancellationHandler(handler: handler)
101+
let record = unsafe _taskAddCancellationHandler(handler: handler)
102102
defer { unsafe _taskRemoveCancellationHandler(record: record) }
103103

104104
return try await operation()

stdlib/public/Concurrency/TaskLocal.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public final class TaskLocal<Value: Sendable>: Sendable, CustomStringConvertible
178178
/// or if the task-local has no value bound, this will return the `defaultValue`
179179
/// of the task local.
180180
public func get() -> Value {
181-
guard let rawValue = _taskLocalValueGet(key: key) else {
181+
guard let rawValue = unsafe _taskLocalValueGet(key: key) else {
182182
return self.defaultValue
183183
}
184184

0 commit comments

Comments
 (0)