Author: erichkeane Date: 2025-05-14T07:03:16-07:00 New Revision: 65a9a3d5b12449f4d32b4f6c01bcc421185a4bf6
URL: https://github.com/llvm/llvm-project/commit/65a9a3d5b12449f4d32b4f6c01bcc421185a4bf6 DIFF: https://github.com/llvm/llvm-project/commit/65a9a3d5b12449f4d32b4f6c01bcc421185a4bf6.diff LOG: [OpenACC] Fix asserts when checking clauses after invalid directive When implementing the parsing for OpenACC I ensured that we could always continue to do diagnostics/etc. For the most part, I was consistent with that assumption throughout clause Sema, but in 3 cases I left in some unreachables for cases where this would happen. I've now properly handled all 3 in a reasonable way. Fixes: #139894 Added: clang/test/SemaOpenACC/gh139894.cpp Modified: clang/lib/Sema/SemaOpenACCClause.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaOpenACCClause.cpp b/clang/lib/Sema/SemaOpenACCClause.cpp index 6187e0e719bb1..88bd963a738ce 100644 --- a/clang/lib/Sema/SemaOpenACCClause.cpp +++ b/clang/lib/Sema/SemaOpenACCClause.cpp @@ -1332,6 +1332,12 @@ OpenACCClause *SemaOpenACCClauseVisitor::VisitVectorClause( break; case OpenACCDirectiveKind::ParallelLoop: break; + case OpenACCDirectiveKind::Invalid: + // This can happen when the directive was not recognized, but we continued + // anyway. Since there is a lot of stuff that can happen (including + // 'allow anything' in the parallel loop case), just skip all checking and + // continue. + break; } } @@ -1369,6 +1375,14 @@ OpenACCClause *SemaOpenACCClauseVisitor::VisitWorkerClause( switch (Clause.getDirectiveKind()) { default: llvm_unreachable("Invalid directive kind for this clause"); + case OpenACCDirectiveKind::Invalid: + // This can happen in cases where the directive was not recognized but we + // continued anyway. Kernels allows kind of any integer argument, so we + // can assume it is that (rather than marking the argument invalid like + // with parallel/serial/routine), and just continue as if nothing + // happened. We'll skip the 'kernels' checking vs num-workers, since this + // MIGHT be something else. + break; case OpenACCDirectiveKind::Loop: switch (SemaRef.getActiveComputeConstructInfo().Kind) { case OpenACCDirectiveKind::Invalid: @@ -2037,6 +2051,12 @@ SemaOpenACC::CheckGangExpr(ArrayRef<const OpenACCClause *> ExistingClauses, default: llvm_unreachable("Non compute construct in active compute construct?"); } + case OpenACCDirectiveKind::Invalid: + // This can happen in cases where the the directive was not recognized but + // we continued anyway. Since the validity checking is all-over the place + // (it can be a star/integer, or a constant expr depending on the tag), we + // just give up and return an ExprError here. + return ExprError(); default: llvm_unreachable("Invalid directive kind for a Gang clause"); } diff --git a/clang/test/SemaOpenACC/gh139894.cpp b/clang/test/SemaOpenACC/gh139894.cpp new file mode 100644 index 0000000000000..f1f6298a97665 --- /dev/null +++ b/clang/test/SemaOpenACC/gh139894.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 %s -fopenacc -verify + +// Ensure that these don't assert, they previously assumed that their directive +// kind would be valid, but we should make sure that we handle that gracefully +// in cases where they don't. + +// expected-error@+1{{invalid OpenACC directive 'foo'}} +#pragma acc foo gang(1) + +// expected-error@+1{{invalid OpenACC directive 'foo'}} +#pragma acc foo vector(1) + +// expected-error@+1{{invalid OpenACC directive 'foo'}} +#pragma acc foo worker(1) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits