From bbb6bb4952b77e57b842b4d3096848123ae690e7 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Sat, 8 Sep 2012 04:00:03 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163452 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/FormatString.h | 6 +- include/clang/Basic/DiagnosticSemaKinds.td | 1 + lib/Analysis/FormatString.cpp | 30 ++++++- lib/Analysis/PrintfFormatString.cpp | 8 +- lib/Analysis/ScanfFormatString.cpp | 8 +- lib/Sema/SemaChecking.cpp | 78 ++++++++++++------- test/Sema/format-strings-gnu.c | 55 +++++++++++++ test/Sema/format-strings-non-iso.c | 2 +- test/Sema/format-strings-scanf.c | 12 +-- test/Sema/format-strings.c | 11 --- 10 files changed, 151 insertions(+), 60 deletions(-) create mode 100644 test/Sema/format-strings-gnu.c diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index 7f50ee392b..c69f17a928 100644 --- 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 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 2b6cb4c34e..a222e0aad3 100644 --- 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; 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 e7ea48688d..45c722a5d5 100644 --- 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 +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(); +} + bool FormatSpecifier::namedTypeToLengthModifier(QualType QT, LengthModifier &LM) { assert(isa(QT) && "Expected a TypedefType"); diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp index 9e4c0fec4c..9a4f0ca6bb 100644 --- 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 2942400621..082c06af58 100644 --- 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 9bddfdc6f0..c477de8531 100644 --- 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 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 index 0000000000..f4910856b0 --- /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 ed8095f10a..e5ff8798f6 100644 --- 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 235ac11faa..be2adddc52 100644 --- 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 86b9296108..2e980a59a2 100644 --- 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); -- 2.40.0