[cfe-commits] r160966 - in /cfe/trunk: lib/Analysis/PrintfFormatString.cpp lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-scanf.c test/Sema/format-strings.c
Chandler Carruth
chandlerc at google.com
Tue Jul 31 00:42:04 PDT 2012
I can find nothing in any spec which would shed light on what the correct
behavior is for the warning, but it seems unlikely to be important to give
fprintf a pointer to a volatile int as opposed to a normal int....
On Tue, Jul 31, 2012 at 12:27 AM, Alexey Samsonov <samsonov at google.com>wrote:
> Hello Hans,
>
> After this patch one of our tests failed with the following error:
>
> error: format specifies type 'int *' but the argument has type 'volatile int *' [-Werror,-Wformat]
> fprintf(stdout, "printing a string: %s%n\n", s, &a);
> ~~ ^~
>
> Do you think that we should fix the code, or Clang shouldn't produce the
> warning in this case?
>
> On Mon, Jul 30, 2012 at 9:11 PM, Hans Wennborg <hans at hanshq.net> wrote:
>
>> Author: hans
>> Date: Mon Jul 30 12:11:32 2012
>> New Revision: 160966
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=160966&view=rev
>> Log:
>> Make -Wformat check the argument type for %n.
>>
>> This makes Clang check that the corresponding argument for "%n" in a
>> format string is a pointer to int.
>>
>> Modified:
>> cfe/trunk/lib/Analysis/PrintfFormatString.cpp
>> cfe/trunk/lib/Analysis/ScanfFormatString.cpp
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>> cfe/trunk/test/Sema/format-strings-scanf.c
>> cfe/trunk/test/Sema/format-strings.c
>>
>> Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=160966&r1=160965&r2=160966&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
>> +++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Mon Jul 30 12:11:32 2012
>> @@ -330,6 +330,8 @@
>> return ArgTypeResult(Ctx.WCharTy, "wchar_t");
>> case ConversionSpecifier::pArg:
>> return ArgTypeResult::CPointerTy;
>> + case ConversionSpecifier::nArg:
>> + return Ctx.getPointerType(Ctx.IntTy);
>> case ConversionSpecifier::ObjCObjArg:
>> return ArgTypeResult::ObjCPointerTy;
>> default:
>> @@ -342,6 +344,10 @@
>>
>> bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
>> ASTContext &Ctx, bool IsObjCLiteral) {
>> + // %n is different from other conversion specifiers; don't try to fix
>> it.
>> + if (CS.getKind() == ConversionSpecifier::nArg)
>> + return false;
>> +
>> // Handle Objective-C objects first. Note that while the '%@'
>> specifier will
>> // not warn for structure pointer or void pointer arguments (because
>> that's
>> // how CoreFoundation objects are implemented), we only show a fixit
>> for '%@'
>>
>> Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=160966&r1=160965&r2=160966&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)
>> +++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Mon Jul 30 12:11:32 2012
>> @@ -303,6 +303,9 @@
>> case ConversionSpecifier::pArg:
>> return
>> ScanfArgTypeResult(ArgTypeResult(ArgTypeResult::CPointerTy));
>>
>> + case ConversionSpecifier::nArg:
>> + return ArgTypeResult(Ctx.IntTy);
>> +
>> default:
>> break;
>> }
>> @@ -315,6 +318,10 @@
>> if (!QT->isPointerType())
>> return false;
>>
>> + // %n is different from other conversion specifiers; don't try to fix
>> it.
>> + if (CS.getKind() == ConversionSpecifier::nArg)
>> + return false;
>> +
>> QualType PT = QT->getPointeeType();
>>
>> // If it's an enum, get its underlying type.
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=160966&r1=160965&r2=160966&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jul 30 12:11:32 2012
>> @@ -2568,8 +2568,6 @@
>> getLocationOfByte(CS.getStart()),
>> /*IsStringLocation*/true,
>> getSpecifierRange(startSpecifier,
>> specifierLen));
>> - // Continue checking the other format specifiers.
>> - return true;
>> }
>>
>> // The remaining checks depend on the data arguments.
>>
>> Modified: cfe/trunk/test/Sema/format-strings-scanf.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-scanf.c?rev=160966&r1=160965&r2=160966&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/format-strings-scanf.c (original)
>> +++ cfe/trunk/test/Sema/format-strings-scanf.c Mon Jul 30 12:11:32 2012
>> @@ -121,3 +121,8 @@
>> scanf("%qd", x); // expected-warning{{format specifies type 'long long
>> *' but the argument has type 'int *'}}
>> scanf("%qd", llx); // no-warning
>> }
>> +
>> +void test_writeback(int *x) {
>> + scanf("%n", (void*)0); // expected-warning{{format specifies type 'int
>> *' but the argument has type 'void *'}}
>> + scanf("%n %c", x, x); // expected-warning{{format specifies type 'char
>> *' but the argument has type 'int *'}}
>> +}
>>
>> Modified: cfe/trunk/test/Sema/format-strings.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=160966&r1=160965&r2=160966&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/format-strings.c (original)
>> +++ cfe/trunk/test/Sema/format-strings.c Mon Jul 30 12:11:32 2012
>> @@ -91,6 +91,7 @@
>>
>> printf("%n",&x); // expected-warning {{'%n' in format string
>> discouraged}}
>> sprintf(b,"%d%%%n",1, &x); // expected-warning {{'%n' in format string
>> dis}}
>> + printf("%n",b); // expected-warning {{'%n' in format string
>> discouraged}} expected-warning{{format specifies type 'int *' but the
>> argument has type 'char *'}}
>> }
>>
>> void check_invalid_specifier(FILE* fp, char *buf)
>> @@ -316,14 +317,14 @@
>> // Bad flag usage
>> printf("%#p", (void *) 0); // expected-warning{{flag '#' results in
>> undefined behavior with 'p' conversion specifier}}
>> printf("%0d", -1); // no-warning
>> - printf("%#n", (void *) 0); // expected-warning{{flag '#' results in
>> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
>> '%n' in format string discouraged (potentially insecure)}}
>> - printf("%-n", (void *) 0); // expected-warning{{flag '-' results in
>> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
>> '%n' in format string discouraged (potentially insecure)}}
>> + printf("%#n", (int *) 0); // expected-warning{{flag '#' results in
>> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
>> '%n' in format string discouraged (potentially insecure)}}
>> + printf("%-n", (int *) 0); // expected-warning{{flag '-' results in
>> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
>> '%n' in format string discouraged (potentially insecure)}}
>> printf("%-p", (void *) 0); // no-warning
>>
>> // Bad optional amount use
>> printf("%.2c", 'a'); // expected-warning{{precision used with 'c'
>> conversion specifier, resulting in undefined behavior}}
>> - printf("%1n", (void *) 0); // expected-warning{{field width used with
>> 'n' conversion specifier, resulting in undefined behavior}}
>> expected-warning{{use of '%n' in format string discouraged (potentially
>> insecure)}}
>> - printf("%.9n", (void *) 0); // expected-warning{{precision used with
>> 'n' conversion specifier, resulting in undefined behavior}}
>> expected-warning{{use of '%n' in format string discouraged (potentially
>> insecure)}}
>> + printf("%1n", (int *) 0); // expected-warning{{field width used with
>> 'n' conversion specifier, resulting in undefined behavior}}
>> expected-warning{{use of '%n' in format string discouraged (potentially
>> insecure)}}
>> + printf("%.9n", (int *) 0); // expected-warning{{precision used with
>> 'n' conversion specifier, resulting in undefined behavior}}
>> expected-warning{{use of '%n' in format string discouraged (potentially
>> insecure)}}
>>
>> // Ignored flags
>> printf("% +f", 1.23); // expected-warning{{flag ' ' is ignored when
>> flag '+' is present}}
>> @@ -436,8 +437,9 @@
>> printf("%18$s\n", 1, "foo"); // expected-warning{{data argument
>> position '18' exceeds the number of data arguments (2)}}
>>
>> const char kFormat3[] = "%n"; // expected-note{{format string is
>> defined here}}
>> - printf(kFormat3, "as"); // expected-warning{{use of '%n' in format
>> string discouraged}}
>> - printf("%n", "as"); // expected-warning{{use of '%n' in format string
>> discouraged}}
>> + printf(kFormat3, (int*)NULL); // expected-warning{{use of '%n' in
>> format string discouraged}}
>> + printf("%n", (int*)NULL); // expected-warning{{use of '%n' in format
>> string discouraged}}
>> +
>>
>> const char kFormat4[] = "%y"; // expected-note{{format string is
>> defined here}}
>> printf(kFormat4, 5); // expected-warning{{invalid conversion specifier
>> 'y'}}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
>
> --
> Alexey Samsonov, MSK
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120731/04c3dd0d/attachment.html>
More information about the cfe-commits
mailing list