diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index f7a658e..783a27c 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 LangOptions &LangOpt) const; + + bool hasStandardConversionSpecifier(const LangOptions &LangOpt) const; + + bool hasStandardLengthConversionCombination(const LangOptions &LangOpt) const; }; } // end analyze_format_string namespace @@ -600,6 +606,10 @@ public: virtual void HandleIncompleteSpecifier(const char *startSpecifier, unsigned specifierLen) {} + virtual void HandleNonStandardLengthModifier(const LengthModifier &LM, + const char *startSpecifier, + unsigned specifierLen) {} + // Printf-specific handlers. virtual bool HandleInvalidPrintfConversionSpecifier( diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index d4c7336..65f86ae 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -290,10 +290,12 @@ 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, - FormatSecurity, FormatY2K, FormatInvalidSpecifier]>, + FormatSecurity, FormatY2K, FormatInvalidSpecifier, + FormatNonStandard]>, DiagCategory<"Format String Issue">; def FormatNonLiteral : DiagGroup<"format-nonliteral", [FormatSecurity]>; def Format2 : DiagGroup<"format=2", diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 4f0eea3..0b5ed90 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: Warning< + "'%0' is a non-standard %1">, InGroup; +def warn_format_non_standard_conversion_spec: Warning< + "using the length modifier '%0' with the conversion specifier '%1' is non-standard">, + InGroup; 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 fcc293b..cd306b3 100644 --- a/lib/Analysis/FormatString.cpp +++ b/lib/Analysis/FormatString.cpp @@ -602,3 +602,75 @@ bool FormatSpecifier::hasValidLengthModifier() const { } llvm_unreachable("Invalid LengthModifier Kind!"); } + +bool FormatSpecifier::hasStandardLengthModifier(const LangOptions &LangOpt) 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: + return LangOpt.GNUMode; + 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 LangOptions &LangOpt) 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 LangOpt.GNUMode; + default: + return true; + } + } + return true; +} diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 1d75ef6..d176147 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1649,7 +1649,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); @@ -1725,6 +1738,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) { @@ -2143,6 +2187,13 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier FixItHint::CreateRemoval( getSpecifierRange(LM.getStart(), LM.getLength()))); + if (!FS.hasStandardLengthModifier(S.getLangOptions())) + HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen); + if (!FS.hasStandardConversionSpecifier(S.getLangOptions())) + HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); + if (!FS.hasStandardLengthConversionCombination(S.getLangOptions())) + HandleNonStandardConversionSpecification(LM, CS, startSpecifier, + specifierLen); // Are we using '%n'? if (CS.getKind() == ConversionSpecifier::nArg) { @@ -2329,6 +2380,14 @@ bool CheckScanfHandler::HandleScanfSpecifier( FixItHint::CreateRemoval(R)); } + if (!FS.hasStandardLengthModifier(S.getLangOptions())) + HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen); + if (!FS.hasStandardConversionSpecifier(S.getLangOptions())) + HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); + if (!FS.hasStandardLengthConversionCombination(S.getLangOptions())) + HandleNonStandardConversionSpecification(LM, CS, startSpecifier, + specifierLen); + // The remaining checks depend on the data arguments. if (HasVAListArg) return true; @@ -4955,4 +5014,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 0b32e82..63d5853 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-int-typedefs.c b/test/Sema/format-strings-int-typedefs.c index 341d49c..787d5fe 100644 --- a/test/Sema/format-strings-int-typedefs.c +++ b/test/Sema/format-strings-int-typedefs.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify -Wno-format-non-standard %s int printf(char const *, ...); int scanf(char const *, ...); diff --git a/test/Sema/format-strings-non-standard-gnu.c b/test/Sema/format-strings-non-standard-gnu.c new file mode 100644 index 0000000..c557ddf --- /dev/null +++ b/test/Sema/format-strings-non-standard-gnu.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=gnu99 %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); // no-warning + + // 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); // no-warning + printf("%Lo", (long long)42); // no-warning + printf("%Lu", (long long)42); // no-warning + printf("%Lx", (long long)42); // no-warning + printf("%LX", (long long)42); // no-warning +} diff --git a/test/Sema/format-strings-non-standard.c b/test/Sema/format-strings-non-standard.c new file mode 100644 index 0000000..5aa1bfe --- /dev/null +++ b/test/Sema/format-strings-non-standard.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 %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-scanf.c b/test/Sema/format-strings-scanf.c index e94af5a..9da0d97 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 -triple i386-apple-darwin9 -Wformat-nonliteral -Wno-format-non-standard %s // Test that -Wformat=0 works: // RUN: %clang_cc1 -fsyntax-only -Werror -Wformat=0 %s diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c index e6ce6e3..4a4fc2f 100644 --- a/test/Sema/format-strings.c +++ b/test/Sema/format-strings.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral -Wno-format-non-standard %s #include typedef __typeof(sizeof(int)) size_t; @@ -245,7 +245,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 *'}} }