maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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.

llvm-svn: 143168
This commit is contained in:
Richard Trieu 2011-10-28 00:41:25 +00:00
parent 080a499ee0
commit 03cf7b70e0
6 changed files with 380 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@ -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">,

5
clang/include/clang/Sema/Sema.h
View File

@ -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,

335
clang/lib/Sema/SemaChecking.cpp
View File

@ -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();

65
clang/test/Sema/format-strings-no-fixit.c Normal file
View File

@ -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);

12
clang/test/Sema/format-strings-scanf.c
View File

@ -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}}
}

76
clang/test/Sema/format-strings.c
View File

@ -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}}
}