[region-isolation] Add 'inout sending' diagnostics.

Specifically:

1. We error now if one transfers an 'inout sending' parameter and does not
reinitialize it before the end of the function.

2. We error now if one merges an 'inout sending' parameter into an actor
isolated region and do not reinitialize it with a non-actor isolated value
before the end of the function.

rdar://126303739
(cherry picked from commit 6fe7496)

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -939,6 +939,9 @@ NOTE(sil_referencebinding_inout_binding_here, none,
 
 NOTE(regionbasedisolation_maybe_race, none,
      "access can happen concurrently", ())
+NOTE(regionbasedisolation_inout_sending_must_be_reinitialized, none,
+     "'inout sending' parameter must be reinitialized before function exit with a non-actor isolated value",
+     ())
 ERROR(regionbasedisolation_unknown_pattern, none,
       "pattern that the region based isolation checker does not understand how to check. Please file a bug",
       ())
@@ -968,6 +971,9 @@ ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
 NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
      "value is %0 since it is in the same region as %1",
      (StringRef, DeclBaseName))
+NOTE(regionbasedisolation_isolated_since_in_same_region_string, none,
+     "%0 is %1since it is in the same region as %2",
+     (Identifier, StringRef, Identifier))
 
 //===---
 // New Transfer Non Sendable Diagnostics
@@ -977,6 +983,13 @@ ERROR(regionbasedisolation_named_transfer_yields_race, none,
       "sending %0 risks causing data races",
       (Identifier))
 
+ERROR(regionbasedisolation_inout_sending_cannot_be_actor_isolated, none,
+      "'inout sending' parameter %0 cannot be %1at end of function",
+      (Identifier, StringRef))
+NOTE(regionbasedisolation_inout_sending_cannot_be_actor_isolated_note, none,
+      "%1%0 risks causing races in between %1uses and caller uses since caller assumes value is not actor isolated",
+      (Identifier, StringRef))
+
 NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
      "sending %1%0 to %2 callee risks causing data races between %2 and local %3 uses",
      (Identifier, StringRef, ActorIsolation, ActorIsolation))

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -410,6 +410,15 @@ enum class PartitionOpKind : uint8_t {
   ///
   /// This is used if we need to reject the program and do not want to assert.
   UnknownPatternError,
+
+  /// Require that a 'inout sending' parameter's region is not transferred and
+  /// disconnected at a specific function exiting term inst.
+  ///
+  /// This ensures that if users transfer away an inout sending parameter, the
+  /// parameter is reinitialized with a disconnected value.
+  ///
+  /// Takes one parameter, the inout parameter that we need to check.
+  RequireInOutSendingAtFunctionExit,
 };
 
 /// PartitionOp represents a primitive operation that can be performed on
@@ -494,6 +503,12 @@ class PartitionOp {
     return PartitionOp(PartitionOpKind::UnknownPatternError, elt, sourceInst);
   }
 
+  static PartitionOp
+  RequireInOutSendingAtFunctionExit(Element elt, SILInstruction *sourceInst) {
+    return PartitionOp(PartitionOpKind::RequireInOutSendingAtFunctionExit, elt,
+                       sourceInst);
+  }
+
   bool operator==(const PartitionOp &other) const {
     return opKind == other.opKind && opArgs == other.opArgs &&
            source == other.source;
@@ -925,6 +940,21 @@ struct PartitionOpEvaluator {
                                                    isolationRegionInfo);
   }
 
+  /// Call our CRTP subclass.
+  void handleInOutSendingNotInitializedAtExitError(
+      const PartitionOp &op, Element elt, Operand *transferringOp) const {
+    return asImpl().handleInOutSendingNotInitializedAtExitError(op, elt,
+                                                                transferringOp);
+  }
+
+  /// Call our CRTP subclass.
+  void handleInOutSendingNotDisconnectedAtExitError(
+      const PartitionOp &op, Element elt,
+      SILDynamicMergedIsolationInfo isolation) const {
+    return asImpl().handleInOutSendingNotDisconnectedAtExitError(op, elt,
+                                                                 isolation);
+  }
+
   /// Call isActorDerived on our CRTP subclass.
   bool isActorDerived(Element elt) const {
     return asImpl().isActorDerived(elt);
@@ -959,8 +989,10 @@ struct PartitionOpEvaluator {
   }
 
   /// Overload of \p getIsolationRegionInfo without an Operand.
-  SILIsolationInfo getIsolationRegionInfo(Region region) const {
-    return getIsolationRegionInfo(region, nullptr).first;
+  SILDynamicMergedIsolationInfo getIsolationRegionInfo(Region region) const {
+    if (auto opt = getIsolationRegionInfo(region, nullptr))
+      return opt->first;
+    return SILDynamicMergedIsolationInfo();
   }
 
   bool isTaskIsolatedDerived(Element elt) const {
@@ -1150,6 +1182,41 @@ struct PartitionOpEvaluator {
         }
       }
       return;
+    case PartitionOpKind::RequireInOutSendingAtFunctionExit: {
+      assert(op.getOpArgs().size() == 1 &&
+             "Require PartitionOp should be passed 1 argument");
+      assert(p.isTrackingElement(op.getOpArgs()[0]) &&
+             "Require PartitionOp's argument should already be tracked");
+
+      // First check if the region of our 'inout sending' element has been
+      // transferred. In that case, we emit a special use after free error.
+      if (auto *transferredOperandSet = p.getTransferred(op.getOpArgs()[0])) {
+        for (auto transferredOperand : transferredOperandSet->data()) {
+          handleInOutSendingNotInitializedAtExitError(op, op.getOpArgs()[0],
+                                                      transferredOperand);
+        }
+        return;
+      }
+
+      // If we were not transferred, check if our region is actor isolated. If
+      // so, error since we need a disconnected value in the inout parameter.
+      Region inoutSendingRegion = p.getRegion(op.getOpArgs()[0]);
+      auto dynamicRegionIsolation = getIsolationRegionInfo(inoutSendingRegion);
+
+      // If we failed to merge emit an unknown pattern error so we fail.
+      if (!dynamicRegionIsolation) {
+        handleUnknownCodePattern(op);
+        return;
+      }
+
+      // Otherwise, emit the error if the dynamic region isolation is not
+      // disconnected.
+      if (!dynamicRegionIsolation.isDisconnected()) {
+        handleInOutSendingNotDisconnectedAtExitError(op, op.getOpArgs()[0],
+                                                     dynamicRegionIsolation);
+      }
+      return;
+    }
     case PartitionOpKind::UnknownPatternError:
       // Begin tracking the specified element in case we have a later use.
       p.trackNewElement(op.getOpArgs()[0]);
@@ -1337,6 +1404,16 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
   /// to our user so that we can emit that error as we process.
   void handleUnknownCodePattern(const PartitionOp &op) const {}
 
+  /// Called if we find an 'inout sending' parameter that is not live at exit.
+  void handleInOutSendingNotInitializedAtExitError(
+      const PartitionOp &op, Element elt, Operand *transferringOp) const {}
+
+  /// Called if we find an 'inout sending' parameter that is live at excit but
+  /// is actor isolated instead of disconnected.
+  void handleInOutSendingNotDisconnectedAtExitError(
+      const PartitionOp &op, Element elt,
+      SILDynamicMergedIsolationInfo actorIsolation) const {}
+
   /// This is used to determine if an element is actor derived. If we determine
   /// that a region containing such an element is transferred, we emit an error
   /// since actor regions cannot be transferred.

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -81,6 +81,12 @@ class ActorInstance {
     return value.getPointer();
   }
 
+  SILValue maybeGetValue() const {
+    if (getKind() != Kind::Value)
+      return SILValue();
+    return getValue();
+  }
+
   bool isValue() const { return getKind() == Kind::Value; }
 
   bool isAccessorInit() const { return getKind() == Kind::ActorAccessorInit; }
@@ -389,7 +395,7 @@ class SILDynamicMergedIsolationInfo {
 
 public:
   SILDynamicMergedIsolationInfo() : innerInfo() {}
-  SILDynamicMergedIsolationInfo(SILIsolationInfo innerInfo)
+  explicit SILDynamicMergedIsolationInfo(SILIsolationInfo innerInfo)
       : innerInfo(innerInfo) {}
 
   /// Returns nullptr only if both this isolation info and \p other are actor
@@ -404,6 +410,8 @@ class SILDynamicMergedIsolationInfo {
 
   operator bool() const { return bool(innerInfo); }
 
+  SILIsolationInfo *operator->() { return &innerInfo; }
+
   SILIsolationInfo getIsolationInfo() const { return innerInfo; }
 
   bool isDisconnected() const { return innerInfo.isDisconnected(); }
@@ -412,6 +420,12 @@ class SILDynamicMergedIsolationInfo {
     return innerInfo.hasSameIsolation(other);
   }
 
+  static SILDynamicMergedIsolationInfo
+  getDisconnected(bool isUnsafeNonIsolated) {
+    return SILDynamicMergedIsolationInfo(
+        SILIsolationInfo::getDisconnected(isUnsafeNonIsolated));
+  }
+
   SWIFT_DEBUG_DUMP { innerInfo.dump(); }
 
   void printForDiagnostics(llvm::raw_ostream &os) const {

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1196,6 +1196,14 @@ struct PartitionOpBuilder {
         PartitionOp::Require(lookupValueID(value), currentInst));
   }
 
+  void addRequireInOutSendingAtFunctionExit(SILValue value) {
+    assert(valueHasID(value, /*dumpIfHasNoID=*/true) &&
+           "required value should already have been encountered");
+    currentInstPartitionOps.emplace_back(
+        PartitionOp::RequireInOutSendingAtFunctionExit(lookupValueID(value),
+                                                       currentInst));
+  }
+
   void addUnknownPatternError(SILValue value) {
     if (AbortOnUnknownPatternMatchError) {
       llvm::report_fatal_error(
@@ -1594,9 +1602,9 @@ class PartitionOpTranslator {
     // compiler exits successfully, actor merge errors could not have happened.
     std::optional<SILDynamicMergedIsolationInfo> mergedInfo;
     if (resultIsolationInfoOverride) {
-      mergedInfo = resultIsolationInfoOverride;
+      mergedInfo = SILDynamicMergedIsolationInfo(resultIsolationInfoOverride);
     } else {
-      mergedInfo = SILIsolationInfo::getDisconnected(false);
+      mergedInfo = SILDynamicMergedIsolationInfo::getDisconnected(false);
     }
 
     for (SILValue src : sourceValues) {
@@ -2296,6 +2304,22 @@ class PartitionOpTranslator {
 #define INST(INST, PARENT) TranslationSemantics visit##INST(INST *inst);
 #include "swift/SIL/SILNodes.def"
 
+  /// Adds requires for all sending inout parameters to make sure that they are
+  /// properly updated before the end of the function.
+  void addRequiresForInOutParameters(TermInst *inst) {
+    assert(inst->isFunctionExiting() && "Must be function exiting term inst?!");
+    for (auto *arg : inst->getFunction()->getArguments()) {
+      auto *fArg = cast<SILFunctionArgument>(arg);
+      if (fArg->getArgumentConvention().isInoutConvention() &&
+          fArg->getKnownParameterInfo().hasOption(SILParameterInfo::Sending)) {
+        if (auto ns = tryToTrackValue(arg)) {
+          auto rep = ns->getRepresentative().getValue();
+          builder.addRequireInOutSendingAtFunctionExit(rep);
+        }
+      }
+    }
+  }
+
   /// Top level switch that translates SIL instructions.
   void translateSILInstruction(SILInstruction *inst) {
     builder.reset(inst);
@@ -2684,12 +2708,8 @@ CONSTANT_TRANSLATION(CondFailInst, Ignored)
 // function_ref/class_method which are considered sendable.
 CONSTANT_TRANSLATION(SwitchValueInst, Ignored)
 CONSTANT_TRANSLATION(UnreachableInst, Ignored)
-CONSTANT_TRANSLATION(UnwindInst, Ignored)
-// Doesn't take a parameter.
-CONSTANT_TRANSLATION(ThrowAddrInst, Ignored)
 
 // Terminators that only need require.
-CONSTANT_TRANSLATION(ThrowInst, Require)
 CONSTANT_TRANSLATION(SwitchEnumAddrInst, Require)
 CONSTANT_TRANSLATION(YieldInst, Require)
 
@@ -2699,6 +2719,33 @@ CONSTANT_TRANSLATION(CondBranchInst, TerminatorPhi)
 CONSTANT_TRANSLATION(CheckedCastBranchInst, TerminatorPhi)
 CONSTANT_TRANSLATION(DynamicMethodBranchInst, TerminatorPhi)
 
+// Function exiting terminators.
+//
+// We handle these especially since we want to make sure that inout parameters
+// that are transferred are forced to be reinitialized.
+//
+// There is an assert in TermInst::isFunctionExiting that makes sure we do this
+// correctly.
+//
+// NOTE: We purposely do not require reinitialization along paths that end in
+// unreachable.
+#ifdef FUNCTION_EXITING_TERMINATOR_TRANSLATION
+#error "FUNCTION_EXITING_TERMINATOR_TRANSLATION already defined?!"
+#endif
+
+#define FUNCTION_EXITING_TERMINATOR_CONSTANT(INST, Kind)                       \
+  TranslationSemantics PartitionOpTranslator::visit##INST(INST *inst) {        \
+    assert(inst->isFunctionExiting() && "Must be function exiting?!");         \
+    addRequiresForInOutParameters(inst);                                       \
+    return TranslationSemantics::Kind;                                         \
+  }
+
+FUNCTION_EXITING_TERMINATOR_CONSTANT(UnwindInst, Ignored)
+FUNCTION_EXITING_TERMINATOR_CONSTANT(ThrowAddrInst, Ignored)
+FUNCTION_EXITING_TERMINATOR_CONSTANT(ThrowInst, Require)
+
+#undef FUNCTION_EXITING_TERMINATOR_CONSTANT
+
 // Today, await_async_continuation just takes Sendable values
 // (UnsafeContinuation and UnsafeThrowingContinuation).
 CONSTANT_TRANSLATION(AwaitAsyncContinuationInst, AssertingIfNonSendable)
@@ -2897,6 +2944,7 @@ PartitionOpTranslator::visitLoadBorrowInst(LoadBorrowInst *lbi) {
 }
 
 TranslationSemantics PartitionOpTranslator::visitReturnInst(ReturnInst *ri) {
+  addRequiresForInOutParameters(ri);
   if (ri->getFunction()->getLoweredFunctionType()->hasSendingResult()) {
     return TranslationSemantics::TransferringNoResult;
   }