[cfe-commits] r97297 - in /cfe/trunk: include/clang/Analysis/Analyses/PrintfFormatString.h include/clang/Basic/DiagnosticSemaKinds.td lib/Analysis/PrintfFormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings.c
Ted Kremenek
kremenek at apple.com
Fri Feb 26 17:41:03 PST 2010
Author: kremenek
Date: Fri Feb 26 19:41:03 2010
New Revision: 97297
URL: http://llvm.org/viewvc/llvm-project?rev=97297&view=rev
Log:
For printf format string checking, add support for positional format strings.
Along the way, coelesce some of the diagnostics.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Analysis/PrintfFormatString.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-strings.c
Modified: cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h?rev=97297&r1=97296&r2=97297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h Fri Feb 26 19:41:03 2010
@@ -150,13 +150,17 @@
class OptionalAmount {
public:
- enum HowSpecified { NotSpecified, Constant, Arg };
+ enum HowSpecified { NotSpecified, Constant, Arg, Invalid };
OptionalAmount(HowSpecified h, unsigned i, const char *st)
: start(st), hs(h), amt(i) {}
- OptionalAmount()
- : start(0), hs(NotSpecified), amt(0) {}
+ OptionalAmount(bool b = true)
+ : start(0), hs(b ? NotSpecified : Invalid), amt(0) {}
+
+ bool isInvalid() const {
+ return hs == Invalid;
+ }
HowSpecified getHowSpecified() const { return hs; }
@@ -191,6 +195,7 @@
unsigned HasSpacePrefix : 1;
unsigned HasAlternativeForm : 1;
unsigned HasLeadingZeroes : 1;
+ unsigned UsesPositionalArg : 1;
unsigned argIndex;
ConversionSpecifier CS;
OptionalAmount FieldWidth;
@@ -198,7 +203,8 @@
public:
FormatSpecifier() : LM(None),
IsLeftJustified(0), HasPlusPrefix(0), HasSpacePrefix(0),
- HasAlternativeForm(0), HasLeadingZeroes(0), argIndex(0) {}
+ HasAlternativeForm(0), HasLeadingZeroes(0), UsesPositionalArg(0),
+ argIndex(0) {}
static FormatSpecifier Parse(const char *beg, const char *end);
@@ -214,6 +220,7 @@
void setHasSpacePrefix() { HasSpacePrefix = 1; }
void setHasAlternativeForm() { HasAlternativeForm = 1; }
void setHasLeadingZeros() { HasLeadingZeroes = 1; }
+ void setUsesPositionalArg() { UsesPositionalArg = 1; }
void setArgIndex(unsigned i) {
assert(CS.consumesDataArgument());
@@ -263,8 +270,11 @@
bool hasAlternativeForm() const { return (bool) HasAlternativeForm; }
bool hasLeadingZeros() const { return (bool) HasLeadingZeroes; }
bool hasSpacePrefix() const { return (bool) HasSpacePrefix; }
+ bool usesPositionalArg() const { return (bool) UsesPositionalArg; }
};
+enum PositionContext { FieldWidthPos = 0, PrecisionPos = 1 };
+
class FormatStringHandler {
public:
FormatStringHandler() {}
@@ -275,6 +285,11 @@
virtual void HandleNullChar(const char *nullCharacter) {}
+ virtual void HandleInvalidPosition(const char *startPos, unsigned posLen,
+ PositionContext p) {}
+
+ virtual void HandleZeroPosition(const char *startPos, unsigned posLen) {}
+
virtual bool
HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
const char *startSpecifier,
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=97297&r1=97296&r2=97297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Feb 26 19:41:03 2010
@@ -2534,6 +2534,15 @@
def warn_printf_conversion_argument_type_mismatch : Warning<
"conversion specifies type %0 but the argument has type %1">,
InGroup<Format>;
+def warn_printf_zero_positional_specifier : Warning<
+ "position arguments in format strings start counting at 1 (not 0)">,
+ InGroup<Format>;
+def warn_printf_invalid_positional_specifier : Warning<
+ "invalid position specified for %select{field width|field precision}0">,
+ InGroup<Format>;
+def warn_printf_mix_positional_nonpositional_args : Warning<
+ "cannot mix positional and non-positional arguments in format string">,
+ InGroup<Format>;
def warn_null_arg : Warning<
"null passed to a callee which requires a non-null argument">,
InGroup<NonNull>;
@@ -2543,15 +2552,10 @@
"format string should not be a wide string">, InGroup<Format>;
def warn_printf_format_string_contains_null_char : Warning<
"format string contains '\\0' within the string body">, InGroup<Format>;
-def warn_printf_asterisk_width_missing_arg : Warning<
- "'*' specified field width is missing a matching 'int' argument">;
-def warn_printf_asterisk_precision_missing_arg : Warning<
- "'.*' specified field precision is missing a matching 'int' argument">;
-def warn_printf_asterisk_width_wrong_type : Warning<
- "field width should have type %0, but argument has type %1">,
- InGroup<Format>;
-def warn_printf_asterisk_precision_wrong_type : Warning<
- "field precision should have type %0, but argument has type %1">,
+def warn_printf_asterisk_missing_arg : Warning<
+ "'%select{*|.*}0' specified field %select{width|precision}0 is missing a matching 'int' argument">;
+def warn_printf_asterisk_wrong_type : Warning<
+ "field %select{width|precision}0 should have type %1, but argument has type %2">,
InGroup<Format>;
def warn_printf_nonsensical_precision: Warning<
"precision used in '%0' conversion specifier (where it has no meaning)">,
Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=97297&r1=97296&r2=97297&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Fri Feb 26 19:41:03 2010
@@ -15,10 +15,12 @@
#include "clang/Analysis/Analyses/PrintfFormatString.h"
#include "clang/AST/ASTContext.h"
-using clang::analyze_printf::FormatSpecifier;
-using clang::analyze_printf::OptionalAmount;
using clang::analyze_printf::ArgTypeResult;
+using clang::analyze_printf::FormatSpecifier;
using clang::analyze_printf::FormatStringHandler;
+using clang::analyze_printf::OptionalAmount;
+using clang::analyze_printf::PositionContext;
+
using namespace clang;
namespace {
@@ -62,36 +64,141 @@
// Methods for parsing format strings.
//===----------------------------------------------------------------------===//
-static OptionalAmount ParseAmount(const char *&Beg, const char *E,
- unsigned &argIndex) {
+static OptionalAmount ParseAmount(const char *&Beg, const char *E) {
const char *I = Beg;
UpdateOnReturn <const char*> UpdateBeg(Beg, I);
- bool foundDigits = false;
unsigned accumulator = 0;
for ( ; I != E; ++I) {
char c = *I;
if (c >= '0' && c <= '9') {
- foundDigits = true;
+ // Ignore '0' on the first character.
+ if (c == '0' && I == Beg)
+ break;
accumulator += (accumulator * 10) + (c - '0');
continue;
}
- if (foundDigits)
+ if (accumulator)
return OptionalAmount(OptionalAmount::Constant, accumulator, Beg);
- if (c == '*') {
- ++I;
- return OptionalAmount(OptionalAmount::Arg, argIndex++, Beg);
- }
-
break;
}
return OptionalAmount();
}
+static OptionalAmount ParseNonPositionAmount(const char *&Beg, const char *E,
+ unsigned &argIndex) {
+ if (*Beg == '*') {
+ ++Beg;
+ return OptionalAmount(OptionalAmount::Arg, argIndex++, Beg);
+ }
+
+ return ParseAmount(Beg, E);
+}
+
+static OptionalAmount ParsePositionAmount(FormatStringHandler &H,
+ const char *Start,
+ const char *&Beg, const char *E,
+ PositionContext p) {
+ if (*Beg == '*') {
+ const char *I = Beg + 1;
+ const OptionalAmount &Amt = ParseAmount(I, E);
+
+ if (Amt.getHowSpecified() == OptionalAmount::NotSpecified) {
+ H.HandleInvalidPosition(Beg, I - Beg, p);
+ return OptionalAmount(false);
+ }
+
+ if (I== E) {
+ // No more characters left?
+ H.HandleIncompleteFormatSpecifier(Start, E - Start);
+ return OptionalAmount(false);
+ }
+
+ if (*I == '$') {
+ const char *Tmp = Beg;
+ Beg = ++I;
+ return OptionalAmount(OptionalAmount::Arg, Amt.getConstantAmount() - 1,
+ Tmp);
+ }
+
+ H.HandleInvalidPosition(Beg, I - Beg, p);
+ return OptionalAmount(false);
+ }
+
+ return ParseAmount(Beg, E);
+}
+
+static bool ParsePrecision(FormatStringHandler &H, FormatSpecifier &FS,
+ const char *Start, const char *&Beg, const char *E,
+ unsigned *argIndex) {
+ if (argIndex) {
+ FS.setPrecision(ParseNonPositionAmount(Beg, E, *argIndex));
+ }
+ else {
+ const OptionalAmount Amt = ParsePositionAmount(H, Start, Beg, E,
+ analyze_printf::PrecisionPos);
+ if (Amt.isInvalid())
+ return true;
+ FS.setPrecision(Amt);
+ }
+ return false;
+}
+
+static bool ParseFieldWidth(FormatStringHandler &H, FormatSpecifier &FS,
+ const char *Start, const char *&Beg, const char *E,
+ unsigned *argIndex) {
+ // FIXME: Support negative field widths.
+ if (argIndex) {
+ FS.setFieldWidth(ParseNonPositionAmount(Beg, E, *argIndex));
+ }
+ else {
+ const OptionalAmount Amt = ParsePositionAmount(H, Start, Beg, E,
+ analyze_printf::FieldWidthPos);
+ if (Amt.isInvalid())
+ return true;
+ FS.setFieldWidth(Amt);
+ }
+ return false;
+}
+
+
+static bool ParseArgPosition(FormatStringHandler &H,
+ FormatSpecifier &FS, const char *Start,
+ const char *&Beg, const char *E) {
+
+ using namespace clang::analyze_printf;
+ const char *I = Beg;
+
+ const OptionalAmount &Amt = ParseAmount(I, E);
+
+ if (I == E) {
+ // No more characters left?
+ H.HandleIncompleteFormatSpecifier(Start, E - Start);
+ return true;
+ }
+
+ if (Amt.getHowSpecified() == OptionalAmount::Constant && *(I++) == '$') {
+ FS.setArgIndex(Amt.getConstantAmount() - 1);
+ FS.setUsesPositionalArg();
+ // Update the caller's pointer if we decided to consume
+ // these characters.
+ Beg = I;
+ return false;
+ }
+
+ // Special case: '%0$', since this is an easy mistake.
+ if (*I == '0' && (I+1) != E && *(I+1) == '$') {
+ H.HandleZeroPosition(Start, I - Start + 2);
+ return true;
+ }
+
+ return false;
+}
+
static FormatSpecifierResult ParseFormatSpecifier(FormatStringHandler &H,
const char *&Beg,
const char *E,
@@ -128,6 +235,14 @@
}
FormatSpecifier FS;
+ if (ParseArgPosition(H, FS, Start, I, E))
+ return true;
+
+ if (I == E) {
+ // No more characters left?
+ H.HandleIncompleteFormatSpecifier(Start, E - Start);
+ return true;
+ }
// Look for flags (if any).
bool hasMore = true;
@@ -151,7 +266,9 @@
}
// Look for the field width (if any).
- FS.setFieldWidth(ParseAmount(I, E, argIndex));
+ if (ParseFieldWidth(H, FS, Start, I, E,
+ FS.usesPositionalArg() ? 0 : &argIndex))
+ return true;
if (I == E) {
// No more characters left?
@@ -167,7 +284,9 @@
return true;
}
- FS.setPrecision(ParseAmount(I, E, argIndex));
+ if (ParsePrecision(H, FS, Start, I, E,
+ FS.usesPositionalArg() ? 0 : &argIndex))
+ return true;
if (I == E) {
// No more characters left?
@@ -245,7 +364,7 @@
}
ConversionSpecifier CS(conversionPosition, k);
FS.setConversionSpecifier(CS);
- if (CS.consumesDataArgument())
+ if (CS.consumesDataArgument() && !FS.usesPositionalArg())
FS.setArgIndex(argIndex++);
if (k == ConversionSpecifier::InvalidSpecifier) {
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=97297&r1=97296&r2=97297&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Feb 26 19:41:03 2010
@@ -110,7 +110,8 @@
}
if (format_idx < TheCall->getNumArgs()) {
Expr *Format = TheCall->getArg(format_idx)->IgnoreParenCasts();
- if (!Format->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
+ if (!Format->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNull))
return true;
}
}
@@ -1051,6 +1052,8 @@
const CallExpr *TheCall;
unsigned FormatIdx;
llvm::BitVector CoveredArgs;
+ bool usesPositionalArgs;
+ bool atFirstArg;
public:
CheckPrintfHandler(Sema &s, const StringLiteral *fexpr,
const Expr *origFormatExpr,
@@ -1061,7 +1064,8 @@
NumDataArgs(numDataArgs),
IsObjCLiteral(isObjCLiteral), Beg(beg),
HasVAListArg(hasVAListArg),
- TheCall(theCall), FormatIdx(formatIdx) {
+ TheCall(theCall), FormatIdx(formatIdx),
+ usesPositionalArgs(false), atFirstArg(true) {
CoveredArgs.resize(numDataArgs);
CoveredArgs.reset();
}
@@ -1076,6 +1080,12 @@
const char *startSpecifier,
unsigned specifierLen);
+ virtual void HandleInvalidPosition(const char *startSpecifier,
+ unsigned specifierLen,
+ analyze_printf::PositionContext p);
+
+ virtual void HandleZeroPosition(const char *startPos, unsigned posLen);
+
void HandleNullChar(const char *nullCharacter);
bool HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1087,9 +1097,8 @@
unsigned specifierLen);
SourceLocation getLocationOfByte(const char *x);
- bool HandleAmount(const analyze_printf::OptionalAmount &Amt,
- unsigned MissingArgDiag, unsigned BadTypeDiag,
- const char *startSpecifier, unsigned specifierLen);
+ bool HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned k,
+ const char *startSpecifier, unsigned specifierLen);
void HandleFlags(const analyze_printf::FormatSpecifier &FS,
llvm::StringRef flag, llvm::StringRef cspec,
const char *startSpecifier, unsigned specifierLen);
@@ -1120,6 +1129,21 @@
<< getFormatSpecifierRange(startSpecifier, specifierLen);
}
+void
+CheckPrintfHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
+ analyze_printf::PositionContext p) {
+ SourceLocation Loc = getLocationOfByte(startPos);
+ S.Diag(Loc, diag::warn_printf_invalid_positional_specifier)
+ << (unsigned) p << getFormatSpecifierRange(startPos, posLen);
+}
+
+void CheckPrintfHandler::HandleZeroPosition(const char *startPos,
+ unsigned posLen) {
+ SourceLocation Loc = getLocationOfByte(startPos);
+ S.Diag(Loc, diag::warn_printf_zero_positional_specifier)
+ << getFormatSpecifierRange(startPos, posLen);
+}
+
bool CheckPrintfHandler::
HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
const char *startSpecifier,
@@ -1176,17 +1200,16 @@
bool
CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt,
- unsigned MissingArgDiag,
- unsigned BadTypeDiag,
- const char *startSpecifier,
+ unsigned k, const char *startSpecifier,
unsigned specifierLen) {
if (Amt.hasDataArgument()) {
if (!HasVAListArg) {
unsigned argIndex = Amt.getArgIndex();
if (argIndex >= NumDataArgs) {
- S.Diag(getLocationOfByte(Amt.getStart()), MissingArgDiag)
- << getFormatSpecifierRange(startSpecifier, specifierLen);
+ S.Diag(getLocationOfByte(Amt.getStart()),
+ diag::warn_printf_asterisk_missing_arg)
+ << k << getFormatSpecifierRange(startSpecifier, specifierLen);
// Don't do any more checking. We will just emit
// spurious errors.
return false;
@@ -1204,7 +1227,9 @@
assert(ATR.isValid());
if (!ATR.matchesType(S.Context, T)) {
- S.Diag(getLocationOfByte(Amt.getStart()), BadTypeDiag)
+ S.Diag(getLocationOfByte(Amt.getStart()),
+ diag::warn_printf_asterisk_wrong_type)
+ << k
<< ATR.getRepresentativeType(S.Context) << T
<< getFormatSpecifierRange(startSpecifier, specifierLen)
<< Arg->getSourceRange();
@@ -1223,22 +1248,30 @@
const char *startSpecifier,
unsigned specifierLen) {
- using namespace analyze_printf;
+ using namespace analyze_printf;
const ConversionSpecifier &CS = FS.getConversionSpecifier();
+ if (atFirstArg) {
+ atFirstArg = false;
+ usesPositionalArgs = FS.usesPositionalArg();
+ }
+ else if (usesPositionalArgs != FS.usesPositionalArg()) {
+ // Cannot mix-and-match positional and non-positional arguments.
+ S.Diag(getLocationOfByte(CS.getStart()),
+ diag::warn_printf_mix_positional_nonpositional_args)
+ << getFormatSpecifierRange(startSpecifier, specifierLen);
+ return false;
+ }
+
// First check if the field width, precision, and conversion specifier
// have matching data arguments.
- if (!HandleAmount(FS.getFieldWidth(),
- diag::warn_printf_asterisk_width_missing_arg,
- diag::warn_printf_asterisk_width_wrong_type,
- startSpecifier, specifierLen)) {
+ if (!HandleAmount(FS.getFieldWidth(), /* field width */ 0,
+ startSpecifier, specifierLen)) {
return false;
}
- if (!HandleAmount(FS.getPrecision(),
- diag::warn_printf_asterisk_precision_missing_arg,
- diag::warn_printf_asterisk_precision_wrong_type,
- startSpecifier, specifierLen)) {
+ if (!HandleAmount(FS.getPrecision(), /* precision */ 1,
+ startSpecifier, specifierLen)) {
return false;
}
Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=97297&r1=97296&r2=97297&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Fri Feb 26 19:41:03 2010
@@ -221,3 +221,16 @@
printf("%S", "hello"); // expected-warning{{but the argument has type 'char *'}}
}
+// Mac OS X supports positional arguments in format strings.
+// This is an IEEE extension (IEEE Std 1003.1).
+// FIXME: This is probably not portable everywhere.
+void test_positional_arguments() {
+ printf("%0$", (int)2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+ printf("%1$d", (int) 2); // no-warning
+ printf("%1$d", (int) 2, 2); // expected-warning{{data argument not used by format string}}
+ printf("%1$d%1$f", (int) 2); // expected-warning{{conversion specifies type 'double' but the argument has type 'int'}}
+ printf("%1$2.2d", (int) 2); // no-warning
+ printf("%2$*1$.2d", (int) 2, (int) 3); // no-warning
+ printf("%2$*8$d", (int) 2, (int) 3); // expected-warning{{specified field width is missing a matching 'int' argument}}
+}
+
More information about the cfe-commits
mailing list