On Thu, Oct 13, 2011 at 3:25 PM, Richard Trieu <[email protected]> wrote:
> On Thu, Oct 13, 2011 at 2:53 PM, Eli Friedman <[email protected]>wrote:
>
>> On Thu, Oct 13, 2011 at 2:38 PM, Richard Trieu <[email protected]> wrote:
>> > http://llvm.org/bugs/show_bug.cgi?id=9751
>> > -Wformat warnings will point to the format string, but no message at the
>> > call site. This patch will move the warning to always point at the call
>> > site. If the format string is part of the function call, one warning
>> will
>> > show. If the format string is defined elsewhere, a warning at the call
>> site
>> > plus a note where the format string is defined.
>> > Also, if a note is present, the fix-it would be moved to the note so
>> they
>> > won't be applied with -fixit. If the format string was used in multiple
>> > places, fixits may cause problems in other places.
>> > printf("%d %d", 1);
>> > warning: more '%' conversions than data arguments [-Wformat]
>> > printf("%d %d", 1);
>> > ~^
>> > const char kFormat[] = "%d %d";
>> > printf(kFormat, 1);
>> > test2.c:8:8: warning: more '%' conversions than data arguments
>> [-Wformat]
>> > printf(kFormat, 1);
>> > ^~~~~~~
>> > test2.c:7:29: note: format string is defined here
>> > const char kFormat[] = "%d %d";
>> > ~^
>> > Patch attached an available at http://codereview.appspot.com/5277043/
>>
>> +namespace {
>> + class StringLiteralFinder
>> + : public ConstStmtVisitor<StringLiteralFinder> {
>> + Sema& S;
>> + const StringLiteral *StringExpr;
>> + bool StringFound;
>> + public:
>> [...]
>>
>> Why is StringLiteralFinder necessary? The caller should know how it
>> found the string literal...
>>
>> That information isn't currently preserved through the diagnostics. The
> StringLiteralFinder is necessary to determine this after the fact. I can go
> look again and see if I can propagate that bit of info through the code.
>
>
Turns out, it was easier to to pass around a bool with this info than
I originally thought. New patch attached.
Index: test/Sema/format-strings-scanf.c
===================================================================
--- test/Sema/format-strings-scanf.c (revision 141726)
+++ test/Sema/format-strings-scanf.c (working copy)
@@ -32,3 +32,15 @@
scanf("%ls", ws); // no-warning
scanf("%#.2Lf", ld); // expected-warning{{invalid conversion specifier '#'}}
}
+
+// Test that the scanf call site is where the warning is attached. If the
+// format string is somewhere else, point to it in a note.
+void pr9751() {
+ int *i;
+ const char kFormat1[] = "%00d"; // expected-note{{format string is defined here}}}
+ scanf(kFormat1, i); // expected-warning{{zero field width in scanf format string is unused}}
+ scanf("%00d", i); // expected-warning{{zero field width in scanf format string is unused}}
+ const char kFormat2[] = "%["; // expected-note{{format string is defined here}}}
+ scanf(kFormat2, &i); // expected-warning{{no closing ']' for '%[' in scanf format string}}
+ scanf("%[", &i); // expected-warning{{no closing ']' for '%[' in scanf format string}}
+}
Index: test/Sema/format-strings.c
===================================================================
--- test/Sema/format-strings.c (revision 141726)
+++ test/Sema/format-strings.c (working copy)
@@ -387,3 +387,79 @@
#pragma clang diagnostic pop
}
+// Make sure warnings are on for next test.
+#pragma GCC diagnostic warning "-Wformat"
+#pragma GCC diagnostic warning "-Wformat-security"
+
+// Test that the printf call site is where the warning is attached. If the
+// format string is somewhere else, point to it in a note.
+void pr9751() {
+ const char kFormat1[] = "%d %d \n"; // expected-note{{format string is defined here}}}
+ printf(kFormat1, 0); // expected-warning{{more '%' conversions than data arguments}}
+ printf("%d %s\n", 0); // expected-warning{{more '%' conversions than data arguments}}
+
+ const char kFormat2[] = "%18$s\n"; // expected-note{{format string is defined here}}
+ printf(kFormat2, 1, "foo"); // expected-warning{{data argument position '18' exceeds the number of data arguments (2)}}
+ 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}}
+
+ const char kFormat4[] = "%y"; // expected-note{{format string is defined here}}
+ printf(kFormat4, 5); // expected-warning{{invalid conversion specifier 'y'}}
+ printf("%y", 5); // expected-warning{{invalid conversion specifier 'y'}}
+
+ const char kFormat5[] = "%."; // expected-note{{format string is defined here}}
+ printf(kFormat5, 5); // expected-warning{{incomplete format specifier}}
+ printf("%.", 5); // expected-warning{{incomplete format specifier}}
+
+ const char kFormat6[] = "%s"; // expected-note{{format string is defined here}}
+ printf(kFormat6, 5); // expected-warning{{conversion specifies type 'char *' but the argument has type 'int'}}
+ printf("%s", 5); // expected-warning{{conversion specifies type 'char *' but the argument has type 'int'}}
+
+ const char kFormat7[] = "%0$"; // expected-note{{format string is defined here}}
+ printf(kFormat7, 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+ printf("%0$", 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+
+ const char kFormat8[] = "%1$d %d"; // expected-note{{format string is defined here}}
+ printf(kFormat8, 4, 4); // expected-warning{{cannot mix positional and non-positional arguments in format string}}
+ printf("%1$d %d", 4, 4); // expected-warning{{cannot mix positional and non-positional arguments in format string}}
+
+ const char kFormat9[] = ""; // expected-note{{format string is defined here}}
+ printf(kFormat9, 4, 4); // expected-warning{{format string is empty}}
+ printf("", 4, 4); // expected-warning{{format string is empty}}
+
+ const char kFormat10[] = "\0%d"; // expected-note{{format string is defined here}}
+ printf(kFormat10, 4); // expected-warning{{format string contains '\0' within the string body}}
+ printf("\0%d", 4); // expected-warning{{format string contains '\0' within the string body}}
+
+ const char kFormat11[] = "%*d"; // expected-note{{format string is defined here}}
+ printf(kFormat11); // expected-warning{{'*' specified field width is missing a matching 'int' argument}}
+ printf("%*d"); // expected-warning{{'*' specified field width is missing a matching 'int' argument}}
+
+ const char kFormat12[] = "%*d"; // expected-note{{format string is defined here}}
+ printf(kFormat12, 4.4); // expected-warning{{field width should have type 'int', but argument has type 'double'}}
+ printf("%*d", 4.4); // expected-warning{{field width should have type 'int', but argument has type 'double'}}
+
+ const char kFormat13[] = "%.3p"; // expected-note{{format string is defined here}}
+ void *p;
+ printf(kFormat13, p); // expected-warning{{precision used with 'p' conversion specifier, resulting in undefined behavior}}
+ printf("%.3p", p); // expected-warning{{precision used with 'p' conversion specifier, resulting in undefined behavior}}
+
+ const char kFormat14[] = "%0s"; // expected-note{{format string is defined here}}
+ printf(kFormat14, "a"); // expected-warning{{flag '0' results in undefined behavior with 's' conversion specifier}}
+ printf("%0s", "a"); // expected-warning{{flag '0' results in undefined behavior with 's' conversion specifier}}
+
+ const char kFormat15[] = "%hhs"; // expected-note{{format string is defined here}}
+ printf(kFormat15, "a"); // expected-warning{{length modifier 'hh' results in undefined behavior or no effect with 's' conversion specifier}}
+ printf("%hhs", "a"); // expected-warning{{length modifier 'hh' results in undefined behavior or no effect with 's' conversion specifier}}
+
+ const char kFormat16[] = "%-0d"; // expected-note{{format string is defined here}}
+ printf(kFormat16, 5); // expected-warning{{flag '0' is ignored when flag '-' is present}}
+ printf("%-0d", 5); // expected-warning{{flag '0' is ignored when flag '-' is present}}
+
+ // Make sure that the "format string is defined here" note is not emitted
+ // when the original string is within the argument expression.
+ printf(1 ? "yes %d" : "no %d"); // expected-warning 2{{more '%' conversions than data arguments}}
+}
Index: test/Sema/format-strings-no-fixit.c
===================================================================
--- test/Sema/format-strings-no-fixit.c (revision 0)
+++ test/Sema/format-strings-no-fixit.c (revision 0)
@@ -0,0 +1,66 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -fixit %t
+// RUN: %clang_cc1 -E -o - %t | FileCheck %s
+
+/* This is a test of the various code modification hints that are
+ provided as part of warning or extension diagnostics. Only
+ warnings for format strings within the fucntion call will be
+ fixed by -fixit. Other format strings will be left alone. */
+
+int printf(char const *, ...);
+int scanf(char const *, ...);
+
+void pr9751() {
+ const char kFormat1[] = "%s";
+ printf(kFormat1, 5);
+ printf("%s", 5);
+
+ const char kFormat2[] = "%.3p";
+ void *p;
+ printf(kFormat2, p);
+ printf("%.3p", p);
+
+ const char kFormat3[] = "%0s";
+ printf(kFormat3, "a");
+ printf("%0s", "a");
+
+ const char kFormat4[] = "%hhs";
+ printf(kFormat4, "a");
+ printf("%hhs", "a");
+
+ const char kFormat5[] = "%-0d";
+ printf(kFormat5, 5);
+ printf("%-0d", 5);
+
+ const char kFormat6[] = "%00d";
+ int *i;
+ scanf(kFormat6, i);
+ scanf("%00d", i);
+}
+
+// CHECK: const char kFormat1[] = "%s";
+// CHECK: printf(kFormat1, 5);
+// CHECK: printf("%d", 5);
+
+// CHECK: const char kFormat2[] = "%.3p";
+// CHECK: void *p;
+// CHECK: printf(kFormat2, p);
+// CHECK: printf("%p", p);
+
+// CHECK: const char kFormat3[] = "%0s";
+// CHECK: printf(kFormat3, "a");
+// CHECK: printf("%s", "a");
+
+// CHECK: const char kFormat4[] = "%hhs";
+// CHECK: printf(kFormat4, "a");
+// CHECK: printf("%s", "a");
+
+// CHECK: const char kFormat5[] = "%-0d";
+// CHECK: printf(kFormat5, 5);
+// CHECK: printf("%-d", 5);
+
+// CHECK: const char kFormat6[] = "%00d";
+// CHECK: int *i;
+// CHECK: scanf(kFormat6, i);
+// CHECK: scanf("%d", i);
+
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 141726)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -4490,9 +4490,6 @@
def warn_format_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>;
def warn_empty_format_string : Warning<
"format string is empty">, InGroup<FormatZeroLength>;
def warn_format_string_is_wide_literal : Warning<
@@ -4519,7 +4516,12 @@
def warn_scanf_scanlist_incomplete : Warning<
"no closing ']' for '%%[' in scanf format string">,
InGroup<Format>;
-
+def note_format_string_defined : Note<"format string is defined here">;
+
+def warn_null_arg : Warning<
+ "null passed to a callee which requires a non-null argument">,
+ InGroup<NonNull>;
+
// CHECK: returning address/reference of stack memory
def warn_ret_stack_addr : Warning<
"address of stack memory associated with local variable %0 returned">,
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 141726)
+++ include/clang/Sema/Sema.h (working copy)
@@ -6084,12 +6084,13 @@
bool SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
bool HasVAListArg, unsigned format_idx,
- unsigned firstDataArg, bool isPrintf);
+ unsigned firstDataArg, bool isPrintf,
+ bool inFunctionCall = true);
void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,
const CallExpr *TheCall, bool HasVAListArg,
unsigned format_idx, unsigned firstDataArg,
- bool isPrintf);
+ bool isPrintf, bool inFunctionCall);
void CheckNonNullArguments(const NonNullAttr *NonNull,
const Expr * const *ExprArgs,
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp (revision 141726)
+++ lib/Sema/SemaChecking.cpp (working copy)
@@ -1188,7 +1188,7 @@
bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
bool HasVAListArg,
unsigned format_idx, unsigned firstDataArg,
- bool isPrintf) {
+ bool isPrintf, bool inFunctionCall) {
tryAgain:
if (E->isTypeDependent() || E->isValueDependent())
return false;
@@ -1200,9 +1200,11 @@
case Stmt::ConditionalOperatorClass: {
const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
return SemaCheckStringLiteral(C->getTrueExpr(), TheCall, HasVAListArg,
- format_idx, firstDataArg, isPrintf)
+ format_idx, firstDataArg, isPrintf,
+ inFunctionCall)
&& SemaCheckStringLiteral(C->getFalseExpr(), TheCall, HasVAListArg,
- format_idx, firstDataArg, isPrintf);
+ format_idx, firstDataArg, isPrintf,
+ inFunctionCall);
}
case Stmt::IntegerLiteralClass:
@@ -1250,7 +1252,7 @@
if (const Expr *Init = VD->getAnyInitializer())
return SemaCheckStringLiteral(Init, TheCall,
HasVAListArg, format_idx, firstDataArg,
- isPrintf);
+ isPrintf, /*inFunctionCall*/false);
}
// For vprintf* functions (i.e., HasVAListArg==true), we add a
@@ -1290,7 +1292,8 @@
const Expr *Arg = CE->getArg(ArgIndex - 1);
return SemaCheckStringLiteral(Arg, TheCall, HasVAListArg,
- format_idx, firstDataArg, isPrintf);
+ format_idx, firstDataArg, isPrintf,
+ inFunctionCall);
}
}
}
@@ -1309,7 +1312,7 @@
if (StrE) {
CheckFormatString(StrE, E, TheCall, HasVAListArg, format_idx,
- firstDataArg, isPrintf);
+ firstDataArg, isPrintf, inFunctionCall);
return true;
}
@@ -1413,19 +1416,22 @@
llvm::BitVector CoveredArgs;
bool usesPositionalArgs;
bool atFirstArg;
+ bool inFunctionCall;
public:
CheckFormatHandler(Sema &s, const StringLiteral *fexpr,
const Expr *origFormatExpr, unsigned firstDataArg,
unsigned numDataArgs, bool isObjCLiteral,
const char *beg, bool hasVAListArg,
- const CallExpr *theCall, unsigned formatIdx)
+ const CallExpr *theCall, unsigned formatIdx,
+ bool inFunctionCall)
: S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr),
FirstDataArg(firstDataArg),
NumDataArgs(numDataArgs),
IsObjCLiteral(isObjCLiteral), Beg(beg),
HasVAListArg(hasVAListArg),
TheCall(theCall), FormatIdx(formatIdx),
- usesPositionalArgs(false), atFirstArg(true) {
+ usesPositionalArgs(false), atFirstArg(true),
+ inFunctionCall(inFunctionCall) {
CoveredArgs.resize(numDataArgs);
CoveredArgs.reset();
}
@@ -1443,11 +1449,24 @@
void HandleNullChar(const char *nullCharacter);
+ template <typename Range>
+ static void EmitFormatDiagnostic(Sema &S, const Expr *StringExpr,
+ bool inFunctionCall,
+ const Expr *ArgumentExpr,
+ PartialDiagnostic PDiag,
+ SourceLocation StringLoc,
+ bool IsStringLocation, Range StringRange,
+ FixItHint Fixit = FixItHint());
+
protected:
bool HandleInvalidConversionSpecifier(unsigned argIndex, SourceLocation Loc,
const char *startSpec,
unsigned specifierLen,
const char *csStart, unsigned csLen);
+
+ void HandlePositionalNonpositionalArgs(SourceLocation Loc,
+ const char *startSpec,
+ unsigned specifierLen);
SourceRange getFormatStringRange();
CharSourceRange getSpecifierRange(const char *startSpecifier,
@@ -1460,6 +1479,14 @@
const analyze_format_string::ConversionSpecifier &CS,
const char *startSpecifier, unsigned specifierLen,
unsigned argIndex);
+
+ template <typename Range>
+ void EmitFormatDiagnostic(PartialDiagnostic PDiag, SourceLocation StringLoc,
+ bool IsStringLocation, Range StringRange,
+ FixItHint Fixit = FixItHint());
+
+ void CheckPositionalAndNonpositionalArgs(
+ const analyze_format_string::FormatSpecifier *FS);
};
}
@@ -1484,32 +1511,36 @@
void CheckFormatHandler::HandleIncompleteSpecifier(const char *startSpecifier,
unsigned specifierLen){
- SourceLocation Loc = getLocationOfByte(startSpecifier);
- S.Diag(Loc, diag::warn_printf_incomplete_specifier)
- << getSpecifierRange(startSpecifier, specifierLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_incomplete_specifier),
+ getLocationOfByte(startSpecifier),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen));
}
void
CheckFormatHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
analyze_format_string::PositionContext p) {
- SourceLocation Loc = getLocationOfByte(startPos);
- S.Diag(Loc, diag::warn_format_invalid_positional_specifier)
- << (unsigned) p << getSpecifierRange(startPos, posLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_format_invalid_positional_specifier)
+ << (unsigned) p,
+ getLocationOfByte(startPos), /*IsStringLocation*/true,
+ getSpecifierRange(startPos, posLen));
}
void CheckFormatHandler::HandleZeroPosition(const char *startPos,
unsigned posLen) {
- SourceLocation Loc = getLocationOfByte(startPos);
- S.Diag(Loc, diag::warn_format_zero_positional_specifier)
- << getSpecifierRange(startPos, posLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_format_zero_positional_specifier),
+ getLocationOfByte(startPos),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startPos, posLen));
}
void CheckFormatHandler::HandleNullChar(const char *nullCharacter) {
if (!IsObjCLiteral) {
// The presence of a null character is likely an error.
- S.Diag(getLocationOfByte(nullCharacter),
- diag::warn_printf_format_string_contains_null_char)
- << getFormatStringRange();
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_printf_format_string_contains_null_char),
+ getLocationOfByte(nullCharacter), /*IsStringLocation*/true,
+ getFormatStringRange());
}
}
@@ -1526,9 +1557,9 @@
signed notCoveredArg = CoveredArgs.find_first();
if (notCoveredArg >= 0) {
assert((unsigned)notCoveredArg < NumDataArgs);
- S.Diag(getDataArg((unsigned) notCoveredArg)->getLocStart(),
- diag::warn_printf_data_arg_not_used)
- << getFormatStringRange();
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_data_arg_not_used),
+ getDataArg((unsigned) notCoveredArg)->getLocStart(),
+ /*IsStringLocation*/false, getFormatStringRange());
}
}
}
@@ -1556,13 +1587,23 @@
keepGoing = false;
}
- S.Diag(Loc, diag::warn_format_invalid_conversion)
- << StringRef(csStart, csLen)
- << getSpecifierRange(startSpec, specifierLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_format_invalid_conversion)
+ << StringRef(csStart, csLen),
+ Loc, /*IsStringLocation*/true,
+ getSpecifierRange(startSpec, specifierLen));
return keepGoing;
}
+void
+CheckFormatHandler::HandlePositionalNonpositionalArgs(SourceLocation Loc,
+ const char *startSpec,
+ unsigned specifierLen) {
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_format_mix_positional_nonpositional_args),
+ Loc, /*isStringLoc*/true, getSpecifierRange(startSpec, specifierLen));
+}
+
bool
CheckFormatHandler::CheckNumArgs(
const analyze_format_string::FormatSpecifier &FS,
@@ -1570,23 +1611,49 @@
const char *startSpecifier, unsigned specifierLen, unsigned argIndex) {
if (argIndex >= NumDataArgs) {
- if (FS.usesPositionalArg()) {
- S.Diag(getLocationOfByte(CS.getStart()),
- diag::warn_printf_positional_arg_exceeds_data_args)
- << (argIndex+1) << NumDataArgs
- << getSpecifierRange(startSpecifier, specifierLen);
- }
- else {
- S.Diag(getLocationOfByte(CS.getStart()),
- diag::warn_printf_insufficient_data_args)
- << getSpecifierRange(startSpecifier, specifierLen);
- }
-
+ PartialDiagnostic PDiag = FS.usesPositionalArg()
+ ? (S.PDiag(diag::warn_printf_positional_arg_exceeds_data_args)
+ << (argIndex+1) << NumDataArgs)
+ : S.PDiag(diag::warn_printf_insufficient_data_args);
+ EmitFormatDiagnostic(
+ PDiag, getLocationOfByte(CS.getStart()), /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen));
return false;
}
return true;
}
+template<typename Range>
+void CheckFormatHandler::EmitFormatDiagnostic(PartialDiagnostic PDiag,
+ SourceLocation Loc,
+ bool IsStringLocation,
+ Range StringRange,
+ FixItHint FixIt) {
+ EmitFormatDiagnostic(S, FExpr, inFunctionCall, TheCall->getArg(FormatIdx),
+ PDiag, Loc, IsStringLocation, StringRange, FixIt);
+}
+
+template<typename Range>
+void CheckFormatHandler::EmitFormatDiagnostic(Sema &S, const Expr *StringExpr,
+ bool inFunctionCall,
+ const Expr *ArgumentExpr,
+ PartialDiagnostic PDiag,
+ SourceLocation Loc,
+ bool IsStringLocation,
+ Range StringRange,
+ FixItHint FixIt) {
+ //if (StringLiteralFinder(S, StringExpr, ArgumentExpr).HasString())
+ if (inFunctionCall)
+ S.Diag(Loc, PDiag) << StringRange << FixIt;
+ else {
+ S.Diag(IsStringLocation ? ArgumentExpr->getExprLoc() : Loc, PDiag)
+ << ArgumentExpr->getSourceRange();
+ S.Diag(IsStringLocation ? Loc : StringRange.getBegin(),
+ diag::note_format_string_defined)
+ << StringRange << FixIt;
+ }
+}
+
//===--- CHECK: Printf format string checking ------------------------------===//
namespace {
@@ -1596,10 +1663,11 @@
const Expr *origFormatExpr, unsigned firstDataArg,
unsigned numDataArgs, bool isObjCLiteral,
const char *beg, bool hasVAListArg,
- const CallExpr *theCall, unsigned formatIdx)
+ const CallExpr *theCall, unsigned formatIdx,
+ bool inFunctionCall)
: CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
numDataArgs, isObjCLiteral, beg, hasVAListArg,
- theCall, formatIdx) {}
+ theCall, formatIdx, inFunctionCall) {}
bool HandleInvalidPrintfConversionSpecifier(
@@ -1649,9 +1717,11 @@
if (!HasVAListArg) {
unsigned argIndex = Amt.getArgIndex();
if (argIndex >= NumDataArgs) {
- S.Diag(getLocationOfByte(Amt.getStart()),
- diag::warn_printf_asterisk_missing_arg)
- << k << getSpecifierRange(startSpecifier, specifierLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_missing_arg)
+ << k,
+ getLocationOfByte(Amt.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen));
// Don't do any more checking. We will just emit
// spurious errors.
return false;
@@ -1669,12 +1739,12 @@
assert(ATR.isValid());
if (!ATR.matchesType(S.Context, T)) {
- S.Diag(getLocationOfByte(Amt.getStart()),
- diag::warn_printf_asterisk_wrong_type)
- << k
- << ATR.getRepresentativeType(S.Context) << T
- << getSpecifierRange(startSpecifier, specifierLen)
- << Arg->getSourceRange();
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_wrong_type)
+ << k << ATR.getRepresentativeType(S.Context)
+ << T << Arg->getSourceRange(),
+ getLocationOfByte(Amt.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen));
// Don't do any more checking. We will just emit
// spurious errors.
return false;
@@ -1692,25 +1762,19 @@
unsigned specifierLen) {
const analyze_printf::PrintfConversionSpecifier &CS =
FS.getConversionSpecifier();
- switch (Amt.getHowSpecified()) {
- case analyze_printf::OptionalAmount::Constant:
- S.Diag(getLocationOfByte(Amt.getStart()),
- diag::warn_printf_nonsensical_optional_amount)
- << type
- << CS.toString()
- << getSpecifierRange(startSpecifier, specifierLen)
- << FixItHint::CreateRemoval(getSpecifierRange(Amt.getStart(),
- Amt.getConstantLength()));
- break;
- default:
- S.Diag(getLocationOfByte(Amt.getStart()),
- diag::warn_printf_nonsensical_optional_amount)
- << type
- << CS.toString()
- << getSpecifierRange(startSpecifier, specifierLen);
- break;
- }
+ FixItHint fixit =
+ Amt.getHowSpecified() == analyze_printf::OptionalAmount::Constant
+ ? FixItHint::CreateRemoval(getSpecifierRange(Amt.getStart(),
+ Amt.getConstantLength()))
+ : FixItHint();
+
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_nonsensical_optional_amount)
+ << type << CS.toString(),
+ getLocationOfByte(Amt.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen),
+ fixit);
}
void CheckPrintfHandler::HandleFlag(const analyze_printf::PrintfSpecifier &FS,
@@ -1720,11 +1784,13 @@
// Warn about pointless flag with a fixit removal.
const analyze_printf::PrintfConversionSpecifier &CS =
FS.getConversionSpecifier();
- S.Diag(getLocationOfByte(flag.getPosition()),
- diag::warn_printf_nonsensical_flag)
- << flag.toString() << CS.toString()
- << getSpecifierRange(startSpecifier, specifierLen)
- << FixItHint::CreateRemoval(getSpecifierRange(flag.getPosition(), 1));
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_nonsensical_flag)
+ << flag.toString() << CS.toString(),
+ getLocationOfByte(flag.getPosition()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen),
+ FixItHint::CreateRemoval(
+ getSpecifierRange(flag.getPosition(), 1)));
}
void CheckPrintfHandler::HandleIgnoredFlag(
@@ -1734,12 +1800,13 @@
const char *startSpecifier,
unsigned specifierLen) {
// Warn about ignored flag with a fixit removal.
- S.Diag(getLocationOfByte(ignoredFlag.getPosition()),
- diag::warn_printf_ignored_flag)
- << ignoredFlag.toString() << flag.toString()
- << getSpecifierRange(startSpecifier, specifierLen)
- << FixItHint::CreateRemoval(getSpecifierRange(
- ignoredFlag.getPosition(), 1));
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_ignored_flag)
+ << ignoredFlag.toString() << flag.toString(),
+ getLocationOfByte(ignoredFlag.getPosition()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen),
+ FixItHint::CreateRemoval(
+ getSpecifierRange(ignoredFlag.getPosition(), 1)));
}
bool
@@ -1758,10 +1825,8 @@
usesPositionalArgs = FS.usesPositionalArg();
}
else if (usesPositionalArgs != FS.usesPositionalArg()) {
- // Cannot mix-and-match positional and non-positional arguments.
- S.Diag(getLocationOfByte(CS.getStart()),
- diag::warn_format_mix_positional_nonpositional_args)
- << getSpecifierRange(startSpecifier, specifierLen);
+ HandlePositionalNonpositionalArgs(getLocationOfByte(CS.getStart()),
+ startSpecifier, specifierLen);
return false;
}
}
@@ -1837,18 +1902,22 @@
// Check the length modifier is valid with the given conversion specifier.
const LengthModifier &LM = FS.getLengthModifier();
if (!FS.hasValidLengthModifier())
- S.Diag(getLocationOfByte(LM.getStart()),
- diag::warn_format_nonsensical_length)
- << LM.toString() << CS.toString()
- << getSpecifierRange(startSpecifier, specifierLen)
- << FixItHint::CreateRemoval(getSpecifierRange(LM.getStart(),
- LM.getLength()));
+ EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length)
+ << LM.toString() << CS.toString(),
+ getLocationOfByte(LM.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen),
+ FixItHint::CreateRemoval(
+ getSpecifierRange(LM.getStart(),
+ LM.getLength())));
// Are we using '%n'?
if (CS.getKind() == ConversionSpecifier::nArg) {
// Issue a warning about this being a possible security issue.
- S.Diag(getLocationOfByte(CS.getStart()), diag::warn_printf_write_back)
- << getSpecifierRange(startSpecifier, specifierLen);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_printf_write_back),
+ getLocationOfByte(CS.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen));
// Continue checking the other format specifiers.
return true;
}
@@ -1889,14 +1958,16 @@
// FIXME: getRepresentativeType() perhaps should return a string
// instead of a QualType to better handle when the representative
// type is 'wint_t' (which is defined in the system headers).
- S.Diag(getLocationOfByte(CS.getStart()),
- diag::warn_printf_conversion_argument_type_mismatch)
- << ATR.getRepresentativeType(S.Context) << Ex->getType()
- << getSpecifierRange(startSpecifier, specifierLen)
- << Ex->getSourceRange()
- << FixItHint::CreateReplacement(
- getSpecifierRange(startSpecifier, specifierLen),
- os.str());
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+ << ATR.getRepresentativeType(S.Context) << Ex->getType()
+ << Ex->getSourceRange(),
+ getLocationOfByte(CS.getStart()),
+ /*IsStringLocation*/true,
+ getSpecifierRange(startSpecifier, specifierLen),
+ FixItHint::CreateReplacement(
+ getSpecifierRange(startSpecifier, specifierLen),
+ os.str()));
}
else {
S.Diag(getLocationOfByte(CS.getStart()),
@@ -1919,10 +1990,11 @@
const Expr *origFormatExpr, unsigned firstDataArg,
unsigned numDataArgs, bool isObjCLiteral,
const char *beg, bool hasVAListArg,
- const CallExpr *theCall, unsigned formatIdx)
+ const CallExpr *theCall, unsigned formatIdx,
+ bool inFunctionCall)
: CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
numDataArgs, isObjCLiteral, beg, hasVAListArg,
- theCall, formatIdx) {}
+ theCall, formatIdx, inFunctionCall) {}
bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
const char *startSpecifier,
@@ -1939,8 +2011,9 @@
void CheckScanfHandler::HandleIncompleteScanList(const char *start,
const char *end) {
- S.Diag(getLocationOfByte(end), diag::warn_scanf_scanlist_incomplete)
- << getSpecifierRange(start, end - start);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_scanf_scanlist_incomplete),
+ getLocationOfByte(end), /*IsStringLocation*/true,
+ getSpecifierRange(start, end - start));
}
bool CheckScanfHandler::HandleInvalidScanfConversionSpecifier(
@@ -1975,10 +2048,8 @@
usesPositionalArgs = FS.usesPositionalArg();
}
else if (usesPositionalArgs != FS.usesPositionalArg()) {
- // Cannot mix-and-match positional and non-positional arguments.
- S.Diag(getLocationOfByte(CS.getStart()),
- diag::warn_format_mix_positional_nonpositional_args)
- << getSpecifierRange(startSpecifier, specifierLen);
+ HandlePositionalNonpositionalArgs(getLocationOfByte(CS.getStart()),
+ startSpecifier, specifierLen);
return false;
}
}
@@ -1989,9 +2060,10 @@
if (Amt.getConstantAmount() == 0) {
const CharSourceRange &R = getSpecifierRange(Amt.getStart(),
Amt.getConstantLength());
- S.Diag(getLocationOfByte(Amt.getStart()),
- diag::warn_scanf_nonzero_width)
- << R << FixItHint::CreateRemoval(R);
+ EmitFormatDiagnostic(S.PDiag(diag::warn_scanf_nonzero_width),
+ getLocationOfByte(Amt.getStart()),
+ /*IsStringLocation*/true, R,
+ FixItHint::CreateRemoval(R));
}
}
@@ -2037,13 +2109,14 @@
const Expr *OrigFormatExpr,
const CallExpr *TheCall, bool HasVAListArg,
unsigned format_idx, unsigned firstDataArg,
- bool isPrintf) {
+ bool isPrintf, bool inFunctionCall) {
// CHECK: is the format string a wide literal?
if (!FExpr->isAscii()) {
- Diag(FExpr->getLocStart(),
- diag::warn_format_string_is_wide_literal)
- << OrigFormatExpr->getSourceRange();
+ CheckFormatHandler::EmitFormatDiagnostic(
+ *this, FExpr, inFunctionCall, TheCall->getArg(format_idx),
+ PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(),
+ /*IsStringLocation*/true, OrigFormatExpr->getSourceRange());
return;
}
@@ -2055,15 +2128,18 @@
// CHECK: empty format string?
if (StrLen == 0 && numDataArgs > 0) {
- Diag(FExpr->getLocStart(), diag::warn_empty_format_string)
- << OrigFormatExpr->getSourceRange();
+ CheckFormatHandler::EmitFormatDiagnostic(
+ *this, FExpr, inFunctionCall, TheCall->getArg(format_idx),
+ PDiag(diag::warn_empty_format_string), FExpr->getLocStart(),
+ /*IsStringLocation*/true, OrigFormatExpr->getSourceRange());
return;
}
if (isPrintf) {
CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
- Str, HasVAListArg, TheCall, format_idx);
+ Str, HasVAListArg, TheCall, format_idx,
+ inFunctionCall);
if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen))
H.DoneProcessing();
@@ -2071,7 +2147,8 @@
else {
CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
- Str, HasVAListArg, TheCall, format_idx);
+ Str, HasVAListArg, TheCall, format_idx,
+ inFunctionCall);
if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen))
H.DoneProcessing();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
