Hi TylerNowicki, aaron.ballman,

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'

In the last example above, the old behavior isn't incorrect. I made the change 
as it simplified the code a bit and made the message consistent regardless of 
what the argument is (rather than just printing the argument if it is an 
identifier).  Happy to change it back if anyone thinks it's a step backwards.

http://reviews.llvm.org/D4197

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/Parser/pragma-loop.cpp
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -904,6 +904,8 @@
 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 {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -543,9 +543,9 @@
 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<
Index: lib/Parse/ParsePragma.cpp
===================================================================
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -1710,9 +1710,13 @@
     // FIXME: All tokens between '(' and ')' should be stored and parsed as a
     // constant expression.
     PP.Lex(Tok);
-    Token Value;
-    if (Tok.is(tok::identifier) || Tok.is(tok::numeric_constant))
-      Value = Tok;
+    if (Tok.is(tok::r_paren)) {
+      // Nothing between the parentheses.
+      PP.Diag(Tok.getLocation(), diag::err_pragma_loop_missing_argument)
+          << OptionInfo;
+      return;
+    }
+    Token Value = Tok;
 
     // Read ')'
     PP.Lex(Tok);
Index: lib/Sema/SemaStmtAttr.cpp
===================================================================
--- lib/Sema/SemaStmtAttr.cpp
+++ lib/Sema/SemaStmtAttr.cpp
@@ -75,35 +75,26 @@
   if (Option == LoopHintAttr::Vectorize || Option == LoopHintAttr::Interleave ||
       Option == LoopHintAttr::Unroll) {
     if (!ValueInfo) {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
-          << /*MissingKeyword=*/true << "";
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
       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)
-          << /*MissingKeyword=*/false << ValueInfo;
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
       return nullptr;
     }
   } else if (Option == LoopHintAttr::VectorizeWidth ||
              Option == LoopHintAttr::InterleaveCount ||
              Option == LoopHintAttr::UnrollCount) {
     // 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)) {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value)
-          << /*MissingValue=*/true << "";
-      return nullptr;
-    }
-
-    if ((ValueInt = ValueAPS.getSExtValue()) < 1) {
-      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value)
-          << /*MissingValue=*/false << ValueInt;
+    if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
+        (ValueInt = ValueAPS.getSExtValue()) < 1) {
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value);
       return nullptr;
     }
   } else
Index: test/Parser/pragma-loop.cpp
===================================================================
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -55,6 +55,10 @@
 /* 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 @@
     List[i] = i;
   }
 
-/* expected-error {{invalid value 0; 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 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 vectorize_width(0)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_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 value -1294967296; 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 vectorize_width(3000000000)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_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 {{missing value; 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 vectorize_width(badvalue)
+/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_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 keyword 'badidentifier'; 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 vectorize(badidentifier)
+/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(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];
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to