Merge pull request #2354 from swiftwasm/main

[pull] swiftwasm from main

Author: pull[bot]

include/swift/AST/DiagnosticsSema.def

@@ -4210,6 +4210,13 @@ ERROR(actor_isolated_self_independent_context,none,
         "actor-isolated %0 %1 can not be referenced from an "
         "'@actorIndependent' context",
         (DescriptiveDeclKind, DeclName))
+ERROR(actor_isolated_inout_state,none,
+      "actor-isolated %0 %1 cannot be passed 'inout' to"
+      "%select{| implicitly}2 'async' function call",
+      (DescriptiveDeclKind, DeclName, bool))
+ERROR(actor_isolated_mutating_func,none,
+      "cannot call mutating async function %0 on actor-isolated %1 %2",
+      (DeclName, DescriptiveDeclKind, DeclName))
 ERROR(actor_isolated_global_actor_context,none,
       "actor-isolated %0 %1 can not be referenced from context of global "
       "actor %2",

include/swift/AST/IRGenOptions.h

@@ -143,6 +143,9 @@ struct PointerAuthOptions : clang::PointerAuthOptions {
 
   /// The function to call to resume running in the parent context.
   PointerAuthSchema AsyncContextResume;
+
+  /// The resume function stored in AsyncTask.
+  PointerAuthSchema TaskResumeFunction;
 };
 
 enum class JITDebugArtifact : unsigned {

lib/IRGen/GenFunc.cpp

@@ -2462,7 +2462,16 @@ IRGenFunction::createAsyncDispatchFn(const FunctionPointer &fnPtr,
 
 void IRGenFunction::emitSuspensionPoint(llvm::Value *toExecutor,
                                         llvm::Value *asyncResume) {
-  // TODO: pointerauth
+                                        
+  llvm::Value *task = getAsyncTask();
+  llvm::Value *callableResume = asyncResume;
+                                        
+  if (auto schema = IGM.getOptions().PointerAuth.TaskResumeFunction) {
+    auto *resumeAddr = Builder.CreateStructGEP(task, 4);
+    auto authInfo = PointerAuthInfo::emit(
+        *this, schema, resumeAddr, PointerAuthEntity());
+    callableResume = emitPointerAuthSign(*this, asyncResume, authInfo);
+  }
 
   // Setup the suspend point.
   SmallVector<llvm::Value *, 8> arguments;
@@ -2474,10 +2483,10 @@ void IRGenFunction::emitSuspensionPoint(llvm::Value *toExecutor,
   arguments.push_back(
       Builder.CreateBitOrPointerCast(suspendFn, IGM.Int8PtrTy));
 
-  arguments.push_back(asyncResume);
+  arguments.push_back(callableResume);
   arguments.push_back(
       Builder.CreateBitOrPointerCast(toExecutor, getAsyncExecutor()->getType()));
-  arguments.push_back(getAsyncTask());
+  arguments.push_back(task);
   arguments.push_back(getAsyncExecutor());
   arguments.push_back(getAsyncContext());
 

lib/IRGen/IRGen.cpp

@@ -699,6 +699,10 @@ static void setPointerAuthOptions(PointerAuthOptions &opts,
   opts.AsyncContextResume =
       PointerAuthSchema(codeKey, /*address*/ true, Discrimination::Constant,
                         SpecialPointerAuthDiscriminators::AsyncContextResume);
+
+  opts.TaskResumeFunction =
+      PointerAuthSchema(codeKey, /*address*/ true, Discrimination::Constant,
+                        SpecialPointerAuthDiscriminators::TaskResumeFunction);
 }
 
 std::unique_ptr<llvm::TargetMachine>

lib/Sema/CSBindings.cpp

@@ -865,7 +865,9 @@ ConstraintSystem::getPotentialBindingForRelationalConstraint(
 
   // If the type we'd be binding to is a dependent member, don't try to
   // resolve this type variable yet.
-  if (type->is<DependentMemberType>()) {
+  if (type->getWithoutSpecifierType()
+          ->lookThroughAllOptionalTypes()
+          ->is<DependentMemberType>()) {
     SmallVector<TypeVariableType *, 4> referencedVars;
     type->getTypeVariables(referencedVars);
 
@@ -1210,15 +1212,19 @@ Optional<Type> ConstraintSystem::checkTypeOfBinding(TypeVariableType *typeVar,
     }
   }
 
-  // If the type is a type variable itself, don't permit the binding.
-  if (type->getRValueType()->is<TypeVariableType>())
-    return None;
+  {
+    auto objType = type->getWithoutSpecifierType();
 
-  // Don't bind to a dependent member type, even if it's currently
-  // wrapped in any number of optionals, because binding producer
-  // might unwrap and try to attempt it directly later.
-  if (type->lookThroughAllOptionalTypes()->is<DependentMemberType>())
-    return None;
+    // If the type is a type variable itself, don't permit the binding.
+    if (objType->is<TypeVariableType>())
+      return None;
+
+    // Don't bind to a dependent member type, even if it's currently
+    // wrapped in any number of optionals, because binding producer
+    // might unwrap and try to attempt it directly later.
+    if (objType->lookThroughAllOptionalTypes()->is<DependentMemberType>())
+      return None;
+  }
 
   // Okay, allow the binding (with the simplified type).
   return type;

lib/Sema/TypeCheckConcurrency.cpp

@@ -611,6 +611,24 @@ findMemberReference(Expr *expr) {
   return None;
 }
 
+/// Return true if the callee of an ApplyExpr is async
+///
+/// Note that this must be called after the implicitlyAsync flag has been set,
+/// or implicitly async calls will not return the correct value.
+static bool isAsyncCall(const ApplyExpr *call) {
+  if (call->implicitlyAsync())
+    return true;
+
+  // Effectively the same as doing a
+  // `cast_or_null<FunctionType>(call->getFn()->getType())`, check the
+  // result of that and then checking `isAsync` if it's defined.
+  Type funcTypeType = call->getFn()->getType();
+  if (!funcTypeType)
+    return false;
+  FunctionType *funcType = funcTypeType->castTo<FunctionType>();
+  return funcType->isAsync();
+}
+
 namespace {
   /// Check for adherence to the actor isolation rules, emitting errors
   /// when actor-isolated declarations are used in an unsafe manner.
@@ -679,6 +697,11 @@ namespace {
         return { true, expr };
       }
 
+      if (auto inout = dyn_cast<InOutExpr>(expr)) {
+        if (!applyStack.empty())
+          diagnoseInOutArg(applyStack.back(), inout, false);
+      }
+
       if (auto lookup = dyn_cast<LookupExpr>(expr)) {
         checkMemberReference(lookup->getBase(), lookup->getMember(),
                              lookup->getLoc());
@@ -720,11 +743,22 @@ namespace {
         Expr *fn = call->getFn()->getValueProvidingExpr();
         if (auto memberRef = findMemberReference(fn)) {
           checkMemberReference(
-              call->getArg(), memberRef->first, memberRef->second, 
+              call->getArg(), memberRef->first, memberRef->second,
               /*isEscapingPartialApply=*/false, /*maybeImplicitAsync=*/true);
 
           call->getArg()->walk(*this);
 
+          if (applyStack.size() >= 2) {
+            ApplyExpr *outerCall = applyStack[applyStack.size() - 2];
+            if (isAsyncCall(outerCall)) {
+              // This call is a partial application within an async call.
+              // If the partial application take a value inout, it is bad.
+              if (InOutExpr *inoutArg = dyn_cast<InOutExpr>(
+                      call->getArg()->getSemanticsProvidingExpr()))
+                diagnoseInOutArg(outerCall, inoutArg, true);
+            }
+          }
+
           // manual clean-up since normal traversal is skipped
           assert(applyStack.back() == dyn_cast<ApplyExpr>(expr));
           applyStack.pop_back();
@@ -860,6 +894,59 @@ namespace {
       return true;
     }
 
+    /// Diagnose an inout argument passed into an async call
+    ///
+    /// \returns true if we diagnosed the entity, \c false otherwise.
+    bool diagnoseInOutArg(const ApplyExpr *call, const InOutExpr *arg,
+                          bool isPartialApply) {
+      // check that the call is actually async
+      if (!isAsyncCall(call))
+        return false;
+
+      Expr *subArg = arg->getSubExpr();
+      if (LookupExpr *baseArg = dyn_cast<LookupExpr>(subArg)) {
+        while (LookupExpr *nextLayer = dyn_cast<LookupExpr>(baseArg->getBase()))
+          baseArg = nextLayer;
+        // subArg: the actual property being passed inout
+        // baseArg: the property in the actor who's property is being passed
+        // inout
+
+        auto memberDecl = baseArg->getMember().getDecl();
+        auto isolation = ActorIsolationRestriction::forDeclaration(memberDecl);
+        switch (isolation) {
+        case ActorIsolationRestriction::Unrestricted:
+        case ActorIsolationRestriction::LocalCapture:
+        case ActorIsolationRestriction::Unsafe:
+        case ActorIsolationRestriction::GlobalActor: // TODO: handle global
+                                                     // actors
+          break;
+        case ActorIsolationRestriction::ActorSelf: {
+          if (isPartialApply) {
+            // The partially applied InoutArg is a property of actor. This can
+            // really only happen when the property is a struct with a mutating
+            // async method.
+            if (auto partialApply = dyn_cast<ApplyExpr>(call->getFn())) {
+              ValueDecl *fnDecl =
+                  cast<DeclRefExpr>(partialApply->getFn())->getDecl();
+              ctx.Diags.diagnose(
+                  call->getLoc(), diag::actor_isolated_mutating_func,
+                  fnDecl->getName(), memberDecl->getDescriptiveKind(),
+                  memberDecl->getName());
+              return true;
+            }
+          } else {
+            ctx.Diags.diagnose(
+                subArg->getLoc(), diag::actor_isolated_inout_state,
+                memberDecl->getDescriptiveKind(), memberDecl->getName(),
+                call->implicitlyAsync());
+            return true;
+          }
+        }
+        }
+      }
+      return false;
+    }
+
     /// Get the actor isolation of the innermost relevant context.
     ActorIsolation getInnermostIsolatedContext(const DeclContext *constDC) {
       // Retrieve the actor isolation for a declaration somewhere in our
@@ -1196,7 +1283,9 @@ namespace {
         llvm_unreachable("Locals cannot be referenced with member syntax");
 
       case ActorIsolationRestriction::Unsafe:
-        return diagnoseReferenceToUnsafe(member, memberLoc);
+        // This case is hit when passing actor state inout to functions in some
+        // cases. The error is emitted by diagnoseInOutArg.
+        return false;
       }
       llvm_unreachable("unhandled actor isolation kind!");
     }

test/Concurrency/actor_inout_isolation.swift

@@ -0,0 +1,133 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+// REQUIRES: concurrency
+
+// Verify that we don't allow actor-isolated state to be passed via inout
+// Check:
+//  - can't pass it into a normal async function
+//  - can't pass it into a first-class async function as a value
+//  - can't pass it into another actor method
+//  - can't pass it into a curried/partially applied function
+//  - can't pass it inout to a function that doesn't directly touch it
+//  - can't pass it into a function that was passed into the calling method
+//  - can't call async mutating function on actor isolated state
+
+struct Point {
+  var x: Int
+  var y: Int
+
+  mutating func setComponents(x: inout Int, y: inout Int) async {
+    defer { (x, y) = (self.x, self.y) }
+    (self.x, self.y) = (x, y)
+  }
+}
+
+actor class TestActor {
+  var position = Point(x: 0, y: 0)
+  var nextPosition = Point(x: 0, y: 1)
+  var value1: Int = 0
+  var value2: Int = 1
+}
+
+func modifyAsynchronously(_ foo: inout Int) async { foo += 1 }
+let modifyAsyncValue = modifyAsynchronously
+
+// external function call
+extension TestActor {
+
+  // Can't pass actor-isolated primitive into a function
+  func inoutAsyncFunctionCall() async {
+    // expected-error@+1{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    await modifyAsynchronously(&value1)
+  }
+
+  func inoutAsyncClosureCall() async {
+    // expected-error@+1{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    await { (_ foo: inout Int) async in foo += 1 }(&value1)
+  }
+
+  // Can't pass actor-isolated primitive into first-class function value
+  func inoutAsyncValueCall() async {
+    // expected-error@+1{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    await modifyAsyncValue(&value1)
+  }
+
+  // Can't pass property of actor-isolated state inout to async function
+  func inoutPropertyStateValueCall() async {
+    // expected-error@+1{{actor-isolated property 'position' cannot be passed 'inout' to 'async' function call}}
+    await modifyAsynchronously(&position.x)
+  }
+}
+
+// internal method call
+extension TestActor {
+  func modifyByValue(_ other: inout Int) async {
+    other += value1
+  }
+
+  func passStateIntoMethod() async {
+    // expected-error@+1{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    await modifyByValue(&value1)
+  }
+}
+
+// external class method call
+class NonAsyncClass {
+  func modifyOtherAsync(_ other : inout Int) async {
+    // ...
+  }
+
+  func modifyOtherNotAsync(_ other: inout Int) {
+    // ...
+  }
+}
+
+// Calling external class/struct async function
+extension TestActor {
+  // Can't pass state into async method of another class
+
+  func passStateIntoDifferentClassMethod() async {
+    let other = NonAsyncClass()
+    let otherCurry = other.modifyOtherAsync
+    // expected-error@+1{{actor-isolated property 'value2' cannot be passed 'inout' to 'async' function call}}
+    await other.modifyOtherAsync(&value2)
+    // expected-error@+1{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    await otherCurry(&value1)
+    other.modifyOtherNotAsync(&value2) // This is okay since it's not async!
+
+  }
+
+  func callMutatingFunctionOnStruct() async {
+    // expected-error@+3:20{{cannot call mutating async function 'setComponents(x:y:)' on actor-isolated property 'position'}}
+    // expected-error@+2:51{{actor-isolated property 'nextPosition' cannot be passed 'inout' to 'async' function call}}
+    // expected-error@+1:71{{actor-isolated property 'nextPosition' cannot be passed 'inout' to 'async' function call}}
+    await position.setComponents(x: &nextPosition.x, y: &nextPosition.y)
+
+    // expected-error@+3:20{{cannot call mutating async function 'setComponents(x:y:)' on actor-isolated property 'position'}}
+    // expected-error@+2:38{{actor-isolated property 'value1' cannot be passed 'inout' to 'async' function call}}
+    // expected-error@+1:50{{actor-isolated property 'value2' cannot be passed 'inout' to 'async' function call}}
+    await position.setComponents(x: &value1, y: &value2)
+  }
+}
+
+// Check implicit async testing
+actor class DifferentActor {
+  func modify(_ state: inout Int) {}
+}
+
+extension TestActor {
+  func modify(_ state: inout Int) {}
+
+  // Actor state passed inout to implicitly async function on an actor of the
+  // same type
+  func modifiedByOtherTestActor(_ other: TestActor) async {
+    //expected-error@+1{{actor-isolated property 'value2' cannot be passed 'inout' to implicitly 'async' function call}}
+    await other.modify(&value2)
+  }
+
+  // Actor state passed inout to an implicitly async function on an actor of a
+  // different type
+  func modifiedByOther(_ other: DifferentActor) async {
+    //expected-error@+1{{actor-isolated property 'value2' cannot be passed 'inout' to implicitly 'async' function call}}
+    await other.modify(&value2)
+  }
+}

test/DebugInfo/async-args.swift

@@ -1,5 +1,6 @@
 // RUN: %target-swift-frontend %s -emit-ir -g -o - \
 // RUN:    -module-name M -enable-experimental-concurrency | %FileCheck %s
+// REQUIRES: concurrency
 
 func use<T>(_ t: T) {}
 func forceSplit() async {