Merge pull request swiftlang#34210 from jckarter/async-await-sil-verifier

jckarter · web-flow · commit aef8f503d88a · 2020-10-19T15:06:55.000-07:00
SIL: Verify invariants of async_continuation instructions.
diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h
@@ -514,6 +514,10 @@ class SILInstruction
            "Operand does not belong to a SILInstruction");
     return isTypeDependentOperand(Op.getOperandNumber());
   }
+      
+  /// Returns true if evaluation of this instruction may cause suspension of an
+  /// async task.
+  bool maySuspend() const;
 
 private:
   /// Predicate used to filter OperandValueRange.
@@ -3039,12 +3043,34 @@ class KeyPathPattern final
   }
 };
 
-/// Accesses the continuation for an async task, to prepare a primitive suspend operation.
+/// Base class for instructions that access the continuation of an async task,
+/// in order to set up a suspension.
 /// The continuation must be consumed by an AwaitAsyncContinuation instruction locally,
 /// and must dynamically be resumed exactly once during the program's ensuing execution.
+class GetAsyncContinuationInstBase
+    : public SingleValueInstruction
+{
+protected:
+  using SingleValueInstruction::SingleValueInstruction;
+  
+public:
+  /// Get the type of the value the async task receives on a resume.
+  CanType getFormalResumeType() const;
+  SILType getLoweredResumeType() const;
+  
+  /// True if the continuation can be used to resume the task by throwing an error.
+  bool throws() const;
+  
+  static bool classof(const SILNode *I) {
+    return I->getKind() >= SILNodeKind::First_GetAsyncContinuationInstBase &&
+           I->getKind() <= SILNodeKind::Last_GetAsyncContinuationInstBase;
+  }
+};
+
+/// Accesses the continuation for an async task, to prepare a primitive suspend operation.
 class GetAsyncContinuationInst final
     : public InstructionBase<SILInstructionKind::GetAsyncContinuationInst,
-                             SingleValueInstruction>
+                             GetAsyncContinuationInstBase>
 {
   friend SILBuilder;
   
@@ -3054,14 +3080,6 @@ class GetAsyncContinuationInst final
   {}
   
 public:
-  
-  /// Get the type of the value the async task receives on a resume.
-  CanType getFormalResumeType() const;
-  SILType getLoweredResumeType() const;
-  
-  /// True if the continuation can be used to resume the task by throwing an error.
-  bool throws() const;
-  
   ArrayRef<Operand> getAllOperands() const { return {}; }
   MutableArrayRef<Operand> getAllOperands() { return {}; }
 };
@@ -3074,23 +3092,14 @@ class GetAsyncContinuationInst final
 /// buffer that receives the incoming value when the continuation is resumed.
 class GetAsyncContinuationAddrInst final
     : public UnaryInstructionBase<SILInstructionKind::GetAsyncContinuationAddrInst,
-                                  SingleValueInstruction>
+                                  GetAsyncContinuationInstBase>
 {
   friend SILBuilder;
   GetAsyncContinuationAddrInst(SILDebugLocation Loc,
                                SILValue Operand,
                                SILType ContinuationTy)
     : UnaryInstructionBase(Loc, Operand, ContinuationTy)
   {}
-  
-public:
-  
-  /// Get the type of the value the async task receives on a resume.
-  CanType getFormalResumeType() const;
-  SILType getLoweredResumeType() const;
-  
-  /// True if the continuation can be used to resume the task by throwing an error.
-  bool throws() const;
 };
 
 /// Instantiates a key path object.
diff --git a/include/swift/SIL/SILNode.h b/include/swift/SIL/SILNode.h
@@ -452,7 +452,7 @@ class alignas(8) SILNode {
   /// If this is a SILArgument or a SILInstruction get its parent module,
   /// otherwise return null.
   SILModule *getModule() const;
-
+  
   /// Pretty-print the node.  If the node is an instruction, the output
   /// will be valid SIL assembly; otherwise, it will be an arbitrary
   /// format suitable for debugging.
diff --git a/include/swift/SIL/SILNodes.def b/include/swift/SIL/SILNodes.def
@@ -719,10 +719,12 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
   // be tightened, though we want to be careful that passes that try to do
   // code motion or eliminate this instruction don't do so without awareness of
   // its structural requirements.
-  SINGLE_VALUE_INST(GetAsyncContinuationInst, get_async_continuation,
-                    SingleValueInstruction, MayHaveSideEffects, MayRelease)
-  SINGLE_VALUE_INST(GetAsyncContinuationAddrInst, get_async_continuation_addr,
-                    SingleValueInstruction, MayHaveSideEffects, MayRelease)
+  ABSTRACT_SINGLE_VALUE_INST(GetAsyncContinuationInstBase, SingleValueInstruction)
+    SINGLE_VALUE_INST(GetAsyncContinuationInst, get_async_continuation,
+                      GetAsyncContinuationInstBase, MayHaveSideEffects, MayRelease)
+    SINGLE_VALUE_INST(GetAsyncContinuationAddrInst, get_async_continuation_addr,
+                      GetAsyncContinuationInstBase, MayHaveSideEffects, MayRelease)
+    SINGLE_VALUE_INST_RANGE(GetAsyncContinuationInstBase, GetAsyncContinuationInst, GetAsyncContinuationAddrInst)
 
   // Key paths
   // TODO: The only "side effect" is potentially retaining the returned key path
diff --git a/lib/SIL/IR/SILInstruction.cpp b/lib/SIL/IR/SILInstruction.cpp
@@ -1519,6 +1519,21 @@ MultipleValueInstruction *MultipleValueInstructionResult::getParent() {
   return reinterpret_cast<MultipleValueInstruction *>(value);
 }
 
+/// Returns true if evaluation of this node may cause suspension of an
+/// async task.
+bool SILInstruction::maySuspend() const {
+  // await_async_continuation always suspends the current task.
+  if (isa<AwaitAsyncContinuationInst>(this))
+    return true;
+  
+  // Fully applying an async function may suspend the caller.
+  if (auto applySite = FullApplySite::isa(const_cast<SILInstruction*>(this))) {
+    return applySite.getOrigCalleeType()->isAsync();
+  }
+  
+  return false;
+}
+
 #ifndef NDEBUG
 
 //---
@@ -1536,7 +1551,7 @@ MultipleValueInstruction *MultipleValueInstructionResult::getParent() {
 // Check that all subclasses of MultipleValueInstructionResult are the same size
 // as MultipleValueInstructionResult.
 //
-// If this changes, we just need to expand the size fo SILInstructionResultArray
+// If this changes, we just need to expand the size of SILInstructionResultArray
 // to contain a stride. But we assume this now so we should enforce it.
 #define MULTIPLE_VALUE_INST_RESULT(ID, PARENT)                                 \
   static_assert(                                                               \
diff --git a/lib/SIL/IR/SILInstructions.cpp b/lib/SIL/IR/SILInstructions.cpp
@@ -2931,12 +2931,12 @@ DestructureTupleInst *DestructureTupleInst::create(const SILFunction &F,
       DestructureTupleInst(M, Loc, Operand, Types, OwnershipKinds);
 }
 
-CanType GetAsyncContinuationInst::getFormalResumeType() const {
+CanType GetAsyncContinuationInstBase::getFormalResumeType() const {
   // The resume type is the type argument to the continuation type.
   return getType().castTo<BoundGenericType>().getGenericArgs()[0];
 }
 
-SILType GetAsyncContinuationInst::getLoweredResumeType() const {
+SILType GetAsyncContinuationInstBase::getLoweredResumeType() const {
   // The lowered resume type is the maximally-abstracted lowering of the
   // formal resume type.
   auto formalType = getFormalResumeType();
@@ -2945,29 +2945,8 @@ SILType GetAsyncContinuationInst::getLoweredResumeType() const {
   return M.Types.getLoweredType(AbstractionPattern::getOpaque(), formalType, c);
 }
 
-bool GetAsyncContinuationInst::throws() const {
+bool GetAsyncContinuationInstBase::throws() const {
   // The continuation throws if it's an UnsafeThrowingContinuation
   return getType().castTo<BoundGenericType>()->getDecl()
     == getFunction()->getASTContext().getUnsafeThrowingContinuationDecl();
 }
-
-CanType GetAsyncContinuationAddrInst::getFormalResumeType() const {
-  // The resume type is the type argument to the continuation type.
-  return getType().castTo<BoundGenericType>().getGenericArgs()[0];
-}
-
-SILType GetAsyncContinuationAddrInst::getLoweredResumeType() const {
-  // The lowered resume type is the maximally-abstracted lowering of the
-  // formal resume type.
-  auto formalType = getFormalResumeType();
-  auto &M = getFunction()->getModule();
-  auto c = getFunction()->getTypeExpansionContext();
-  return M.Types.getLoweredType(AbstractionPattern::getOpaque(), formalType, c);
-}
-
-bool GetAsyncContinuationAddrInst::throws() const {
-  // The continuation throws if it's an UnsafeThrowingContinuation
-  return getType().castTo<BoundGenericType>()->getDecl()
-    == getFunction()->getASTContext().getUnsafeThrowingContinuationDecl();
-}
-
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -662,7 +662,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
   // Used for dominance checking within a basic block.
   llvm::DenseMap<const SILInstruction *, unsigned> InstNumbers;
-
+  
   DeadEndBlocks DEBlocks;
   LoadBorrowNeverInvalidatedAnalysis loadBorrowNeverInvalidatedAnalysis;
   bool SingleFunction = true;
@@ -4748,6 +4748,72 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                     "Type of witness instruction does not match actual type of "
                     "witnessed function");
   }
+  
+  void checkGetAsyncContinuationInstBase(GetAsyncContinuationInstBase *GACI) {
+    auto resultTy = GACI->getType();
+    auto &C = resultTy.getASTContext();
+    auto resultBGT = resultTy.getAs<BoundGenericType>();
+    require(resultBGT, "Instruction type must be a continuation type");
+    auto resultDecl = resultBGT->getDecl();
+    require(resultDecl == C.getUnsafeContinuationDecl()
+             || resultDecl == C.getUnsafeThrowingContinuationDecl(),
+            "Instruction type must be a continuation type");
+  }
+  
+  void checkGetAsyncContinuationInst(GetAsyncContinuationInst *GACI) {
+    checkGetAsyncContinuationInstBase(GACI);
+  }
+  
+  void checkGetAsyncContinuationAddrInst(GetAsyncContinuationAddrInst *GACI) {
+    checkGetAsyncContinuationInstBase(GACI);
+
+    requireSameType(GACI->getOperand()->getType(),
+                    GACI->getLoweredResumeType().getAddressType(),
+                    "Operand type must match continuation resume type");
+  }
+  
+  void checkAwaitAsyncContinuationInst(AwaitAsyncContinuationInst *AACI) {
+    // The operand must be a GetAsyncContinuation* instruction.
+    auto cont = dyn_cast<GetAsyncContinuationInstBase>(AACI->getOperand());
+    require(cont, "can only await the result of a get_async_continuation instruction");
+    bool isAddressForm = isa<GetAsyncContinuationAddrInst>(cont);
+
+    auto &C = cont->getType().getASTContext();
+    
+    // The shape of the successors depends on the continuation instruction being
+    // awaited.
+    require((bool)AACI->getErrorBB() == cont->throws(),
+            "must have an error successor if and only if the continuation is throwing");
+    if (cont->throws()) {
+      require(AACI->getErrorBB()->getNumArguments() == 1,
+              "error successor must take one argument");
+      auto arg = AACI->getErrorBB()->getArgument(0);
+      auto errorType = C.getErrorDecl()->getDeclaredType()->getCanonicalType();
+      requireSameType(arg->getType(),
+                      SILType::getPrimitiveObjectType(errorType),
+              "error successor argument must have Error type");
+      
+      if (AACI->getFunction()->hasOwnership()) {
+        require(arg->getOwnershipKind() == ValueOwnershipKind::Owned,
+                "error successor argument must be owned");
+      }
+    }
+    if (isAddressForm) {
+      require(AACI->getResumeBB()->getNumArguments() == 0,
+              "resume successor must take no arguments for get_async_continuation_addr");
+    } else {
+      require(AACI->getResumeBB()->getNumArguments() == 1,
+              "resume successor must take one argument for get_async_continuation");
+      auto arg = AACI->getResumeBB()->getArgument(0);
+
+      requireSameType(arg->getType(), cont->getLoweredResumeType(),
+                      "resume successor must take an argument of the continuation resume type");
+      if (AACI->getFunction()->hasOwnership()) {
+        require(arg->getOwnershipKind() == ValueOwnershipKind::Owned,
+                "resume successor argument must be owned");
+      }
+    }
+  }
 
   // This verifies that the entry block of a SIL function doesn't have
   // any predecessors and also verifies the entry point arguments.
@@ -4902,13 +4968,17 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       std::set<SILInstruction*> ActiveOps;
 
       CFGState CFG = Normal;
+      
+      GetAsyncContinuationInstBase *GotAsyncContinuation = nullptr;
     };
   };
 
   /// Verify the various control-flow-sensitive rules of SIL:
   ///
   /// - stack allocations and deallocations must obey a stack discipline
   /// - accesses must be uniquely ended
+  /// - async continuations must be awaited before getting the continuation again, suspending
+  ///  the task, or exiting the function
   /// - flow-sensitive states must be equivalent on all paths into a block
   void verifyFlowSensitiveRules(SILFunction *F) {
     // Do a traversal of the basic blocks.
@@ -4924,6 +4994,20 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       for (SILInstruction &i : *BB) {
         CurInstruction = &i;
 
+        if (i.maySuspend()) {
+          // Instructions that may suspend an async context must not happen
+          // while the continuation is being accessed, with the exception of
+          // the AwaitAsyncContinuationInst that completes suspending the task.
+          if (auto aaci = dyn_cast<AwaitAsyncContinuationInst>(&i)) {
+            require(state.GotAsyncContinuation == aaci->getOperand(),
+                    "encountered await_async_continuation that doesn't match active gotten continuation");
+            state.GotAsyncContinuation = nullptr;
+          } else {
+            require(!state.GotAsyncContinuation,
+                    "cannot suspend async task while unawaited continuation is active");
+          }
+        }
+          
         if (i.isAllocatingStack()) {
           state.Stack.push_back(cast<SingleValueInstruction>(&i));
 
@@ -4946,13 +5030,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
             bool present = state.ActiveOps.erase(beginOp);
             require(present, "operation has already been ended");
           }
-
+        } else if (auto gaci = dyn_cast<GetAsyncContinuationInstBase>(&i)) {
+          require(!state.GotAsyncContinuation,
+                  "get_async_continuation while unawaited continuation is already active");
+          state.GotAsyncContinuation = gaci;
         } else if (auto term = dyn_cast<TermInst>(&i)) {
           if (term->isFunctionExiting()) {
             require(state.Stack.empty(),
                     "return with stack allocs that haven't been deallocated");
             require(state.ActiveOps.empty(),
                     "return with operations still active");
+            require(!state.GotAsyncContinuation,
+                    "return with unawaited async continuation");
 
             if (isa<UnwindInst>(term)) {
               require(state.CFG == VerifyFlowSensitiveRulesDetails::YieldUnwind,
@@ -4970,12 +5059,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               }
             }
           }
-
+          
           if (isa<YieldInst>(term)) {
             require(state.CFG != VerifyFlowSensitiveRulesDetails::YieldOnceResume,
                     "encountered multiple 'yield's along single path");
             require(state.CFG == VerifyFlowSensitiveRulesDetails::Normal,
                     "encountered 'yield' on abnormal CFG path");
+            require(!state.GotAsyncContinuation,
+                    "encountered 'yield' while an unawaited continuation is active");
           }
 
           auto successors = term->getSuccessors();
@@ -5037,6 +5128,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                     "inconsistent active-operations sets entering basic block");
             require(state.CFG == foundState.CFG,
                     "inconsistent coroutine states entering basic block");
+            require(state.GotAsyncContinuation == foundState.GotAsyncContinuation,
+                    "inconsistent active async continuations entering basic block");
           }
         }
       }
