[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

[Rachel Craik via Phabricator via cfe-commits]

rcraik created this revision.

When the value specified for //n// in ordered(//n//) is larger than the number of loops a segmentation fault can occur in one of two ways when attempting to print out a diagnostic for an associated depend(sink : //vec//):

1. The iteration vector //vec// contains less than //n// items
2. The iteration vector //vec// contains a variable that is not a loop control variable

This patch addresses both of these issues.


https://reviews.llvm.org/D38049

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/ordered_messages.cpp


Index: test/OpenMP/ordered_messages.cpp
===================================================================
--- test/OpenMP/ordered_messages.cpp
+++ test/OpenMP/ordered_messages.cpp
@@ -270,5 +270,13 @@
     }
   }
 
+#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}}
+  for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}}
+#pragma omp ordered depend(sink : i)
+    int j;
+#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}}
+    foo();
+  }
+
   return foo<int>(); // expected-note {{in instantiation of function template specialization 'foo<int>' requested here}}
 }
Index: lib/Sema/SemaOpenMP.cpp
===================================================================
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10510,9 +10510,14 @@
         if (!CurContext->isDependentContext() &&
             DSAStack->getParentOrderedRegionParam() &&
             DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-          Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-              << DSAStack->getParentLoopControlVariable(
-                     DepCounter.getZExtValue());
+          ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+              DepCounter.getZExtValue());
+          if (VD) {
+            Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
+                << 1 << VD;
+          } else {
+             Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0;
+          }
           continue;
         }
         OpsOffs.push_back({RHS, OOK});
@@ -10545,8 +10550,9 @@
 
     if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink &&
         TotalDepCount > VarList.size() &&
-        DSAStack->getParentOrderedRegionParam()) {
-      Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration)
+        DSAStack->getParentOrderedRegionParam() &&
+        DSAStack->getParentLoopControlVariable(VarList.size() + 1)) {
+      Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1
           << DSAStack->getParentLoopControlVariable(VarList.size() + 1);
     }
     if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink &&
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8869,7 +8869,7 @@
 def err_omp_depend_clause_thread_simd : Error<
   "'depend' clauses cannot be mixed with '%0' clause">;
 def err_omp_depend_sink_expected_loop_iteration : Error<
-  "expected %0 loop iteration variable">;
+  "expected%select{| %1}0 loop iteration variable">;
 def err_omp_depend_sink_unexpected_expr : Error<
   "unexpected expression: number of expressions is larger than the number of associated loops">;
 def err_omp_depend_sink_expected_plus_minus : Error<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38049.115876.patch
Type: text/x-patch
Size: 2992 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170919/9ba63e0c/attachment.bin>