[clang][pr55896]:co_yield/co_await thread-safety

[urnathan]

Summary

co_await and co_yield are represented by (classes derived from)
CoroutineSuspendExpr. That has a number of child nodes, not all of
which are used for code-generation. In particular the operand is
represented multiple times, and, like the problem with co_return
(55406) it must only be emitted in the CFG exactly once. The operand
also appears inside OpaqueValueExprs, but that's ok.

This adds a visitor for SuspendExprs to emit the required children in
the correct order. Note that this CFG is pre-coro xform. We don't
have initial or final suspend points.

[bruno]

LGTM, thanks for the detailed explanation/comments. One minor nitpick inline.

clang/lib/Analysis/CFG.cpp

3164

Remove the extra spaces after "."

[urnathan]

clang/lib/Analysis/CFG.cpp

3164

It's gonna take my fingers a long time to get that right -- particularly working in multiple code bases :)

clang/lib/Analysis/CFG.cpp

private:
   CFGBlock *VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *S);
   CFGBlock *VisitObjCAtThrowStmt(ObjCAtThrowStmt *S);
   CFGBlock *VisitObjCAtTryStmt(ObjCAtTryStmt *S);
   CFGBlock *VisitObjCAutoreleasePoolStmt(ObjCAutoreleasePoolStmt *S);
   CFGBlock *VisitObjCForCollectionStmt(ObjCForCollectionStmt *S);
   CFGBlock *VisitObjCMessageExpr(ObjCMessageExpr *E, AddStmtChoice asc);
   CFGBlock *VisitPseudoObjectExpr(PseudoObjectExpr *E);
   CFGBlock *VisitReturnStmt(Stmt *S);
+  CFGBlock *VisitCoroutineSuspendExpr(CoroutineSuspendExpr *S,
+                                      AddStmtChoice asc);
   CFGBlock *VisitSEHExceptStmt(SEHExceptStmt *S);
   CFGBlock *VisitSEHFinallyStmt(SEHFinallyStmt *S);
   CFGBlock *VisitSEHLeaveStmt(SEHLeaveStmt *S);
   CFGBlock *VisitSEHTryStmt(SEHTryStmt *S);
   CFGBlock *VisitStmtExpr(StmtExpr *S, AddStmtChoice asc);
   CFGBlock *VisitSwitchStmt(SwitchStmt *S);
   CFGBlock *VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
                                           AddStmtChoice asc);
switch (S->getStmtClass()) {
 
     case Stmt::PseudoObjectExprClass:
       return VisitPseudoObjectExpr(cast<PseudoObjectExpr>(S));
 
     case Stmt::ReturnStmtClass:
     case Stmt::CoreturnStmtClass:
       return VisitReturnStmt(S);
 
+    case Stmt::CoyieldExprClass:
+    case Stmt::CoawaitExprClass:
+      return VisitCoroutineSuspendExpr(cast<CoroutineSuspendExpr>(S), asc);
+
     case Stmt::SEHExceptStmtClass:
       return VisitSEHExceptStmt(cast<SEHExceptStmt>(S));
 
     case Stmt::SEHFinallyStmtClass:
       return VisitSEHFinallyStmt(cast<SEHFinallyStmt>(S));
 
     case Stmt::SEHLeaveStmtClass:
       return VisitSEHLeaveStmt(cast<SEHLeaveStmt>(S));
if (Expr *RV = CRS->getOperand())
     if (RV->getType()->isVoidType() && !isa<InitListExpr>(RV))
       // A non-initlist void expression.
       if (CFGBlock *R = Visit(RV))
         B = R;
 
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitCoroutineSuspendExpr(CoroutineSuspendExpr *E,
+                                                AddStmtChoice asc) {
+  // We're modelling the pre-coro-xform CFG. Thus just evalate the various
+  // active components of the co_await or co_yield. Note we do not model the
+  // edge from the builtin_suspend to the exit node.
+  if (asc.alwaysAdd(*this, E)) {
+    autoCreateBlock();
+    appendStmt(Block, E);
+  }
+  CFGBlock *B = Block;
+  if (auto *R = Visit(E->getResumeExpr()))
+    B = R;
+  if (auto *R = Visit(E->getSuspendExpr()))
+    B = R;
+  if (auto *R = Visit(E->getReadyExpr()))
+    B = R;
+  if (auto *R = Visit(E->getCommonExpr()))
+    B = R;
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitSEHExceptStmt(SEHExceptStmt *ES) {
   // SEHExceptStmt are treated like labels, so they are the first statement in a
   // block.
 
   // Save local scope position because in case of exception variable ScopePos
   // won't be restored when traversing AST.
   SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
 

clang/test/SemaCXX/thread-safety-coro.cpp

public:
   struct promise_type {
   public:
     std::suspend_always initial_suspend() noexcept;
     std::suspend_always final_suspend() noexcept;
 
     Task get_return_object() noexcept;
     void unhandled_exception() noexcept;
     void return_value(int value) noexcept;
+
+    std::suspend_always yield_value(int value) noexcept;
   };
 };
 
 Task Foo() noexcept {
   // ICE'd
+  co_yield({ int frame = 0; 0; });
+  co_await({ int frame = 0; std::suspend_always(); });
   co_return({ int frame = 0; 0; });
 }