From a7a31ff3a52edf536cb99c71df1d411e0a48c3e1 Mon Sep 17 00:00:00 2001 From: TB Schardl Date: Tue, 30 Jun 2020 01:27:36 +0000 Subject: [PATCH] [Cilk] Add semantic checks to disallow invalid _Cilk_spawns in binary expressions --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/Sema/SemaExpr.cpp | 16 ++++++++++++++++ clang/test/Cilk/spawn-builtin.c | 2 +- clang/test/Cilk/spawntest.cpp | 6 ++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b44dbe2b787a..713780065051 100644 --- 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 index 6a809d7d597b..327e5f2d2932 100644 --- 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(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 index 1798d44316cb..9e4378fa5ca2 100644 --- 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 index 1542fe20fc71..ffba48d9c617 100644 --- 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}} +}