[clang] [OpenACC] Fix Crash on collapse that doesn't check its transform (PR #191836)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

GH191833 reports a problem with tree transformation of a collapse clause when the expression in the clause is invalid. This patch makes sure we skip out of transforming this clause if it ever encounters an invalid expression.

I also analyzed the rest of the clauses in this visitor and found 1 other that was suspicious, so I added a check for that one as well.  The rest seemingly were all done correctly.

Fixes: #<!-- -->191833

---
Full diff: https://github.com/llvm/llvm-project/pull/191836.diff


2 Files Affected:

- (modified) clang/lib/Sema/TreeTransform.h (+12) 
- (modified) clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp (+18) 


``````````diff
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 07499ff2cd392..9aa5396da4e00 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12651,13 +12651,22 @@ void OpenACCClauseTransform<Derived>::VisitCollapseClause(
 
   ExprResult NewLoopCount = Self.TransformExpr(LoopCount);
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   NewLoopCount = Self.getSema().OpenACC().ActOnIntExpr(
       OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
       NewLoopCount.get()->getBeginLoc(), NewLoopCount.get());
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   NewLoopCount =
       Self.getSema().OpenACC().CheckCollapseLoopCount(NewLoopCount.get());
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   ParsedClause.setCollapseDetails(C.hasForce(), NewLoopCount.get());
   NewClause = OpenACCCollapseClause::Create(
       Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
@@ -12681,6 +12690,9 @@ void OpenACCClauseTransform<Derived>::VisitTileClause(
         OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
         NewSizeExpr.get()->getBeginLoc(), NewSizeExpr.get());
 
+    if (!NewSizeExpr.isUsable())
+      return;
+
     NewSizeExpr = Self.getSema().OpenACC().CheckTileSizeExpr(NewSizeExpr.get());
 
     if (!NewSizeExpr.isUsable())
diff --git a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
index ed71552dfdb10..adc481837f74a 100644
--- a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
+++ b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
@@ -594,3 +594,21 @@ void no_dupes_since_last_device_type() {
 #pragma acc loop collapse(NotConstexpr) device_type(radeon, nvidia) collapse(NotConstexpr) device_type(host) collapse(NotConstexpr)
   for(unsigned j = 0; j < 5; ++j);
 }
+
+namespace gh191833 {
+  // Instantiating a collapse referring to an invalid struct caused us to try
+  // get the begin-loc of the `T{}` statement when we shouldn't.
+struct S {
+  typename T; // expected-error{{expected a qualified name after 'typename'}}
+};
+
+template<typename T>
+void foo() {
+#pragma acc loop collapse(T{})
+  for (int i = 0; i < 5; ++i);
+}
+
+void bar() {
+  foo<S>();
+}
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/191836