Fix PR20069: bad loop pragma arguments crash FE

This patch fixes a crash when handling malformed arguments to loop pragmas such
as: "#pragma clang loop vectorize(()".  Essentially any argument which is not an
identifier or constant resulted in a crash.  This patch also changes a couple of
the error messages which weren't quite correct.  New behavior with this patch vs
old behavior:

#pragma clang loop vectorize(1)
OLD: error: missing keyword; expected 'enable' or 'disable'
NEW: error: invalid argument; expected 'enable' or 'disable'

#pragma clang loop vectorize()
OLD: error: expected ')'
NEW: error: missing argument to loop pragma 'vectorize'

#pragma clang loop vectorize_width(bad)
OLD: error: missing value; expected a positive integer value
NEW: error: invalid argument; expected a positive integer value

#pragma clang loop vectorize(bad)
OLD: invalid keyword 'bad'; expected 'enable' or 'disable'
NEW: error: invalid argument; expected 'enable' or 'disable'

http://reviews.llvm.org/D4197

Patch by Mark Heffernan

llvm-svn: 211292

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -909,6 +909,8 @@ def err_omp_more_one_clause : Error<
 def err_pragma_loop_invalid_option : Error<
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
   "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
+def err_pragma_loop_missing_argument : Error<
+  "missing argument to loop pragma %0">;
 } // end of Parse Issue category.
 
 let CategoryName = "Modules Issue" in {

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -543,9 +543,9 @@ def err_pragma_pop_visibility_mismatch : Error<
 def note_surrounding_namespace_starts_here : Note<
   "surrounding namespace with visibility attribute starts here">;
 def err_pragma_loop_invalid_value : Error<
-  "%select{invalid|missing}0 value%select{ %1|}0; expected a positive integer value">;
+  "invalid argument; expected a positive integer value">;
 def err_pragma_loop_invalid_keyword : Error<
-  "%select{invalid|missing}0 keyword%select{ %1|}0; expected 'enable' or 'disable'">;
+  "invalid argument; expected 'enable' or 'disable'">;
 def err_pragma_loop_compatibility : Error<
   "%select{incompatible|duplicate}0 directives '%1(%2)' and '%3(%4)'">;
 def err_pragma_loop_precedes_nonloop : Error<

clang/lib/Parse/ParsePragma.cpp

@@ -1710,9 +1710,13 @@ void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
     // FIXME: All tokens between '(' and ')' should be stored and parsed as a
     // constant expression.
     PP.Lex(Tok);
-    Token Value;
+    if (Tok.is(tok::r_paren)) {
-    if (Tok.is(tok::identifier) || Tok.is(tok::numeric_constant))
+      // Nothing between the parentheses.
-      Value = Tok;
+      PP.Diag(Tok.getLocation(), diag::err_pragma_loop_missing_argument)
+          << OptionInfo;
+      return;
+    }
+    Token Value = Tok;
 
     // Read ')'
     PP.Lex(Tok);

clang/lib/Sema/SemaStmtAttr.cpp

@@ -75,18 +75,15 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
   if (Option == LoopHintAttr::Vectorize || Option == LoopHintAttr::Interleave ||
       Option == LoopHintAttr::Unroll) {
     if (!ValueInfo) {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
-          << /*MissingKeyword=*/true << "";
       return nullptr;
     }
-
     if (ValueInfo->isStr("disable"))
       ValueInt = 0;
     else if (ValueInfo->isStr("enable"))
       ValueInt = 1;
     else {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
-          << /*MissingKeyword=*/false << ValueInfo;
       return nullptr;
     }
   } else if (Option == LoopHintAttr::VectorizeWidth ||
@@ -95,15 +92,9 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
     // FIXME: We should support template parameters for the loop hint value.
     // See bug report #19610.
     llvm::APSInt ValueAPS;
-    if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context)) {
+    if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value)
+        (ValueInt = ValueAPS.getSExtValue()) < 1) {
-          << /*MissingValue=*/true << "";
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value);
-      return nullptr;
-    }
-
-    if ((ValueInt = ValueAPS.getSExtValue()) < 1) {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value)
-          << /*MissingValue=*/false << ValueInt;
       return nullptr;
     }
   } else

clang/test/Parser/pragma-loop.cpp

@@ -55,6 +55,10 @@ void test(int *List, int Length) {
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave_count(4
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll_count(4
 
+/* expected-error {{missing argument to loop pragma 'vectorize'}} */ #pragma clang loop vectorize()
+/* expected-error {{missing argument to loop pragma 'interleave_count'}} */ #pragma clang loop interleave_count()
+/* expected-error {{missing argument to loop pragma 'unroll'}} */ #pragma clang loop unroll()
+
 /* expected-error {{missing option}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
@@ -65,34 +69,46 @@ void test(int *List, int Length) {
     List[i] = i;
   }
 
-/* expected-error {{invalid value 0; expected a positive integer value}} */ #pragma clang loop vectorize_width(0)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(0)
-/* expected-error {{invalid value 0; expected a positive integer value}} */ #pragma clang loop interleave_count(0)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(0)
-/* expected-error {{invalid value 0; expected a positive integer value}} */ #pragma clang loop unroll_count(0)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(0)
   while (i-5 < Length) {
     List[i] = i;
   }
 
-/* expected-error {{invalid value -1294967296; expected a positive integer value}} */ #pragma clang loop vectorize_width(3000000000)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(3000000000)
-/* expected-error {{invalid value -1294967296; expected a positive integer value}} */ #pragma clang loop interleave_count(3000000000)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(3000000000)
-/* expected-error {{invalid value -1294967296; expected a positive integer value}} */ #pragma clang loop unroll_count(3000000000)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(3000000000)
   while (i-6 < Length) {
     List[i] = i;
   }
 
-/* expected-error {{missing value; expected a positive integer value}} */ #pragma clang loop vectorize_width(badvalue)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(badvalue)
-/* expected-error {{missing value; expected a positive integer value}} */ #pragma clang loop interleave_count(badvalue)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(badvalue)
-/* expected-error {{missing value; expected a positive integer value}} */ #pragma clang loop unroll_count(badvalue)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(badvalue)
   while (i-6 < Length) {
     List[i] = i;
   }
 
-/* expected-error {{invalid keyword 'badidentifier'; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
-/* expected-error {{invalid keyword 'badidentifier'; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
-/* expected-error {{invalid keyword 'badidentifier'; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
   while (i-7 < Length) {
     List[i] = i;
   }
 
+// PR20069 - Loop pragma arguments that are not identifiers or numeric
+// constants crash FE.
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(()
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(*)
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(=)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(^)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(/)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(==)
+  while (i-8 < Length) {
+    List[i] = i;
+  }
+
 #pragma clang loop vectorize(enable)
 /* expected-error {{expected a for, while, or do-while loop to follow the '#pragma clang loop' directive}} */ int j = Length;
   List[0] = List[1];