Warn about non-standard format strings (pr12017)

This adds the -Wformat-non-standard flag (off by default,
enabled by -pedantic), which warns about non-standard
things in format strings (such as the 'q' length modifier,
the 'S' conversion specifier, etc.)

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@151154 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 179 insertions, 11 deletions

diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h
index f7a658ea63..9dc8f8878d 100644
--- a/include/clang/Analysis/Analyses/FormatString.h
+++ b/include/clang/Analysis/Analyses/FormatString.h
@@ -348,6 +348,12 @@ public:
   bool usesPositionalArg() const { return UsesPositionalArg; }
 
   bool hasValidLengthModifier() const;
+
+  bool hasStandardLengthModifier() const;
+
+  bool hasStandardConversionSpecifier(const LangOptions &LangOpt) const;
+
+  bool hasStandardLengthConversionCombination() const;
 };
 
 } // end analyze_format_string namespace
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index d4c733634b..7b45a9a71c 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -290,6 +290,7 @@ def Unused : DiagGroup<"unused",
 // Format settings.
 def FormatInvalidSpecifier : DiagGroup<"format-invalid-specifier">;
 def FormatSecurity : DiagGroup<"format-security">;
+def FormatNonStandard : DiagGroup<"format-non-standard">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def Format : DiagGroup<"format",
                        [FormatExtraArgs, FormatZeroLength, NonNull,
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 4f0eea3830..2f5ef02f76 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4847,6 +4847,11 @@ def warn_printf_nonsensical_flag: Warning<
 def warn_format_nonsensical_length: Warning<
   "length modifier '%0' results in undefined behavior or no effect with '%1' conversion specifier">,
   InGroup<Format>;
+def warn_format_non_standard: ExtWarn<
+  "'%0' is a non-standard %1">, InGroup<FormatNonStandard>, DefaultIgnore;
+def warn_format_non_standard_conversion_spec: ExtWarn<
+  "using the length modifier '%0' with the conversion specifier '%1' is non-standard">,
+  InGroup<FormatNonStandard>, DefaultIgnore;
 def warn_printf_ignored_flag: Warning<
   "flag '%0' is ignored when flag '%1' is present">,
   InGroup<Format>;
diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp
index fcc293bf30..5785655c94 100644
--- a/lib/Analysis/FormatString.cpp
+++ b/lib/Analysis/FormatString.cpp
@@ -602,3 +602,74 @@ bool FormatSpecifier::hasValidLengthModifier() const {
   }
   llvm_unreachable("Invalid LengthModifier Kind!");
 }
+
+bool FormatSpecifier::hasStandardLengthModifier() const {
+  switch (LM.getKind()) {
+    case LengthModifier::None:
+    case LengthModifier::AsChar:
+    case LengthModifier::AsShort:
+    case LengthModifier::AsLong:
+    case LengthModifier::AsLongLong:
+    case LengthModifier::AsIntMax:
+    case LengthModifier::AsSizeT:
+    case LengthModifier::AsPtrDiff:
+    case LengthModifier::AsLongDouble:
+      return true;
+    case LengthModifier::AsAllocate:
+    case LengthModifier::AsMAllocate:
+    case LengthModifier::AsQuad:
+      return false;
+  }
+  llvm_unreachable("Invalid LengthModifier Kind!");
+}
+
+bool FormatSpecifier::hasStandardConversionSpecifier(const LangOptions &LangOpt) const {
+  switch (CS.getKind()) {
+    case ConversionSpecifier::cArg:
+    case ConversionSpecifier::dArg:
+    case ConversionSpecifier::iArg:
+    case ConversionSpecifier::oArg:
+    case ConversionSpecifier::uArg:
+    case ConversionSpecifier::xArg:
+    case ConversionSpecifier::XArg:
+    case ConversionSpecifier::fArg:
+    case ConversionSpecifier::FArg:
+    case ConversionSpecifier::eArg:
+    case ConversionSpecifier::EArg:
+    case ConversionSpecifier::gArg:
+    case ConversionSpecifier::GArg:
+    case ConversionSpecifier::aArg:
+    case ConversionSpecifier::AArg:
+    case ConversionSpecifier::sArg:
+    case ConversionSpecifier::pArg:
+    case ConversionSpecifier::nArg:
+    case ConversionSpecifier::ObjCObjArg:
+    case ConversionSpecifier::ScanListArg:
+    case ConversionSpecifier::PercentArg:
+      return true;
+    case ConversionSpecifier::CArg:
+    case ConversionSpecifier::SArg:
+      return LangOpt.ObjC1 || LangOpt.ObjC2;
+    case ConversionSpecifier::InvalidSpecifier:
+    case ConversionSpecifier::PrintErrno:
+      return false;
+  }
+  llvm_unreachable("Invalid ConversionSpecifier Kind!");
+}
+
+bool FormatSpecifier::hasStandardLengthConversionCombination() const {
+  if (LM.getKind() == LengthModifier::AsLongDouble) {
+    switch(CS.getKind()) {
+        case ConversionSpecifier::dArg:
+        case ConversionSpecifier::iArg:
+        case ConversionSpecifier::oArg:
+        case ConversionSpecifier::uArg:
+        case ConversionSpecifier::xArg:
+        case ConversionSpecifier::XArg:
+          return false;
+        default:
+          return true;
+    }
+  }
+  return true;
+}
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 7f42b2059d..a89e813cc8 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1663,7 +1663,20 @@ public:
 
   void HandleIncompleteSpecifier(const char *startSpecifier,
                                  unsigned specifierLen);
-    
+
+  void HandleNonStandardLengthModifier(
+      const analyze_format_string::LengthModifier &LM,
+      const char *startSpecifier, unsigned specifierLen);
+
+  void HandleNonStandardConversionSpecifier(
+      const analyze_format_string::ConversionSpecifier &CS,
+      const char *startSpecifier, unsigned specifierLen);
+
+  void HandleNonStandardConversionSpecification(
+      const analyze_format_string::LengthModifier &LM,
+      const analyze_format_string::ConversionSpecifier &CS,
+      const char *startSpecifier, unsigned specifierLen);
+
   virtual void HandleInvalidPosition(const char *startSpecifier,
                                      unsigned specifierLen,
                                      analyze_format_string::PositionContext p);
@@ -1739,6 +1752,37 @@ void CheckFormatHandler::HandleIncompleteSpecifier(const char *startSpecifier,
                        getSpecifierRange(startSpecifier, specifierLen));
 }
 
+void CheckFormatHandler::HandleNonStandardLengthModifier(
+    const analyze_format_string::LengthModifier &LM,
+    const char *startSpecifier, unsigned specifierLen) {
+  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << LM.toString()
+                       << "length modifier",
+                       getLocationOfByte(LM.getStart()),
+                       /*IsStringLocation*/true,
+                       getSpecifierRange(startSpecifier, specifierLen));
+}
+
+void CheckFormatHandler::HandleNonStandardConversionSpecifier(
+    const analyze_format_string::ConversionSpecifier &CS,
+    const char *startSpecifier, unsigned specifierLen) {
+  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << CS.toString()
+                       << "conversion specifier",
+                       getLocationOfByte(CS.getStart()),
+                       /*IsStringLocation*/true,
+                       getSpecifierRange(startSpecifier, specifierLen));
+}
+
+void CheckFormatHandler::HandleNonStandardConversionSpecification(
+    const analyze_format_string::LengthModifier &LM,
+    const analyze_format_string::ConversionSpecifier &CS,
+    const char *startSpecifier, unsigned specifierLen) {
+  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_conversion_spec)
+                       << LM.toString() << CS.toString(),
+                       getLocationOfByte(LM.getStart()),
+                       /*IsStringLocation*/true,
+                       getSpecifierRange(startSpecifier, specifierLen));
+}
+
 void
 CheckFormatHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
                                      analyze_format_string::PositionContext p) {
@@ -2157,6 +2201,13 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
                          FixItHint::CreateRemoval(
                            getSpecifierRange(LM.getStart(),
                                              LM.getLength())));
+  if (!FS.hasStandardLengthModifier())
+    HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen);
+  if (!FS.hasStandardConversionSpecifier(S.getLangOptions()))
+    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
+  if (!FS.hasStandardLengthConversionCombination())
+    HandleNonStandardConversionSpecification(LM, CS, startSpecifier,
+                                             specifierLen);
 
   // Are we using '%n'?
   if (CS.getKind() == ConversionSpecifier::nArg) {
@@ -2343,6 +2394,14 @@ bool CheckScanfHandler::HandleScanfSpecifier(
                          FixItHint::CreateRemoval(R));
   }
 
+  if (!FS.hasStandardLengthModifier())
+    HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen);
+  if (!FS.hasStandardConversionSpecifier(S.getLangOptions()))
+    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
+  if (!FS.hasStandardLengthConversionCombination())
+    HandleNonStandardConversionSpecification(LM, CS, startSpecifier,
+                                             specifierLen);
+
   // The remaining checks depend on the data arguments.
   if (HasVAListArg)
     return true;
@@ -4971,4 +5030,3 @@ void Sema::DiagnoseEmptyLoopBody(const Stmt *S,
     Diag(NBody->getSemiLoc(), diag::note_empty_body_on_separate_line);
   }
 }
-
diff --git a/test/Sema/format-strings-c90.c b/test/Sema/format-strings-c90.c
index 0b32e82d87..63d58532cb 100644
--- a/test/Sema/format-strings-c90.c
+++ b/test/Sema/format-strings-c90.c
@@ -5,9 +5,8 @@ int scanf(const char * restrict, ...);
 int printf(const char *restrict, ...);
 
 void foo(char **sp, float *fp, int *ip) {
-  /* TODO: Warn that the 'a' length modifier is an extension. */
-  scanf("%as", sp);
-  scanf("%a[abc]", sp);
+  scanf("%as", sp); /* expected-warning{{'a' is a non-standard length modifier}} */
+  scanf("%a[abc]", sp); /* expected-warning{{'a' is a non-standard length modifier}} */
 
   /* TODO: Warn that the 'a' conversion specifier is a C99 feature. */
   scanf("%a", fp);
@@ -21,7 +20,11 @@ void foo(char **sp, float *fp, int *ip) {
   scanf("%da", ip);
 
   /* Test argument type check for the 'a' length modifier. */
-  scanf("%as", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}} */
-  scanf("%aS", fp); /* expected-warning{{format specifies type 'wchar_t **' (aka 'int **') but the argument has type 'float *'}} */
-  scanf("%a[abc]", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}} */
+  scanf("%as", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}}
+                       expected-warning{{'a' is a non-standard length modifier}} */
+  scanf("%aS", fp); /* expected-warning{{format specifies type 'wchar_t **' (aka 'int **') but the argument has type 'float *'}}
+                       expected-warning{{'a' is a non-standard length modifier}}
+                       expected-warning{{'S' is a non-standard conversion specifier}} */
+  scanf("%a[abc]", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}}
+                           expected-warning{{'a' is a non-standard length modifier}} */
 }
diff --git a/test/Sema/format-strings-non-standard.c b/test/Sema/format-strings-non-standard.c
new file mode 100644
index 0000000000..a24d43a6e1
--- /dev/null
+++ b/test/Sema/format-strings-non-standard.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -pedantic %s
+
+int printf(const char *restrict, ...);
+int scanf(const char * restrict, ...);
+
+void f(void) {
+  char *cp;
+
+  // The 'q' length modifier.
+  printf("%qd", (long long)42); // expected-warning{{'q' is a non-standard length modifier}}
+  scanf("%qd", (long long *)0); // expected-warning{{'q' is a non-standard length modifier}}
+
+  // The 'm' length modifier.
+  scanf("%ms", &cp); // expected-warning{{'m' is a non-standard length modifier}}
+
+  // The 'S' and 'C' conversion specifiers.
+  printf("%S", L"foo"); // expected-warning{{'S' is a non-standard conversion specifier}}
+  printf("%C", L'x'); // expected-warning{{'C' is a non-standard conversion specifier}}
+
+  // Combining 'L' with an integer conversion specifier.
+  printf("%Li", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'i' is non-standard}}
+  printf("%Lo", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'o' is non-standard}}
+  printf("%Lu", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'u' is non-standard}}
+  printf("%Lx", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'x' is non-standard}}
+  printf("%LX", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'X' is non-standard}}
+}
diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index 55b8201fd3..086c5c6d6f 100644
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -268,7 +268,6 @@ void test_unicode_conversions(wchar_t *s) {
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
-  // FIXME: we report the expected type as 'int*' instead of 'wchar_t*'
   printf("%S", "hello"); // expected-warning{{but the argument has type 'char *'}}
 }
 
diff --git a/test/SemaCXX/format-strings.cpp b/test/SemaCXX/format-strings.cpp
index 74fc7a7810..314617d491 100644
--- a/test/SemaCXX/format-strings.cpp
+++ b/test/SemaCXX/format-strings.cpp
@@ -9,8 +9,7 @@ extern int vprintf(const char *restrict, va_list);
 }
 
 void f(char **sp, float *fp) {
-  // TODO: Warn that the 'a' length modifier is an extension.
-  scanf("%as", sp);
+  scanf("%as", sp); // expected-warning{{'a' is a non-standard length modifier}}
 
   // TODO: Warn that the 'a' conversion specifier is a C++11 feature.
   printf("%a", 1.0);