Merge pull request swiftlang#76042 from xedin/rdar-133415157

[Sema/SILGen] Emit dynamic actor isolation checks for closures

Author: xedin

include/swift/AST/Expr.h

@@ -266,7 +266,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
     Kind : 2
   );
 
-  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1,
+  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1,
     /// True if closure parameters were synthesized from anonymous closure
     /// variables.
     HasAnonymousClosureVars : 1,
@@ -288,7 +288,13 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
 
     /// True if we're in the common case where the GlobalActorAttributeRequest
     /// request returned a pair of null pointers.
-    NoGlobalActorAttribute : 1
+    NoGlobalActorAttribute : 1,
+
+    /// Indicates whether this closure literal would require dynamic actor 
+    /// isolation checks when it either specifies or inherits isolation
+    /// and was passed as an argument to a function that is not fully 
+    /// concurrency checked.
+    RequiresDynamicIsolationChecking : 1
   );
 
   SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
@@ -4158,6 +4164,7 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.InheritActorContext = false;
     Bits.ClosureExpr.IsPassedToSendingParameter = false;
     Bits.ClosureExpr.NoGlobalActorAttribute = false;
+    Bits.ClosureExpr.RequiresDynamicIsolationChecking = false;
   }
 
   SourceRange getSourceRange() const;
@@ -4223,6 +4230,16 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.IsPassedToSendingParameter = value;
   }
 
+  /// True if this is an isolated closure literal that is passed 
+  /// to a callee that has not been concurrency checked.
+  bool requiresDynamicIsolationChecking() const {
+    return Bits.ClosureExpr.RequiresDynamicIsolationChecking;
+  }
+
+  void setRequiresDynamicIsolationChecking(bool value = true) {
+    Bits.ClosureExpr.RequiresDynamicIsolationChecking = value;
+  }
+
   /// Determine whether this closure expression has an
   /// explicitly-specified result type.
   bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

include/swift/AST/Types.h

@@ -3874,9 +3874,6 @@ struct ParameterListInfo {
   /// actor context from the context in which it was created.
   bool inheritsActorContext(unsigned paramIdx) const;
 
-  /// Whether there is any contextual information set on this parameter list.
-  bool anyContextualInfo() const;
-
   bool isVariadicGenericParameter(unsigned paramIdx) const;
 
   /// Returns true if this is a sending parameter.

lib/AST/Type.cpp

@@ -1379,10 +1379,6 @@ bool ParameterListInfo::inheritsActorContext(unsigned paramIdx) const {
       : false;
 }
 
-bool ParameterListInfo::anyContextualInfo() const {
-  return implicitSelfCapture.any() || inheritActorContext.any();
-}
-
 bool ParameterListInfo::isVariadicGenericParameter(unsigned paramIdx) const {
   return paramIdx < variadicGenerics.size()
       ? variadicGenerics[paramIdx]

lib/SILGen/SILGenConcurrency.cpp

@@ -25,6 +25,15 @@
 using namespace swift;
 using namespace Lowering;
 
+static void loadExpectedExecutorForLocalVar(SILGenFunction &SGF, VarDecl *var) {
+  auto loc = RegularLocation::getAutoGeneratedLocation(SGF.F.getLocation());
+  Type actorType = var->getTypeInContext();
+  RValue actorInstanceRV =
+      SGF.emitRValueForDecl(loc, var, actorType, AccessSemantics::Ordinary);
+  ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
+  SGF.ExpectedExecutor = SGF.emitLoadActorExecutor(loc, actorInstance);
+}
+
 void SILGenFunction::emitExpectedExecutor() {
   // Whether the given declaration context is nested within an actor's
   // destructor.
@@ -80,17 +89,6 @@ void SILGenFunction::emitExpectedExecutor() {
   // which are allowed to be available on OSes where concurrency is not
   // available. rdar://106827064
 
-  // Local function to load the expected executor from a local actor
-  auto loadExpectedExecutorForLocalVar = [&](VarDecl *var) {
-    auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
-    Type actorType = var->getTypeInContext();
-    RValue actorInstanceRV = emitRValueForDecl(
-        loc, var, actorType, AccessSemantics::Ordinary);
-    ManagedValue actorInstance =
-        std::move(actorInstanceRV).getScalarValue();
-    ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
-  };
-
   if (auto *funcDecl =
         dyn_cast_or_null<AbstractFunctionDecl>(FunctionDC->getAsDecl())) {
     auto actorIsolation = getActorIsolation(funcDecl);
@@ -112,7 +110,7 @@ void SILGenFunction::emitExpectedExecutor() {
           (wantDataRaceChecks && funcDecl->isLocalCapture())) {
         auto loweredCaptures = SGM.Types.getLoweredLocalCaptures(SILDeclRef(funcDecl));
         if (auto isolatedParam = loweredCaptures.getIsolatedParamCapture()) {
-          loadExpectedExecutorForLocalVar(isolatedParam);
+          loadExpectedExecutorForLocalVar(*this, isolatedParam);
         } else {
           auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
           ManagedValue actorArg;
@@ -131,7 +129,7 @@ void SILGenFunction::emitExpectedExecutor() {
                 actorIsolation.getActorInstanceParameter() - 1;
             auto param = funcDecl->getParameters()->get(isolatedParamIdx);
             assert(param->isIsolated());
-            loadExpectedExecutorForLocalVar(param);
+            loadExpectedExecutorForLocalVar(*this, param);
           }
         }
       }
@@ -159,7 +157,8 @@ void SILGenFunction::emitExpectedExecutor() {
 
     case ActorIsolation::ActorInstance: {
       if (wantExecutor) {
-        loadExpectedExecutorForLocalVar(actorIsolation.getActorInstance());
+        loadExpectedExecutorForLocalVar(*this,
+                                        actorIsolation.getActorInstance());
       }
       break;
     }
@@ -610,6 +609,47 @@ static bool isCheckExpectedExecutorIntrinsicAvailable(SILGenModule &SGM) {
   return true;
 }
 
+void SILGenFunction::emitExpectedExecutorPreconditionForClosure() {
+  auto closure = dyn_cast_or_null<ClosureExpr>(FunctionDC);
+  if (!closure)
+    return;
+
+  auto &ctx = closure->getASTContext();
+  if (!ctx.LangOpts.isDynamicActorIsolationCheckingEnabled())
+    return;
+
+  // Asynchronous closures would hop to the expected executor.
+  if (F.isAsync() || !closure->requiresDynamicIsolationChecking())
+    return;
+
+  auto actorIsolation = closure->getActorIsolation();
+  switch (actorIsolation) {
+  case ActorIsolation::Unspecified:
+  case ActorIsolation::Nonisolated:
+  case ActorIsolation::NonisolatedUnsafe:
+    break;
+
+  case ActorIsolation::Erased:
+    llvm_unreachable("closure cannot have erased isolation");
+
+  case ActorIsolation::ActorInstance: {
+    loadExpectedExecutorForLocalVar(*this, actorIsolation.getActorInstance());
+    break;
+  }
+
+  case ActorIsolation::GlobalActor:
+    ExpectedExecutor =
+        emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
+    break;
+  }
+
+  if (ExpectedExecutor) {
+    emitPreconditionCheckExpectedExecutor(
+        RegularLocation::getAutoGeneratedLocation(F.getLocation()),
+        ExpectedExecutor);
+  }
+}
+
 void SILGenFunction::emitPreconditionCheckExpectedExecutor(
     SILLocation loc, ActorIsolation isolation,
     std::optional<ManagedValue> actorSelf) {

lib/SILGen/SILGenFunction.h

@@ -1186,6 +1186,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   void emitPreconditionCheckExpectedExecutor(
       SILLocation loc, SILValue executor);
 
+  /// Emit a precondition check to ensure that the closure is executing in
+  /// the expected isolation context.
+  void emitExpectedExecutorPreconditionForClosure();
+
   /// Gets a reference to the current executor for the task.
   /// \returns a value of type Builtin.Executor
   SILValue emitGetCurrentExecutor(SILLocation loc);

lib/SILGen/SILGenProlog.cpp

@@ -1381,6 +1381,7 @@ void SILGenFunction::emitProlog(
   }
 
   emitExpectedExecutor();
+  emitExpectedExecutorPreconditionForClosure();
 
   // IMPORTANT: This block should be the last one in `emitProlog`,
   // since it terminates BB and no instructions should be insterted after it.

lib/Sema/CSApply.cpp

@@ -58,6 +58,11 @@
 using namespace swift;
 using namespace constraints;
 
+static bool isClosureLiteralExpr(Expr *expr) {
+  expr = expr->getSemanticsProvidingExpr();
+  return (isa<CaptureListExpr>(expr) || isa<ClosureExpr>(expr));
+}
+
 bool Solution::hasFixedType(TypeVariableType *typeVar) const {
   auto knownBinding = typeBindings.find(typeVar);
   return knownBinding != typeBindings.end();
@@ -5963,24 +5968,29 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
 /// Apply the contextually Sendable flag to the given expression,
 static void applyContextualClosureFlags(Expr *expr, bool implicitSelfCapture,
                                         bool inheritActorContext,
-                                        bool isPassedToSendingParameter) {
+                                        bool isPassedToSendingParameter,
+                                        bool requiresDynamicIsolationChecking) {
   if (auto closure = dyn_cast<ClosureExpr>(expr)) {
     closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
     closure->setInheritsActorContext(inheritActorContext);
     closure->setIsPassedToSendingParameter(isPassedToSendingParameter);
+    closure->setRequiresDynamicIsolationChecking(
+        requiresDynamicIsolationChecking);
     return;
   }
 
   if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
     applyContextualClosureFlags(captureList->getClosureBody(),
                                 implicitSelfCapture, inheritActorContext,
-                                isPassedToSendingParameter);
+                                isPassedToSendingParameter,
+                                requiresDynamicIsolationChecking);
   }
 
   if (auto identity = dyn_cast<IdentityExpr>(expr)) {
     applyContextualClosureFlags(identity->getSubExpr(), implicitSelfCapture,
                                 inheritActorContext,
-                                isPassedToSendingParameter);
+                                isPassedToSendingParameter,
+                                requiresDynamicIsolationChecking);
   }
 }
 
@@ -6084,10 +6094,48 @@ ArgumentList *ExprRewriter::coerceCallArguments(
         return placeholder;
       };
 
+  bool closuresRequireDynamicIsolationChecking = [&]() {
+    auto *decl = callee.getDecl();
+    // If this is something like `{ @MainActor in ... }()`, let's consider
+    // callee as concurrency checked.
+    if (!decl)
+      return false;
+
+    if (auto declaredIn = decl->findImport(dc))
+      return !declaredIn->module.importedModule->isConcurrencyChecked();
+
+    // Both the caller and the allee are in the same module.
+    if (dc->getParentModule() == decl->getModuleContext()) {
+      return !dc->getASTContext().isSwiftVersionAtLeast(6);
+    }
+
+    // If we cannot figure out where the callee came from, let's conservatively
+    // assume that closure arguments require dynamic isolation checks.
+    return true;
+  }();
+
+  auto applyFlagsToArgument = [&paramInfo,
+                               &closuresRequireDynamicIsolationChecking](
+                                  unsigned paramIdx, Expr *argument) {
+    if (!isClosureLiteralExpr(argument))
+      return;
+
+    bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
+    bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
+    bool isPassedToSendingParameter = paramInfo.isSendingParameter(paramIdx);
+
+    applyContextualClosureFlags(argument, isImplicitSelfCapture,
+                                inheritsActorContext,
+                                isPassedToSendingParameter,
+                                closuresRequireDynamicIsolationChecking);
+  };
+
   // Quickly test if any further fix-ups for the argument types are necessary.
   auto matches = args->matches(params, [&](Expr *E) { return cs.getType(E); });
-  if (matches && !shouldInjectWrappedValuePlaceholder &&
-      !paramInfo.anyContextualInfo()) {
+  if (matches && !shouldInjectWrappedValuePlaceholder) {
+    for (unsigned paramIdx : indices(params)) {
+      applyFlagsToArgument(paramIdx, args->getExpr(paramIdx));
+    }
     return args;
   }
 
@@ -6204,15 +6252,10 @@ ArgumentList *ExprRewriter::coerceCallArguments(
     // for things like trailing closures and args to property wrapper params.
     arg.setLabel(param.getLabel());
 
-    // Determine whether the closure argument should be treated as having
-    // implicit self capture or inheriting actor context.
-    bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
-    bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
-    bool isPassedToSendingParameter = paramInfo.isSendingParameter(paramIdx);
-
-    applyContextualClosureFlags(argExpr, isImplicitSelfCapture,
-                                inheritsActorContext,
-                                isPassedToSendingParameter);
+    // Determine whether the argument should be marked as having
+    // implicit self capture, inheriting actor context, is passed to a
+    // `sending` parameter etc.
+    applyFlagsToArgument(paramIdx, argExpr);
 
     // If the types exactly match, this is easy.
     auto paramType = param.getOldType();
@@ -6356,11 +6399,6 @@ ArgumentList *ExprRewriter::coerceCallArguments(
   return ArgumentList::createTypeChecked(ctx, args, newArgs);
 }
 
-static bool isClosureLiteralExpr(Expr *expr) {
-  expr = expr->getSemanticsProvidingExpr();
-  return (isa<CaptureListExpr>(expr) || isa<ClosureExpr>(expr));
-}
-
 /// Whether the given expression is a closure that should inherit
 /// the actor context from where it was formed.
 static bool closureInheritsActorContext(Expr *expr) {