[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(); // expected-note {{in instantiation of function template 
specialization 'foo' 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<


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(); // expected-note {{in instantiation of function template specialization 'foo' 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_

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

[Rachel Craik via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313675: [OpenMP] fix seg-faults printing diagnostics with 
invalid ordered(n) values (authored by rcraik).

Changed prior to commit:
  https://reviews.llvm.org/D38049?vs=115876&id=115901#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38049

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


Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/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: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8875,7 +8875,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<
Index: cfe/trunk/test/OpenMP/ordered_messages.cpp
===
--- cfe/trunk/test/OpenMP/ordered_messages.cpp
+++ cfe/trunk/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(); // expected-note {{in instantiation of function template 
specialization 'foo' requested here}}
 }


Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/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

[PATCH] D34649: Remove addtional parameters in function std::next() and std::prev()

[Rachel Craik via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308932: Remove addtional parameters in function std::next() 
and std::prev() (authored by rcraik).

Changed prior to commit:
  https://reviews.llvm.org/D34649?vs=104030&id=107977#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34649

Files:
  libcxx/trunk/include/iterator
  
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
  
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp


Index: libcxx/trunk/include/iterator
===
--- libcxx/trunk/include/iterator
+++ libcxx/trunk/include/iterator
@@ -604,21 +604,27 @@
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-_InputIter
+typename enable_if
+<
+__is_input_iterator<_InputIter>::value, 
+_InputIter
+>::type
 next(_InputIter __x,
- typename iterator_traits<_InputIter>::difference_type __n = 1,
- typename enable_if<__is_input_iterator<_InputIter>::value>::type* = 0)
+ typename iterator_traits<_InputIter>::difference_type __n = 1)
 {
 _VSTD::advance(__x, __n);
 return __x;
 }
 
-template 
+template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-_BidiretionalIter
-prev(_BidiretionalIter __x,
- typename iterator_traits<_BidiretionalIter>::difference_type __n = 1,
- typename 
enable_if<__is_bidirectional_iterator<_BidiretionalIter>::value>::type* = 0)
+typename enable_if
+<
+__is_bidirectional_iterator<_BidirectionalIter>::value, 
+_BidirectionalIter
+>::type
+prev(_BidirectionalIter __x,
+ typename iterator_traits<_BidirectionalIter>::difference_type __n = 1)
 {
 _VSTD::advance(__x, -__n);
 return __x;
Index: 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
===
--- 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
+++ 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
@@ -24,6 +24,9 @@
 test(It i, typename std::iterator_traits::difference_type n, It x)
 {
 assert(std::next(i, n) == x);
+
+It (*next)(It, typename std::iterator_traits::difference_type) = 
std::next;
+assert(next(i, n) == x);
 }
 
 template 
Index: 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
===
--- 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
+++ 
libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
@@ -22,6 +22,9 @@
 test(It i, typename std::iterator_traits::difference_type n, It x)
 {
 assert(std::prev(i, n) == x);
+
+It (*prev)(It, typename std::iterator_traits::difference_type) = 
std::prev;
+assert(prev(i, n) == x);
 }
 
 template 


Index: libcxx/trunk/include/iterator
===
--- libcxx/trunk/include/iterator
+++ libcxx/trunk/include/iterator
@@ -604,21 +604,27 @@
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-_InputIter
+typename enable_if
+<
+__is_input_iterator<_InputIter>::value, 
+_InputIter
+>::type
 next(_InputIter __x,
- typename iterator_traits<_InputIter>::difference_type __n = 1,
- typename enable_if<__is_input_iterator<_InputIter>::value>::type* = 0)
+ typename iterator_traits<_InputIter>::difference_type __n = 1)
 {
 _VSTD::advance(__x, __n);
 return __x;
 }
 
-template 
+template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-_BidiretionalIter
-prev(_BidiretionalIter __x,
- typename iterator_traits<_BidiretionalIter>::difference_type __n = 1,
- typename enable_if<__is_bidirectional_iterator<_BidiretionalIter>::value>::type* = 0)
+typename enable_if
+<
+__is_bidirectional_iterator<_BidirectionalIter>::value, 
+_BidirectionalIter
+>::type
+prev(_BidirectionalIter __x,
+ typename iterator_traits<_BidirectionalIter>::difference_type __n = 1)
 {
 _VSTD::advance(__x, -__n);
 return __x;
Index: libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
===
--- libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
+++ libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp
@@ -24,6 +24,9 @@
 test(It i, typename std::iterator_traits::difference_type n, It x)
 {
 assert(std::next(i, n) == x);
+
+It (*next)(It, typename std::iterator_traits::difference_type) = std::next;
+assert(next(i, n) == x);
 }
 
 template 
Index: libcxx/trunk/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
=