Reid, thank you for the review!

I fixed the use-after-free bug you found (good catch, BTW!) I also created a 
test that checks -E (not sure this is exactly what you wanted; let me know if I 
misunderstood you).


http://reviews.llvm.org/D9856

Files:
  lib/Lex/Pragma.cpp
  test/Preprocessor/pragma_microsoft.c
  test/Preprocessor/pragma_microsoft_E.c

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: test/Preprocessor/pragma_microsoft_E.c
===================================================================
--- test/Preprocessor/pragma_microsoft_E.c
+++ test/Preprocessor/pragma_microsoft_E.c
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 %s -E -fms-extensions 2>&1 | FileCheck %s
+// REQUIRES: non-ps4-sdk
+
+// This test simply checks that these pragmas successfully roundtrip through the
+// preprocessor. Error messages are checked in pragma_microsoft.c.
+
+#define FOO 1
+#define BAR "2"
+
+#pragma comment(linker," bar=" BAR)
+
+#pragma comment( user, "Compiled on " __DATE__ " at " __TIME__ ) 
+
+#define foo compiler
+#pragma comment(foo)   // macro expand kind.
+
+#pragma comment(user, "foo\abar\nbaz\tsome	thing")
+
+#pragma detect_mismatch("test", "1")
+#pragma detect_mismatch("test", BAR)
+
+// __pragma
+
+__pragma(comment(linker," bar=" BAR))
+
+#define MACRO_WITH__PRAGMA { \
+  __pragma(warning(push)); \
+  __pragma(warning(disable: 10000)); \
+  1 + (2 > 3) ? 4 : 5; \
+  __pragma(warning(pop)); \
+}
+
+// This should include macro_arg_directive even though the include
+// is looking for test.h  This allows us to assign to "n"
+#pragma include_alias("test.h", "macro_arg_directive.h" )
+#include "test.h"
+void test( void ) {
+  n = 12;
+}
+
+// Make sure that the names match exactly for a replacement, including path information.  If
+// this were to fail, we would get a file not found error
+#pragma include_alias(".\pp-record.h", "does_not_exist.h")
+#include "pp-record.h"
+
+// It's expected that we can map "bar" and <bar> separately
+#define test
+// We can't actually look up stdio.h because we're using cc1 without header paths, but this will ensure
+// that we get the right bar.h, because the "bar.h" will undef test for us, where <bar.h> won't
+#pragma include_alias(<bar.h>, <stdio.h>)
+#pragma include_alias("bar.h", "pr2086.h")  // This should #undef test
+
+#include "bar.h"
+#if defined(test)
+// This should not warn because test should not be defined
+#pragma include_alias("test.h")
+#endif
+
+// Test to make sure there are no use-after-free problems
+#define B "pp-record.h"
+#pragma include_alias("quux.h", B)
+void g() {}
+#include "quux.h"
+
+// Test that we ignore pragma warning.
+#pragma warning(push)
+#pragma warning(push, 1)
+#pragma warning(disable : 4705)
+#pragma warning(disable : 123 456 789 ; error : 321)
+#pragma warning(once : 321)
+#pragma warning(suppress : 321)
+#pragma warning(default : 321)
+#pragma warning(pop)
+#pragma warning(1: 123)
+#pragma warning(2: 234 567)
+#pragma warning(3: 123; 4: 678)
+
+#pragma warning(push, 0)
+// FIXME: We could probably support pushing warning level 0.
+#pragma warning(pop)
+
+// Test that we parsed all pragmas successfully and not produced any warnings
+// CHECK-NOT: warning:
+
Index: test/Preprocessor/pragma_microsoft.c
===================================================================
--- test/Preprocessor/pragma_microsoft.c
+++ test/Preprocessor/pragma_microsoft.c
@@ -27,7 +27,7 @@
 
 // __pragma
 
-__pragma(comment(linker," bar=" BAR))                                            
+__pragma(comment(linker," bar=" BAR))
 
 #define MACRO_WITH__PRAGMA { \
   __pragma(warning(push)); \
@@ -98,6 +98,10 @@
 #pragma warning(suppress : 321)
 #pragma warning(default : 321)
 #pragma warning(pop)
+#pragma warning(1: 123)
+#pragma warning(2: 234 567)
+#pragma warning(3: 123; 4: 678)
+#pragma warning(5: 123) // expected-warning {{expected 'push', 'pop', 'default', 'disable', 'error', 'once', 'suppress', 1, 2, 3, or 4}}
 
 #pragma warning(push, 0)
 // FIXME: We could probably support pushing warning level 0.
Index: lib/Lex/Pragma.cpp
===================================================================
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -22,6 +22,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
@@ -1036,12 +1037,8 @@
 
     PP.Lex(Tok);
     IdentifierInfo *II = Tok.getIdentifierInfo();
-    if (!II) {
-      PP.Diag(Tok, diag::warn_pragma_warning_spec_invalid);
-      return;
-    }
 
-    if (II->isStr("push")) {
+    if (II && II->isStr("push")) {
       // #pragma warning( push[ ,n ] )
       int Level = -1;
       PP.Lex(Tok);
@@ -1058,7 +1055,7 @@
       }
       if (Callbacks)
         Callbacks->PragmaWarningPush(DiagLoc, Level);
-    } else if (II->isStr("pop")) {
+    } else if (II && II->isStr("pop")) {
       // #pragma warning( pop )
       PP.Lex(Tok);
       if (Callbacks)
@@ -1068,23 +1065,40 @@
       //                  [; warning-specifier : warning-number-list...] )
       while (true) {
         II = Tok.getIdentifierInfo();
-        if (!II) {
+        if (!II && !Tok.is(tok::numeric_constant)) {
           PP.Diag(Tok, diag::warn_pragma_warning_spec_invalid);
           return;
         }
 
         // Figure out which warning specifier this is.
-        StringRef Specifier = II->getName();
-        bool SpecifierValid =
-            llvm::StringSwitch<bool>(Specifier)
-                .Cases("1", "2", "3", "4", true)
-                .Cases("default", "disable", "error", "once", "suppress", true)
-                .Default(false);
+        bool SpecifierValid;
+        StringRef Specifier;
+        llvm::SmallString<1> SpecifierBuf;
+        if (II) {
+          Specifier = II->getName();
+          SpecifierValid = llvm::StringSwitch<bool>(Specifier)
+                               .Cases("default", "disable", "error", "once",
+                                      "suppress", true)
+                               .Default(false);
+          // If we read a correct specifier, snatch next token (that should be
+          // ":", checked later).
+          if (SpecifierValid)
+            PP.Lex(Tok);
+        } else {
+          // Token is a numeric constant. It should be either 1, 2, 3 or 4.
+          uint64_t Value;
+          Specifier = PP.getSpelling(Tok, SpecifierBuf);
+          if (PP.parseSimpleIntegerLiteral(Tok, Value)) {
+            SpecifierValid = (Value >= 1) && (Value <= 4);
+          } else
+            SpecifierValid = false;
+          // Next token already snatched by parseSimpleIntegerLiteral.
+        }
+
         if (!SpecifierValid) {
           PP.Diag(Tok, diag::warn_pragma_warning_spec_invalid);
           return;
         }
-        PP.Lex(Tok);
         if (Tok.isNot(tok::colon)) {
           PP.Diag(Tok, diag::warn_pragma_warning_expected) << ":";
           return;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to