diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 74272ba523f177666f85f254b1a404a67ab77c51..bd8276fc0f926a5a3753070ea240a4f189768615 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4655,9 +4655,6 @@ def warn_format_invalid_positional_specifier : Warning<
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_printf_ignored_flag: Warning<
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">,
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index cba8134fca4d299b2b484b124e7627e91a01a099..f1b0befcbf40500aa0b50fcabe9ee2d861f9e4ea 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6185,12 +6185,13 @@ private:
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,
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 6e81c3c9661d5eff9cacacfecd39c94e8158d28d..715abb9c0a2a5cddc2faeceeaaf75bdbe1bb5a59 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1215,7 +1215,7 @@ bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) {
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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
if (StrE) {
CheckFormatString(StrE, E, TheCall, HasVAListArg, format_idx,
- firstDataArg, isPrintf);
+ firstDataArg, isPrintf, inFunctionCall);
return true;
}
@@ -1440,19 +1443,22 @@ protected:
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 @@ public:
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 @@ protected:
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 @@ SourceLocation CheckFormatHandler::getLocationOfByte(const char *x) {
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 @@ void CheckFormatHandler::DoneProcessing() {
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 @@ CheckFormatHandler::HandleInvalidConversionSpecifier(unsigned argIndex,
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 @@ CheckFormatHandler::CheckNumArgs(
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 @@ public:
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 @@ bool CheckPrintfHandler::HandleAmount(
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 @@ bool CheckPrintfHandler::HandleAmount(
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 @@ void CheckPrintfHandler::HandleInvalidAmount(
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 @@ void CheckPrintfHandler::HandleFlag(const analyze_printf::PrintfSpecifier &FS,
// 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 @@ void CheckPrintfHandler::HandleIgnoredFlag(
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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
// 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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
// 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 @@ public:
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 @@ public:
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 @@ bool CheckScanfHandler::HandleScanfSpecifier(
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 @@ bool CheckScanfHandler::HandleScanfSpecifier(
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 @@ void Sema::CheckFormatString(const StringLiteral *FExpr,
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 @@ void Sema::CheckFormatString(const StringLiteral *FExpr,
// 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 @@ void Sema::CheckFormatString(const StringLiteral *FExpr,
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();
diff --git a/test/Sema/format-strings-no-fixit.c b/test/Sema/format-strings-no-fixit.c
new file mode 100644
index 0000000000000000000000000000000000000000..701e945f69021a00d51907f44a3f4da4e5edf79b
--- /dev/null
+++ b/test/Sema/format-strings-no-fixit.c
@@ -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);
diff --git a/test/Sema/format-strings-scanf.c b/test/Sema/format-strings-scanf.c
index 42b6c03ffbff69d6863d0df998d01d8df9f97e93..ee559daef6151fb36628403939c5097c987f2752 100644
--- a/test/Sema/format-strings-scanf.c
+++ b/test/Sema/format-strings-scanf.c
@@ -32,3 +32,15 @@ void bad_length_modifiers(char *s, void *p, wchar_t *ws, long double *ld) {
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}}
+}
diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index 4f2ad99a5727c4083e50727471c984d74e817989..c4bbc836d458b8a73e4df7e0ee0895afc372f3d9 100644
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -393,3 +393,79 @@ void test_suppress_invalid_specifier() {
#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}}
+}