[clang] 65a9a3d - [OpenACC] Fix asserts when checking clauses after invalid directive

[via cfe-commits]

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 at +1{{invalid OpenACC directive 'foo'}}
+#pragma acc foo gang(1)
+
+// expected-error at +1{{invalid OpenACC directive 'foo'}}
+#pragma acc foo vector(1)
+
+// expected-error at +1{{invalid OpenACC directive 'foo'}}
+#pragma acc foo worker(1)