Skip to content

[OpenMP] Fix a crash on invalid with unroll partial #139280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 9, 2025

Conversation

AaronBallman
Copy link
Collaborator

You cannot get the integer constant expression's value if the expression contains errors.

Fixes #139267

You cannot get the integer constant expression's value if the
expression contains errors.

Fixes llvm#139267
@AaronBallman AaronBallman added clang Clang issues not falling into any other category openmp clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-invalid labels May 9, 2025
@llvmbot llvmbot added the clang:openmp OpenMP related changes to Clang label May 9, 2025
@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-openmp

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

You cannot get the integer constant expression's value if the expression contains errors.

Fixes #139267


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+2-1)
  • (modified) clang/test/OpenMP/unroll_messages.cpp (+9)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f7d56cfd19ce5..aefce7829310d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -901,6 +901,8 @@ OpenMP Support
 - Added support 'no_openmp_constructs' assumption clause.
 - Added support for 'self_maps' in map and requirement clause.
 - Added support for 'omp stripe' directive.
+- Fixed a crashing bug with ``omp unroll partial`` if the argument to
+  ``partial`` was an invalid expression. (#GH139267)
 - Fixed a crashing bug with ``omp tile sizes`` if the argument to ``sizes`` was
   an invalid expression. (#GH139073)
 - Fixed a crashing bug with ``omp distribute dist_schedule`` if the argument to
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 50211c5cf3ed8..15d568fcb9cc4 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14912,7 +14912,8 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
   // Determine the unroll factor.
   uint64_t Factor;
   SourceLocation FactorLoc;
-  if (Expr *FactorVal = PartialClause->getFactor()) {
+  if (Expr *FactorVal = PartialClause->getFactor();
+      FactorVal && !FactorVal->containsErrors()) {
     Factor = FactorVal->getIntegerConstantExpr(Context)->getZExtValue();
     FactorLoc = FactorVal->getExprLoc();
   } else {
diff --git a/clang/test/OpenMP/unroll_messages.cpp b/clang/test/OpenMP/unroll_messages.cpp
index 75bca4b8ad1e0..17d5ed83e162f 100644
--- a/clang/test/OpenMP/unroll_messages.cpp
+++ b/clang/test/OpenMP/unroll_messages.cpp
@@ -128,3 +128,12 @@ void template_inst(int n) {
   // expected-note@+1 {{in instantiation of function template specialization 'templated_func<int, -1>' requested here}}
   templated_func<int, -1>(n);
 }
+
+namespace GH139267 {
+void f(void) {
+  // This would previously crash with follow-on recovery after issuing the error.
+#pragma omp unroll partial(a) // expected-error {{use of undeclared identifier 'a'}}
+  for (int i = 0; i < 10; i++)
+    ;
+}
+}

@AaronBallman AaronBallman merged commit ab6c4f5 into llvm:main May 9, 2025
10 of 11 checks passed
@AaronBallman AaronBallman deleted the aballman-gh139267 branch May 9, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category crash-on-invalid openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenMP] Assertion 'this->_M_is_engaged()' failed.
3 participants