Merge pull request #85164 from gottesmm/pr-39de26ee629693f844665c42d2bdff56838838a0

[rbi] When translating a Store, make sure to require src even if the value being stored is Sendable

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -2982,7 +2982,7 @@ class PartitionOpTranslator {
   /// dest to src. If the \p dest could be aliased, then we must instead treat
   /// them as merges, to ensure any aliases of \p dest are also updated.
   void translateSILStore(Operand *dest, Operand *src,
-                          SILIsolationInfo resultIsolationInfoOverride = {}) {
+                         SILIsolationInfo resultIsolationInfoOverride = {}) {
     SILValue destValue = dest->get();
 
     if (auto destResult = tryToTrackValue(destValue)) {
@@ -3010,7 +3010,14 @@ class PartitionOpTranslator {
                                resultIsolationInfoOverride);
     }
 
-    // Stores to storage of non-Sendable type can be ignored.
+    // If we reached this point, our destination is something that is Sendable
+    // and is not an address that comes from a non-Sendable base... so we can
+    // ignore the store part. But we still need tosee if our src (which also
+    // must be Sendable) comes from a non-Sendable base. In such a case, we need
+    // to require that.
+    if (auto srcResult = tryToTrackValue(src->get())) {
+      builder.addRequire(*srcResult);
+    }
   }
 
   void translateSILTupleAddrConstructor(TupleAddrConstructorInst *inst) {

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -965,6 +965,19 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
     }
   }
 
+  if (auto *bbi = dyn_cast<BeginBorrowInst>(inst)) {
+    if (bbi->isFromVarDecl()) {
+      // See if we have the actual AST information on our instruction.
+      if (auto *varDecl = bbi->getLoc().getAsASTNode<VarDecl>()) {
+        auto isolation = swift::getActorIsolation(varDecl);
+        if (isolation.getKind() == ActorIsolation::NonisolatedUnsafe) {
+          return SILIsolationInfo::getDisconnected(
+              true /*is nonisolated(unsafe)*/);
+        }
+      }
+    }
+  }
+
   /// Consider non-Sendable metatypes to be task-isolated, so they cannot cross
   /// into another isolation domain.
   if (auto *mi = dyn_cast<MetatypeInst>(inst)) {

test/Concurrency/transfernonsendable.swift

@@ -2025,3 +2025,42 @@ func avoidThinkingClosureParameterIsSending() {
     }
   }
 }
+
+enum RequireSrcWhenStoringEvenWhenSendable {
+  func test<T: Sendable>(t: T) {
+    var result: T = t
+    Task { // expected-ni-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
+      // expected-ni-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      // expected-ni-ns-warning @-2 {{sending value of non-Sendable type '@concurrent () async -> ()' risks causing data races}}
+      // expected-ni-ns-note @-3 {{Passing value of non-Sendable type '@concurrent () async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      result = t
+    }
+    useValue(result) // expected-note {{access can happen concurrently}}
+  }
+
+  func test2() {
+    var result: Any = 0
+    Task { // expected-ni-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
+      // expected-ni-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      // expected-ni-ns-warning @-2 {{sending value of non-Sendable type '@concurrent () async -> ()' risks causing data races}}
+      // expected-ni-ns-note @-3 {{Passing value of non-Sendable type '@concurrent () async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      result = 0
+    }
+    useValue(result) // expected-note {{access can happen concurrently}}
+  }
+
+  protocol Initializable {
+    init()
+  }
+
+  func test3<T: Initializable & SendableMetatype>(type: T.Type) {
+    var result = type.init()
+    Task { // expected-ni-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
+      // expected-ni-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      // expected-ni-ns-warning @-2 {{sending value of non-Sendable type '@concurrent () async -> ()' risks causing data races}}
+      // expected-ni-ns-note @-3 {{Passing value of non-Sendable type '@concurrent () async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
+      result = type.init()
+    }
+    useValue(result) // expected-note {{access can happen concurrently}}
+  }
+}

test/Concurrency/transfernonsendable_nonisolatedunsafe.swift

@@ -329,6 +329,18 @@ func useAfterTransferLetSquelchedIndirectAddressOnly<T : ProvidesStaticValue>(_
   print(ns4)
 }
 
+func testNonisolatedUnsafeForwardDeclaredVar<T: Sendable>(_ body: @escaping @Sendable () async throws -> T) throws -> T {
+  nonisolated(unsafe) var result: Result<T, Error>!
+  Task {
+    do {
+      result = .success(try await body())
+    } catch {
+      result = .failure(error)
+    }
+  }
+  return try result.get()
+}
+
 ////////////////////////
 // MARK: Global Tests //
 ////////////////////////