From: Hans Wennborg Date: Fri, 9 Mar 2012 10:10:54 +0000 (+0000) Subject: -Wformat-non-iso: warn about positional arguments (pr12017) X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f85626453123f9691bcef13cff963f556e209c27;p=clang -Wformat-non-iso: warn about positional arguments (pr12017) This renames the -Wformat-non-standard flag to -Wformat-non-iso, rewords the current warnings a bit (pointing out that a format string is not supported by ISO C rather than being "non standard"), and adds a warning about positional arguments. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@152403 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index 9dc8f8878d..9ec27ce91d 100644 --- a/include/clang/Analysis/Analyses/FormatString.h +++ b/include/clang/Analysis/Analyses/FormatString.h @@ -306,9 +306,9 @@ protected: LengthModifier LM; OptionalAmount FieldWidth; ConversionSpecifier CS; - /// Positional arguments, an IEEE extension: - /// IEEE Std 1003.1, 2004 Edition - /// http://www.opengroup.org/onlinepubs/009695399/functions/printf.html + /// Positional arguments, an IEEE extension: + /// IEEE Std 1003.1, 2004 Edition + /// http://www.opengroup.org/onlinepubs/009695399/functions/printf.html bool UsesPositionalArg; unsigned argIndex; public: @@ -598,6 +598,8 @@ public: virtual void HandleNullChar(const char *nullCharacter) {} + virtual void HandlePosition(const char *startPos, unsigned posLen) {} + virtual void HandleInvalidPosition(const char *startPos, unsigned posLen, PositionContext p) {} diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index f5de122e0e..26dcc40aba 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -302,7 +302,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 FormatNonStandard : DiagGroup<"format-non-iso">; 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 fadcdf9d36..034741ed88 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -4944,10 +4944,13 @@ 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_positional_arg: ExtWarn< + "positional arguments are not supported by ISO C">, InGroup, DefaultIgnore; def warn_format_non_standard: ExtWarn< - "'%0' is a non-standard %1">, InGroup, DefaultIgnore; + "'%0' %select{length modifier|conversion specifier}1 is not supported by ISO C">, + InGroup, DefaultIgnore; def warn_format_non_standard_conversion_spec: ExtWarn< - "using the length modifier '%0' with the conversion specifier '%1' is non-standard">, + "using length modifier '%0' with conversion specifier '%1' is not supported by ISO C">, InGroup, DefaultIgnore; def warn_printf_ignored_flag: Warning< "flag '%0' is ignored when flag '%1' is present">, diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp index 5785655c94..dd2f2406d4 100644 --- a/lib/Analysis/FormatString.cpp +++ b/lib/Analysis/FormatString.cpp @@ -156,6 +156,9 @@ clang::analyze_format_string::ParseArgPosition(FormatStringHandler &H, } if (Amt.getHowSpecified() == OptionalAmount::Constant && *(I++) == '$') { + // Warn that positional arguments are non-standard. + H.HandlePosition(Start, I - Start); + // Special case: '%0$', since this is an easy mistake. if (Amt.getConstantAmount() == 0) { H.HandleZeroPosition(Start, I - Start); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 3d9f5b3afd..ad763c346d 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1677,6 +1677,8 @@ public: const analyze_format_string::ConversionSpecifier &CS, const char *startSpecifier, unsigned specifierLen); + virtual void HandlePosition(const char *startPos, unsigned posLen); + virtual void HandleInvalidPosition(const char *startSpecifier, unsigned specifierLen, analyze_format_string::PositionContext p); @@ -1756,7 +1758,7 @@ 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", + << 0, getLocationOfByte(LM.getStart()), /*IsStringLocation*/true, getSpecifierRange(startSpecifier, specifierLen)); @@ -1766,7 +1768,7 @@ 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", + << 1, getLocationOfByte(CS.getStart()), /*IsStringLocation*/true, getSpecifierRange(startSpecifier, specifierLen)); @@ -1783,6 +1785,14 @@ void CheckFormatHandler::HandleNonStandardConversionSpecification( getSpecifierRange(startSpecifier, specifierLen)); } +void CheckFormatHandler::HandlePosition(const char *startPos, + unsigned posLen) { + EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_positional_arg), + getLocationOfByte(startPos), + /*IsStringLocation*/true, + getSpecifierRange(startPos, posLen)); +} + void CheckFormatHandler::HandleInvalidPosition(const char *startPos, unsigned posLen, analyze_format_string::PositionContext p) { diff --git a/test/Sema/format-strings-c90.c b/test/Sema/format-strings-c90.c index 63d58532cb..66ca507347 100644 --- a/test/Sema/format-strings-c90.c +++ b/test/Sema/format-strings-c90.c @@ -5,8 +5,8 @@ int scanf(const char * restrict, ...); int printf(const char *restrict, ...); void foo(char **sp, float *fp, int *ip) { - 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}} */ + scanf("%as", sp); /* expected-warning{{'a' length modifier is not supported by ISO C}} */ + scanf("%a[abc]", sp); /* expected-warning{{'a' length modifier is not supported by ISO C}} */ /* TODO: Warn that the 'a' conversion specifier is a C99 feature. */ scanf("%a", fp); @@ -21,10 +21,10 @@ void foo(char **sp, float *fp, int *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 *'}} - expected-warning{{'a' is a non-standard length modifier}} */ + expected-warning{{'a' length modifier is not supported by ISO C}} */ 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}} */ + expected-warning{{'a' length modifier is not supported by ISO C}} + expected-warning{{'S' conversion specifier is not supported by ISO C}} */ 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}} */ + expected-warning{{'a' length modifier is not supported by ISO C}} */ } diff --git a/test/Sema/format-strings-fixit.c b/test/Sema/format-strings-fixit.c index 80b1be0578..800691ecc8 100644 --- a/test/Sema/format-strings-fixit.c +++ b/test/Sema/format-strings-fixit.c @@ -35,7 +35,10 @@ void test() { printf("%0-f", 1.23); // - flag should stay // Positional arguments +#pragma clang diagnostic push // Don't warn about using positional arguments. +#pragma clang diagnostic ignored "-Wformat-non-iso" printf("%1$f:%2$.*3$f:%4$.*3$f\n", 1, 2, 3, 4); +#pragma clang diagnostic pop // Precision printf("%10.5d", 1l); // (bug 7394) @@ -46,7 +49,10 @@ void test() { // Bad length modifiers printf("%hhs", "foo"); +#pragma clang diagnostic push // Don't warn about using positional arguments. +#pragma clang diagnostic ignored "-Wformat-non-iso" printf("%1$zp", (void *)0); +#pragma clang diagnostic pop // Preserve the original formatting for unsigned integers. unsigned long val = 42; diff --git a/test/Sema/format-strings-non-iso.c b/test/Sema/format-strings-non-iso.c new file mode 100644 index 0000000000..ed8095f10a --- /dev/null +++ b/test/Sema/format-strings-non-iso.c @@ -0,0 +1,29 @@ +// 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' length modifier is not supported by ISO C}} + scanf("%qd", (long long *)0); // expected-warning{{'q' length modifier is not supported by ISO C}} + + // The 'm' length modifier. + scanf("%ms", &cp); // expected-warning{{'m' length modifier is not supported by ISO C}} + + // The 'S' and 'C' conversion specifiers. + printf("%S", L"foo"); // expected-warning{{'S' conversion specifier is not supported by ISO C}} + printf("%C", L'x'); // expected-warning{{'C' conversion specifier is not supported by ISO C}} + + // Combining 'L' with an integer conversion specifier. + printf("%Li", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'i' is not supported by ISO C}} + printf("%Lo", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'o' is not supported by ISO C}} + printf("%Lu", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'u' is not supported by ISO C}} + printf("%Lx", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'x' is not supported by ISO C}} + printf("%LX", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'X' is not supported by ISO C}} + + // Positional arguments. + printf("%1$d", 42); // expected-warning{{positional arguments are not supported by ISO C}} +} diff --git a/test/Sema/format-strings-non-standard.c b/test/Sema/format-strings-non-standard.c deleted file mode 100644 index a24d43a6e1..0000000000 --- a/test/Sema/format-strings-non-standard.c +++ /dev/null @@ -1,26 +0,0 @@ -// 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/SemaCXX/format-strings.cpp b/test/SemaCXX/format-strings.cpp index 314617d491..6b0df29353 100644 --- a/test/SemaCXX/format-strings.cpp +++ b/test/SemaCXX/format-strings.cpp @@ -9,7 +9,7 @@ extern int vprintf(const char *restrict, va_list); } void f(char **sp, float *fp) { - scanf("%as", sp); // expected-warning{{'a' is a non-standard length modifier}} + scanf("%as", sp); // expected-warning{{'a' length modifier is not supported by ISO C}} // TODO: Warn that the 'a' conversion specifier is a C++11 feature. printf("%a", 1.0);