From 76517426dc8bf7734c07eefc35171a6bfdba1a2b Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Wed, 22 Feb 2012 10:17:01 +0000 Subject: [PATCH] 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 --- .../clang/Analysis/Analyses/FormatString.h | 6 ++ include/clang/Basic/DiagnosticGroups.td | 1 + include/clang/Basic/DiagnosticSemaKinds.td | 5 ++ lib/Analysis/FormatString.cpp | 71 +++++++++++++++++++ lib/Sema/SemaChecking.cpp | 62 +++++++++++++++- test/Sema/format-strings-c90.c | 15 ++-- test/Sema/format-strings-non-standard.c | 26 +++++++ test/Sema/format-strings.c | 1 - test/SemaCXX/format-strings.cpp | 3 +- 9 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 test/Sema/format-strings-non-standard.c 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; +def warn_format_non_standard: ExtWarn< + "'%0' is a non-standard %1">, InGroup, DefaultIgnore; +def warn_format_non_standard_conversion_spec: ExtWarn< + "using the length modifier '%0' with the conversion specifier '%1' is non-standard">, + InGroup, DefaultIgnore; def warn_printf_ignored_flag: Warning< "flag '%0' is ignored when flag '%1' is present">, InGroup; 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); -- 2.40.0