Format strings: %Ld isn't available on Darwin or Windows.

This seems to be a GNU libc extension; we offer a fixit to %lld on
these platforms.

<rdar://problem/11518237>

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

diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h
index 7f50ee392be602b20c42439e7b582940ee30a21a..c69f17a928950341e103d6c96d216724e6b95a15 100644 (file)
--- a/include/clang/Analysis/Analyses/FormatString.h
+++ b/include/clang/Analysis/Analyses/FormatString.h
@@ -23,6 +23,8 @@
 
 namespace clang {
 
+class TargetInfo;
+
 //===----------------------------------------------------------------------===//
 /// Common components of both fprintf and fscanf format strings.
 namespace analyze_format_string {
@@ -348,10 +350,12 @@ public:
 
   bool usesPositionalArg() const { return UsesPositionalArg; }
 
-  bool hasValidLengthModifier() const;
+  bool hasValidLengthModifier(const TargetInfo &Target) const;
 
   bool hasStandardLengthModifier() const;
 
+  llvm::Optional<LengthModifier> getCorrectedLengthModifier() const;
+
   bool hasStandardConversionSpecifier(const LangOptions &LangOpt) const;
 
   bool hasStandardLengthConversionCombination() const;

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 2b6cb4c34e7e5cdcbf190c2e2251ced89989b3b3..a222e0aad30e0a71758931aa52f31b753dc717af 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5474,6 +5474,7 @@ def warn_scanf_scanlist_incomplete : Warning<
   "no closing ']' for '%%[' in scanf format string">,
   InGroup<Format>;
 def note_format_string_defined : Note<"format string is defined here">;
+def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
 def note_printf_c_str: Note<"did you mean to call the %0 method?">;
 
 def warn_null_arg : Warning<

diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp
index e7ea48688daea50a831dc65be20c50de86198d9d..45c722a5d5462a5e1afe027451233117ebfafb1f 100644 (file)
--- a/lib/Analysis/FormatString.cpp
+++ b/lib/Analysis/FormatString.cpp
@@ -14,6 +14,7 @@
 
 #include "FormatStringParsing.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/TargetInfo.h"
 
 using clang::analyze_format_string::ArgType;
 using clang::analyze_format_string::FormatStringHandler;
@@ -548,7 +549,7 @@ void OptionalAmount::toString(raw_ostream &os) const {
   }
 }
 
-bool FormatSpecifier::hasValidLengthModifier() const {
+bool FormatSpecifier::hasValidLengthModifier(const TargetInfo &Target) const {
   switch (LM.getKind()) {
     case LengthModifier::None:
       return true;
@@ -611,14 +612,15 @@ bool FormatSpecifier::hasValidLengthModifier() const {
         case ConversionSpecifier::gArg:
         case ConversionSpecifier::GArg:
           return true;
-        // GNU extension.
+        // GNU libc extension.
         case ConversionSpecifier::dArg:
         case ConversionSpecifier::iArg:
         case ConversionSpecifier::oArg:
         case ConversionSpecifier::uArg:
         case ConversionSpecifier::xArg:
         case ConversionSpecifier::XArg:
-          return true;
+          return !Target.getTriple().isOSDarwin() &&
+                 !Target.getTriple().isOSWindows();
         default:
           return false;
       }
@@ -719,6 +721,28 @@ bool FormatSpecifier::hasStandardLengthConversionCombination() const {
   return true;
 }
 
+llvm::Optional<LengthModifier>
+FormatSpecifier::getCorrectedLengthModifier() 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: {
+      LengthModifier FixedLM(LM);
+      FixedLM.setKind(LengthModifier::AsLongLong);
+      return FixedLM;
+    }
+    default:
+      break;
+    }
+  }
+
+  return llvm::Optional<LengthModifier>();
+}
+
 bool FormatSpecifier::namedTypeToLengthModifier(QualType QT,
                                                 LengthModifier &LM) {
   assert(isa<TypedefType>(QT) && "Expected a TypedefType");

diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index 9e4c0fec4c0473e6ee973ef0c5e23e17b866d52f..9a4f0ca6bba839c05d84ac45ff122662d49bc5eb 100644 (file)
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -482,9 +482,11 @@ bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
     namedTypeToLengthModifier(QT, LM);
 
   // If fixing the length modifier was enough, we are done.
-  const analyze_printf::ArgType &ATR = getArgType(Ctx, IsObjCLiteral);
-  if (hasValidLengthModifier() && ATR.isValid() && ATR.matchesType(Ctx, QT))
-    return true;
+  if (hasValidLengthModifier(Ctx.getTargetInfo())) {
+    const analyze_printf::ArgType &ATR = getArgType(Ctx, IsObjCLiteral);
+    if (ATR.isValid() && ATR.matchesType(Ctx, QT))
+      return true;
+  }
 
   // Set conversion specifier and disable any flags which do not apply to it.
   // Let typedefs to char fall through to int, as %c is silly for uint8_t.

diff --git a/lib/Analysis/ScanfFormatString.cpp b/lib/Analysis/ScanfFormatString.cpp
index 2942400621d1a18517a70c2eb251fee5386d50b5..082c06af5893233bcd44981a507e1ed4fb18651d 100644 (file)
--- a/lib/Analysis/ScanfFormatString.cpp
+++ b/lib/Analysis/ScanfFormatString.cpp
@@ -430,9 +430,11 @@ bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
     namedTypeToLengthModifier(PT, LM);
 
   // If fixing the length modifier was enough, we are done.
-  const analyze_scanf::ArgType &AT = getArgType(Ctx);
-  if (hasValidLengthModifier() && AT.isValid() && AT.matchesType(Ctx, QT))
-    return true;
+  if (hasValidLengthModifier(Ctx.getTargetInfo())) {
+    const analyze_scanf::ArgType &AT = getArgType(Ctx);
+    if (AT.isValid() && AT.matchesType(Ctx, QT))
+      return true;
+  }
 
   // Figure out the conversion specifier.
   if (PT->isRealFloatingType())

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 9bddfdc6f0b920b0a2e9e242c3534da6c66fe9d3..c477de8531b33909d62e2bd147aee60ae3f49a3e 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1938,6 +1938,11 @@ public:
   void HandleIncompleteSpecifier(const char *startSpecifier,
                                  unsigned specifierLen);
 
+  void HandleInvalidLengthModifier(
+      const analyze_format_string::FormatSpecifier &FS,
+      const analyze_format_string::ConversionSpecifier &CS,
+      const char *startSpecifier, unsigned specifierLen);
+
   void HandleNonStandardLengthModifier(
       const analyze_format_string::LengthModifier &LM,
       const char *startSpecifier, unsigned specifierLen);
@@ -2028,6 +2033,38 @@ void CheckFormatHandler::HandleIncompleteSpecifier(const char *startSpecifier,
                        getSpecifierRange(startSpecifier, specifierLen));
 }
 
+void CheckFormatHandler::HandleInvalidLengthModifier(
+    const analyze_format_string::FormatSpecifier &FS,
+    const analyze_format_string::ConversionSpecifier &CS,
+    const char *startSpecifier, unsigned specifierLen) {
+  using namespace analyze_format_string;
+
+  const LengthModifier &LM = FS.getLengthModifier();
+  CharSourceRange LMRange = getSpecifierRange(LM.getStart(), LM.getLength());
+
+  // See if we know how to fix this length modifier.
+  llvm::Optional<LengthModifier> FixedLM = FS.getCorrectedLengthModifier();
+  if (FixedLM) {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length)
+                           << LM.toString() << CS.toString(),
+                         getLocationOfByte(LM.getStart()),
+                         /*IsStringLocation*/true,
+                         getSpecifierRange(startSpecifier, specifierLen));
+
+    S.Diag(getLocationOfByte(LM.getStart()), diag::note_format_fix_specifier)
+      << FixedLM->toString()
+      << FixItHint::CreateReplacement(LMRange, FixedLM->toString());
+
+  } else {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length)
+                           << LM.toString() << CS.toString(),
+                         getLocationOfByte(LM.getStart()),
+                         /*IsStringLocation*/true,
+                         getSpecifierRange(startSpecifier, specifierLen),
+                         FixItHint::CreateRemoval(LMRange));
+  }
+}
+
 void CheckFormatHandler::HandleNonStandardLengthModifier(
     const analyze_format_string::LengthModifier &LM,
     const char *startSpecifier, unsigned specifierLen) {
@@ -2566,23 +2603,17 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
 
   // Check the length modifier is valid with the given conversion specifier.
   const LengthModifier &LM = FS.getLengthModifier();
-  if (!FS.hasValidLengthModifier())
-    EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length)
-                           << LM.toString() << CS.toString(),
-                         getLocationOfByte(LM.getStart()),
-                         /*IsStringLocation*/true,
-                         getSpecifierRange(startSpecifier, specifierLen),
-                         FixItHint::CreateRemoval(
-                           getSpecifierRange(LM.getStart(),
-                                             LM.getLength())));
-  if (!FS.hasStandardLengthModifier())
+  if (!FS.hasValidLengthModifier(S.getASTContext().getTargetInfo()))
+    HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen);
+  else if (!FS.hasStandardLengthModifier())
     HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen);
-  if (!FS.hasStandardConversionSpecifier(S.getLangOpts()))
-    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
-  if (!FS.hasStandardLengthConversionCombination())
+  else if (!FS.hasStandardLengthConversionCombination())
     HandleNonStandardConversionSpecification(LM, CS, startSpecifier,
                                              specifierLen);
 
+  if (!FS.hasStandardConversionSpecifier(S.getLangOpts()))
+    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
+
   // The remaining checks depend on the data arguments.
   if (HasVAListArg)
     return true;
@@ -2886,24 +2917,17 @@ bool CheckScanfHandler::HandleScanfSpecifier(
   
   // Check the length modifier is valid with the given conversion specifier.
   const LengthModifier &LM = FS.getLengthModifier();
-  if (!FS.hasValidLengthModifier()) {
-    const CharSourceRange &R = getSpecifierRange(LM.getStart(), LM.getLength());
-    EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length)
-                         << LM.toString() << CS.toString()
-                         << getSpecifierRange(startSpecifier, specifierLen),
-                         getLocationOfByte(LM.getStart()),
-                         /*IsStringLocation*/true, R,
-                         FixItHint::CreateRemoval(R));
-  }
-
-  if (!FS.hasStandardLengthModifier())
+  if (!FS.hasValidLengthModifier(S.getASTContext().getTargetInfo()))
+    HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen);
+  else if (!FS.hasStandardLengthModifier())
     HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen);
-  if (!FS.hasStandardConversionSpecifier(S.getLangOpts()))
-    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
-  if (!FS.hasStandardLengthConversionCombination())
+  else if (!FS.hasStandardLengthConversionCombination())
     HandleNonStandardConversionSpecification(LM, CS, startSpecifier,
                                              specifierLen);
 
+  if (!FS.hasStandardConversionSpecifier(S.getLangOpts()))
+    HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
+
   // The remaining checks depend on the data arguments.
   if (HasVAListArg)
     return true;

diff --git a/test/Sema/format-strings-gnu.c b/test/Sema/format-strings-gnu.c
new file mode 100644 (file)
index 0000000..f491085
--- /dev/null
+++ b/test/Sema/format-strings-gnu.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -triple i386-apple-darwin9 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple thumbv6-apple-ios4.0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-mingw32 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-pc-win32 %s
+
+// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux-gnu -DALLOWED %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-unknown-freebsd -DALLOWED %s
+
+int printf(const char *restrict, ...);
+int scanf(const char * restrict, ...) ;
+
+void test() {
+  long notLongEnough = 1;
+  long long quiteLong = 2;
+
+  printf("%Ld", notLongEnough); // expected-warning {{format specifies type 'long long' but the argument has type 'long'}}
+  printf("%Ld", quiteLong);
+
+#ifndef ALLOWED
+  // expected-warning@-4 {{length modifier 'L' results in undefined behavior or no effect with 'd' conversion specifier}}
+  // expected-note@-5 {{did you mean to use 'll'?}}
+
+  // expected-warning@-6 {{length modifier 'L' results in undefined behavior or no effect with 'd' conversion specifier}}
+  // expected-note@-7 {{did you mean to use 'll'?}}
+#endif
+}
+
+void testAlwaysInvalid() {
+  // We should not suggest 'll' here!
+  printf("%Lc", 'a'); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 'c' conversion specifier}}
+  printf("%Ls", "a"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
+}
+
+#ifdef ALLOWED
+// PR 9466: clang: doesn't know about %Lu, %Ld, and %Lx
+void printf_longlong(long long x, unsigned long long y) {
+  printf("%Ld", y); // no-warning
+  printf("%Lu", y); // no-warning
+  printf("%Lx", y); // no-warning
+  printf("%Ld", x); // no-warning
+  printf("%Lu", x); // no-warning
+  printf("%Lx", x); // no-warning
+  printf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
+}
+
+void scanf_longlong(long long *x, unsigned long long *y) {
+  scanf("%Ld", y); // no-warning
+  scanf("%Lu", y); // no-warning
+  scanf("%Lx", y); // no-warning
+  scanf("%Ld", x); // no-warning
+  scanf("%Lu", x); // no-warning
+  scanf("%Lx", x); // no-warning
+  scanf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
+}
+#endif

diff --git a/test/Sema/format-strings-non-iso.c b/test/Sema/format-strings-non-iso.c
index ed8095f10a4f57c9c08a848ebf5fd7ab09433c34..e5ff8798f63691a6ae8997dc8b267b814dc080b9 100644 (file)
--- a/test/Sema/format-strings-non-iso.c
+++ b/test/Sema/format-strings-non-iso.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -pedantic %s
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -verify -std=c99 -pedantic %s
 
 int printf(const char *restrict, ...);
 int scanf(const char * restrict, ...);

diff --git a/test/Sema/format-strings-scanf.c b/test/Sema/format-strings-scanf.c
index 235ac11faa1c0744c6539f0b6e3cc2c9400f918c..be2adddc52b290c1a19d9f58d9fc1c30e3fb1626 100644 (file)
--- a/test/Sema/format-strings-scanf.c
+++ b/test/Sema/format-strings-scanf.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple i386-apple-darwin9 -Wformat-nonliteral %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral %s
 
 // Test that -Wformat=0 works:
 // RUN: %clang_cc1 -fsyntax-only -Werror -Wformat=0 %s
@@ -113,16 +113,6 @@ void test_alloc_extension(char **sp, wchar_t **lsp, float *fp) {
   scanf("%m[abc]", fp); // expected-warning{{format specifies type 'char **' but the argument has type 'float *'}}
 }
 
-void test_longlong(long long *x, unsigned long long *y) {
-  scanf("%Ld", y); // no-warning
-  scanf("%Lu", y); // no-warning
-  scanf("%Lx", y); // no-warning
-  scanf("%Ld", x); // no-warning
-  scanf("%Lu", x); // no-warning
-  scanf("%Lx", x); // no-warning
-  scanf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
-}
-
 void test_quad(int *x, long long *llx) {
   scanf("%qd", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'int *'}}
   scanf("%qd", llx); // no-warning

diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index 86b9296108c6e632596419c4a7a2cb91602b3c16..2e980a59a2d2bdbc56f3c1932ee1506bda2a6d4e 100644 (file)
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -531,17 +531,6 @@ void pr9751() {
          0.0); // expected-warning{{format specifies}}
 }
 
-// PR 9466: clang: doesn't know about %Lu, %Ld, and %Lx 
-void printf_longlong(long long x, unsigned long long y) {
-  printf("%Ld", y); // no-warning
-  printf("%Lu", y); // no-warning
-  printf("%Lx", y); // no-warning
-  printf("%Ld", x); // no-warning
-  printf("%Lu", x); // no-warning
-  printf("%Lx", x); // no-warning
-  printf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
-}
-
 void __attribute__((format(strfmon,1,2))) monformat(const char *fmt, ...);
 void __attribute__((format(strftime,1,0))) dateformat(const char *fmt);