Merge pull request swiftlang#76234 from gottesmm/pr-3cfcc173e99fd3e7aa08ddb66b5e28ac5b3e9704

[region-isolation] Do not allow for a disconnected value passed as an explicitly sent parameter to be reused.

Author: gottesmm

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1226,11 +1226,17 @@ struct PartitionOpEvaluator {
       }
 
       // Next see if we are disconnected and have the same isolation. In such a
-      // case, we do not transfer since the disconnected value is allowed to be
-      // resued after we return.
-      if (transferredRegionIsolation.isDisconnected() && calleeIsolationInfo &&
-          transferredRegionIsolation.hasSameIsolation(calleeIsolationInfo))
-        return;
+      // case, if we are not marked explicitly as sending, we do not transfer
+      // since the disconnected value is allowed to be resued after we
+      // return. If we are passed as a sending parameter, we cannot do this.
+      if (auto *sourceInst = Impl::getSourceInst(op)) {
+        if (auto fas = FullApplySite::isa(sourceInst);
+            (!fas || !fas.isSending(*op.getSourceOp())) &&
+            transferredRegionIsolation.isDisconnected() &&
+            calleeIsolationInfo &&
+            transferredRegionIsolation.hasSameIsolation(calleeIsolationInfo))
+          return;
+      }
 
       // Mark op.getOpArgs()[0] as transferred.
       TransferringOperandState &state = operandToStateMap.get(op.getSourceOp());
@@ -1578,6 +1584,9 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
 
   static SILLocation getLoc(SILInstruction *inst) { return inst->getLoc(); }
   static SILLocation getLoc(Operand *op) { return op->getUser()->getLoc(); }
+  static SILInstruction *getSourceInst(const PartitionOp &partitionOp) {
+    return partitionOp.getSourceInst();
+  }
   static SILIsolationInfo getIsolationInfo(const PartitionOp &partitionOp) {
     return SILIsolationInfo::get(partitionOp.getSourceInst());
   }

test/Concurrency/transfernonsendable_sending_params.swift

@@ -7,7 +7,9 @@
 // MARK: Declarations //
 ////////////////////////
 
-class NonSendableKlass {}
+class NonSendableKlass {
+  func use() {}
+}
 
 struct NonSendableStruct {
   var first = NonSendableKlass()
@@ -55,6 +57,9 @@ func throwingFunction() throws { fatalError() }
 func transferArg(_ x: sending NonSendableKlass) {
 }
 
+func transferArgAsync(_ x: sending NonSendableKlass) async {
+}
+
 func transferArgWithOtherParam(_ x: sending NonSendableKlass, _ y: NonSendableKlass) {
 }
 
@@ -559,3 +564,56 @@ extension MyActor {
     }
   }
 }
+
+// We would normally not error here since transferArg is nonisolated and c is
+// disconnected. Since c is passed as sending, we shouldn't squelch this.
+func disconnectedPassedSendingToNonIsolatedCallee(
+) async -> Void {
+    let c = NonSendableKlass()
+    transferArg(c) // expected-warning {{sending 'c' risks causing data races}}
+    // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
+    c.use() // expected-note {{access can happen concurrently}}
+}
+
+// We would normally not error here since transferArg is nonisolated and c is
+// disconnected. Since c is passed as sending, we shouldn't squelch this.
+func disconnectedPassedSendingToAsyncNonIsolatedCallee(
+) async -> Void {
+    let c = NonSendableKlass()
+    await transferArgAsync(c) // expected-warning {{sending 'c' risks causing data races}}
+    // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
+    c.use()  // expected-note {{access can happen concurrently}}
+}
+
+// We would normally not error here since transferArg is nonisolated and c is
+// disconnected. Since c is passed as sending, we shouldn't squelch this.
+func disconnectedPassedSendingToNonIsolatedCalleeIsolatedParam2(
+    isolation: isolated (any Actor)? = nil
+) async -> Void {
+    let c = NonSendableKlass()
+    transferArg(c) // expected-warning {{sending 'c' risks causing data races}}
+    // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
+    c.use() // expected-note {{access can happen concurrently}}
+}
+
+// We would normally not error here since transferArg is nonisolated and c is
+// disconnected. Since c is passed as sending, we shouldn't squelch this.
+func disconnectedPassedSendingToAsyncNonIsolatedCalleeIsolatedParam2(
+  isolation: isolated (any Actor)? = nil
+) async -> Void {
+    let c = NonSendableKlass()
+    await transferArgAsync(c) // expected-warning {{sending 'c' risks causing data races}}
+    // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
+    c.use() // expected-note {{access can happen concurrently}}
+}
+
+// We would normally not error here since transferArg is nonisolated and c is
+// disconnected. Since c is passed as sending, we shouldn't squelch this.
+func disconnectedPassedSendingToNonIsolatedCalleeIsolatedParam3(
+    isolation: isolated (any Actor)? = nil
+) -> Void {
+    let c = NonSendableKlass()
+    transferArg(c) // expected-warning {{sending 'c' risks causing data races}}
+    // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
+    c.use() // expected-note {{access can happen concurrently}}
+}

unittests/SILOptimizer/PartitionUtilsTest.cpp

@@ -62,6 +62,10 @@ struct MockedPartitionOpEvaluator final
     return {};
   }
 
+  static SILInstruction *getSourceInst(const PartitionOp &partitionOp) {
+    return nullptr;
+  }
+
   static bool doesFunctionHaveSendingResult(const PartitionOp &partitionOp) {
     return false;
   }
@@ -107,6 +111,10 @@ struct MockedPartitionOpEvaluatorWithFailureCallback final
   static SILIsolationInfo getIsolationInfo(const PartitionOp &partitionOp) {
     return {};
   }
+
+  static SILInstruction *getSourceInst(const PartitionOp &partitionOp) {
+    return nullptr;
+  }
 };
 
 } // namespace