[Cilk] Add semantic checks to disallow invalid _Cilk_spawns in binary…

… expressions
OpenCilk · Jun 30, 2020 · a7a31ff · a7a31ff
1 parent 41bde2b
commit a7a31ff
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 3 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8860,8 +8860,8 @@ def err_spawn_invalid_decl : Error<
   "_Cilk_spawn not supported in a '%0Decl'">;
 def err_spawn_spawn : Error<
   "consecutive _Cilk_spawn tokens not allowed">;
-def err_spawn_not_whole_expr : Error<
-  "_Cilk_spawn is not at statement level">;
+def err_invalid_spawn_expr : Error<
+  "invalid _Cilk_spawn in expression">;
 def err_cannot_spawn_builtin: Error<
   "builtin function cannot be spawned">;
 def err_cannot_spawn_function: Error<

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -12313,6 +12313,12 @@ static inline UnaryOperatorKind ConvertTokenKindToUnaryOpcode(
   return Opc;
 }
 
+/// Check if Expr is an illegal spawn expression.
+static void CheckForIllegalSpawn(Sema &S, Expr *Expr) {
+  if (isa<CilkSpawnExpr>(Expr->IgnoreImplicit()))
+    S.Diag(Expr->getExprLoc(), diag::err_invalid_spawn_expr);
+}
+
 /// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself.
 /// This warning suppressed in the event of macro expansions.
 static void DiagnoseSelfAssignment(Sema &S, Expr *LHSExpr, Expr *RHSExpr,
@@ -12538,6 +12544,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     }
   }
 
+  // Check for illegal spawns
+  if (!BinaryOperator::isAssignmentOp(Opc))
+    CheckForIllegalSpawn(*this, RHS.get());
+  CheckForIllegalSpawn(*this, LHS.get());
+
   switch (Opc) {
   case BO_Assign:
     ResultTy = CheckAssignmentOperands(LHS.get(), RHS, OpLoc, QualType());
@@ -12982,6 +12993,11 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc,
     break;
   }
 
+  // Check for illegal spawns
+  if (!BinaryOperator::isAssignmentOp(Opc))
+    CheckForIllegalSpawn(S, RHS);
+  CheckForIllegalSpawn(S, LHS);
+
   // Find all of the overloaded operators visible from this
   // point. We perform both an operator-name lookup from the local
   // scope and an argument-dependent lookup based on the types of

diff --git a/clang/test/Cilk/spawn-builtin.c b/clang/test/Cilk/spawn-builtin.c
@@ -64,7 +64,7 @@ void spawn_trap() {
 // CHECK-NEXT: sync within %[[SYNCREG]]
 
 void spawn_assume() {
-  _Cilk_spawn __builtin_assume(0); // expected-warning{{Failed to produce spawn}}
+  _Cilk_spawn __builtin_assume(0); // expected-warning{{Failed to emit spawn}}
 }
 
 // It doesn't make sense to spawn an assume, so we expect not to find any

diff --git a/clang/test/Cilk/spawntest.cpp b/clang/test/Cilk/spawntest.cpp
@@ -95,3 +95,9 @@ int spawn_assign_eval_order_tests(int n) {
   Arr[i++] += _Cilk_spawn bar(i); // expected-warning {{unsequenced modification and access to 'i'}}
   return 0;
 }
+
+void invalid_spawn_expr() {
+  int x = 0;
+  x + _Cilk_spawn 7; // expected-warning {{expression result unused}} expected-error {{invalid _Cilk_spawn in expression}}
+  int y = x + _Cilk_spawn 7; // expected-error {{invalid _Cilk_spawn in expression}}
+}