[cfe-commits] r143168 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/Sema/format-strings-no-fixit.c test/Sema/format-strings-scanf.c test/Sema/format-strings.c
Richard Trieu
rtrieu at google.com
Thu Oct 27 17:41:25 PDT 2011
Author: rtrieu
Date: Thu Oct 27 19:41:25 2011
New Revision: 143168
URL: http://llvm.org/viewvc/llvm-project?rev=143168&view=rev
Log:
Fix for PR9751 to change the behavior of -Wformat warnings. If the format
string is part of the function call, then there is no difference. If the
format string is not, the warning will point to the call site and a note
will point to where the format string is.
Fix-it hints for strings are moved to the note if a note is emitted. This will
prevent changes to format strings that may be used in multiple places.
Added:
cfe/trunk/test/Sema/format-strings-no-fixit.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-strings-scanf.c
cfe/trunk/test/Sema/format-strings.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=143168&r1=143167&r2=143168&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 27 19:41:25 2011
@@ -4655,9 +4655,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_static_array_too_small : Warning<
"array argument is too small; contains %0 elements, callee requires at least %1">,
InGroup<DiagGroup<"array-bounds">>;
@@ -4689,7 +4686,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">,
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=143168&r1=143167&r2=143168&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Oct 27 19:41:25 2011
@@ -6185,12 +6185,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,
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=143168&r1=143167&r2=143168&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Oct 27 19:41:25 2011
@@ -1215,7 +1215,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;
@@ -1227,9 +1227,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:
@@ -1277,7 +1279,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
@@ -1317,7 +1319,8 @@
const Expr *Arg = CE->getArg(ArgIndex - 1);
return SemaCheckStringLiteral(Arg, TheCall, HasVAListArg,
- format_idx, firstDataArg, isPrintf);
+ format_idx, firstDataArg, isPrintf,
+ inFunctionCall);
}
}
}
@@ -1336,7 +1339,7 @@
if (StrE) {
CheckFormatString(StrE, E, TheCall, HasVAListArg, format_idx,
- firstDataArg, isPrintf);
+ firstDataArg, isPrintf, inFunctionCall);
return true;
}
@@ -1440,19 +1443,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();
}
@@ -1470,11 +1476,23 @@
void HandleNullChar(const char *nullCharacter);
+ template <typename Range>
+ static void EmitFormatDiagnostic(Sema &S, 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,
@@ -1487,6 +1505,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);
};
}
@@ -1511,32 +1537,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());
}
}
@@ -1553,9 +1583,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());
}
}
}
@@ -1583,13 +1613,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,
@@ -1597,23 +1637,74 @@
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, inFunctionCall, TheCall->getArg(FormatIdx), PDiag,
+ Loc, IsStringLocation, StringRange, FixIt);
+}
+
+/// \brief If the format string is not within the funcion call, emit a note
+/// so that the function call and string are in diagnostic messages.
+///
+/// \param inFunctionCall if true, the format string is within the function
+/// call and only one diagnostic message will be produced. Otherwise, an
+/// extra note will be emitted pointing to location of the format string.
+///
+/// \param ArgumentExpr the expression that is passed as the format string
+/// argument in the function call. Used for getting locations when two
+/// diagnostics are emitted.
+///
+/// \param PDiag the callee should already have provided any strings for the
+/// diagnostic message. This function only adds locations and fixits
+/// to diagnostics.
+///
+/// \param Loc primary location for diagnostic. If two diagnostics are
+/// required, one will be at Loc and a new SourceLocation will be created for
+/// the other one.
+///
+/// \param IsStringLocation if true, Loc points to the format string should be
+/// used for the note. Otherwise, Loc points to the argument list and will
+/// be used with PDiag.
+///
+/// \param StringRange some or all of the string to highlight. This is
+/// templated so it can accept either a CharSourceRange or a SourceRange.
+///
+/// \param Fixit optional fix it hint for the format string.
+template<typename Range>
+void CheckFormatHandler::EmitFormatDiagnostic(Sema &S, bool InFunctionCall,
+ const Expr *ArgumentExpr,
+ PartialDiagnostic PDiag,
+ SourceLocation Loc,
+ bool IsStringLocation,
+ Range StringRange,
+ FixItHint FixIt) {
+ 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 {
@@ -1623,10 +1714,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(
@@ -1676,9 +1768,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;
@@ -1696,12 +1790,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;
@@ -1719,25 +1813,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,
@@ -1747,11 +1835,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(
@@ -1761,12 +1851,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
@@ -1785,10 +1876,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;
}
}
@@ -1864,18 +1953,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;
}
@@ -1916,14 +2009,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()),
@@ -1946,10 +2041,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,
@@ -1966,8 +2062,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(
@@ -2002,10 +2099,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;
}
}
@@ -2016,9 +2111,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));
}
}
@@ -2064,13 +2160,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, inFunctionCall, TheCall->getArg(format_idx),
+ PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(),
+ /*IsStringLocation*/true, OrigFormatExpr->getSourceRange());
return;
}
@@ -2082,15 +2179,18 @@
// CHECK: empty format string?
if (StrLen == 0 && numDataArgs > 0) {
- Diag(FExpr->getLocStart(), diag::warn_empty_format_string)
- << OrigFormatExpr->getSourceRange();
+ CheckFormatHandler::EmitFormatDiagnostic(
+ *this, 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();
@@ -2098,7 +2198,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();
Added: cfe/trunk/test/Sema/format-strings-no-fixit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-no-fixit.c?rev=143168&view=auto
==============================================================================
--- cfe/trunk/test/Sema/format-strings-no-fixit.c (added)
+++ cfe/trunk/test/Sema/format-strings-no-fixit.c Thu Oct 27 19:41:25 2011
@@ -0,0 +1,65 @@
+// 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 function 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);
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=143168&r1=143167&r2=143168&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-scanf.c (original)
+++ cfe/trunk/test/Sema/format-strings-scanf.c Thu Oct 27 19:41:25 2011
@@ -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}}
+}
Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=143168&r1=143167&r2=143168&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Thu Oct 27 19:41:25 2011
@@ -393,3 +393,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}}
+}
More information about the cfe-commits
mailing list