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 <[email protected]>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 <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > > > -- > Alexey Samsonov, MSK > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
