Fixes PR9751. Any other expression kinds we should look through in
NoteCallLocation? I'm a little unhappy with the pile of dyn_cast<>s
stolen from Sema::SemaCheckStringLiteral for handling format_arg
functions, but I don't think it's worth pulling them out into a
helper.
-Matt
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 0b3f0c6..680783b 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4521,7 +4521,9 @@ def warn_printf_ignored_flag: Warning<
def warn_scanf_scanlist_incomplete : Warning<
"no closing ']' for '%%[' in scanf format string">,
InGroup<Format>;
-
+def note_format_call_location : Note<
+ "format string used here">;
+
// 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/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 74c69a3..dc53508 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1494,6 +1494,8 @@ protected:
const analyze_format_string::ConversionSpecifier &CS,
const char *startSpecifier, unsigned specifierLen,
unsigned argIndex);
+
+ void NoteCallLocation();
};
}
@@ -1521,6 +1523,7 @@ void CheckFormatHandler::HandleIncompleteSpecifier(const char *startSpecifier,
SourceLocation Loc = getLocationOfByte(startSpecifier);
S.Diag(Loc, diag::warn_printf_incomplete_specifier)
<< getSpecifierRange(startSpecifier, specifierLen);
+ NoteCallLocation();
}
void
@@ -1529,6 +1532,7 @@ CheckFormatHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
SourceLocation Loc = getLocationOfByte(startPos);
S.Diag(Loc, diag::warn_format_invalid_positional_specifier)
<< (unsigned) p << getSpecifierRange(startPos, posLen);
+ NoteCallLocation();
}
void CheckFormatHandler::HandleZeroPosition(const char *startPos,
@@ -1536,6 +1540,7 @@ void CheckFormatHandler::HandleZeroPosition(const char *startPos,
SourceLocation Loc = getLocationOfByte(startPos);
S.Diag(Loc, diag::warn_format_zero_positional_specifier)
<< getSpecifierRange(startPos, posLen);
+ NoteCallLocation();
}
void CheckFormatHandler::HandleNullChar(const char *nullCharacter) {
@@ -1544,6 +1549,7 @@ void CheckFormatHandler::HandleNullChar(const char *nullCharacter) {
S.Diag(getLocationOfByte(nullCharacter),
diag::warn_printf_format_string_contains_null_char)
<< getFormatStringRange();
+ NoteCallLocation();
}
}
@@ -1593,6 +1599,7 @@ CheckFormatHandler::HandleInvalidConversionSpecifier(unsigned argIndex,
S.Diag(Loc, diag::warn_format_invalid_conversion)
<< StringRef(csStart, csLen)
<< getSpecifierRange(startSpec, specifierLen);
+ NoteCallLocation();
return keepGoing;
}
@@ -1615,12 +1622,40 @@ CheckFormatHandler::CheckNumArgs(
diag::warn_printf_insufficient_data_args)
<< getSpecifierRange(startSpecifier, specifierLen);
}
-
+ NoteCallLocation();
+
return false;
}
return true;
}
+/// If the format string is not a literal within the call (i.e., is a literal
+/// defined elsewhere), emit a note pointing at the callsite.
+void
+CheckFormatHandler::NoteCallLocation() {
+ const Expr* FormatArg = TheCall->getArg(FormatIdx)->IgnoreParenImpCasts();
+ // Look through format_arg functions, e.g. gettext().
+ if (const CallExpr* CE = dyn_cast<CallExpr>(FormatArg)) {
+ if (const ImplicitCastExpr *ICE
+ = dyn_cast<ImplicitCastExpr>(CE->getCallee())) {
+ if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(ICE->getSubExpr())) {
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+ if (const FormatArgAttr *FA = FD->getAttr<FormatArgAttr>()) {
+ FormatArg =
+ CE->getArg(FA->getFormatIdx() - 1)->IgnoreParenImpCasts();
+ }
+ }
+ }
+ }
+ }
+ if (FormatArg == OrigFormatExpr)
+ return;
+ SourceRange FormatArgRange = FormatArg->getSourceRange();
+ S.Diag(FormatArgRange.getBegin(), diag::note_format_call_location)
+ << FormatArgRange;
+}
+
+
//===--- CHECK: Printf format string checking ------------------------------===//
namespace {
@@ -1686,6 +1721,7 @@ bool CheckPrintfHandler::HandleAmount(
S.Diag(getLocationOfByte(Amt.getStart()),
diag::warn_printf_asterisk_missing_arg)
<< k << getSpecifierRange(startSpecifier, specifierLen);
+ NoteCallLocation();
// Don't do any more checking. We will just emit
// spurious errors.
return false;
@@ -1709,6 +1745,7 @@ bool CheckPrintfHandler::HandleAmount(
<< ATR.getRepresentativeType(S.Context) << T
<< getSpecifierRange(startSpecifier, specifierLen)
<< Arg->getSourceRange();
+ NoteCallLocation();
// Don't do any more checking. We will just emit
// spurious errors.
return false;
@@ -1735,6 +1772,7 @@ void CheckPrintfHandler::HandleInvalidAmount(
<< getSpecifierRange(startSpecifier, specifierLen)
<< FixItHint::CreateRemoval(getSpecifierRange(Amt.getStart(),
Amt.getConstantLength()));
+ NoteCallLocation();
break;
default:
@@ -1743,6 +1781,7 @@ void CheckPrintfHandler::HandleInvalidAmount(
<< type
<< CS.toString()
<< getSpecifierRange(startSpecifier, specifierLen);
+ NoteCallLocation();
break;
}
}
@@ -1759,6 +1798,7 @@ void CheckPrintfHandler::HandleFlag(const analyze_printf::PrintfSpecifier &FS,
<< flag.toString() << CS.toString()
<< getSpecifierRange(startSpecifier, specifierLen)
<< FixItHint::CreateRemoval(getSpecifierRange(flag.getPosition(), 1));
+ NoteCallLocation();
}
void CheckPrintfHandler::HandleIgnoredFlag(
@@ -1774,6 +1814,7 @@ void CheckPrintfHandler::HandleIgnoredFlag(
<< getSpecifierRange(startSpecifier, specifierLen)
<< FixItHint::CreateRemoval(getSpecifierRange(
ignoredFlag.getPosition(), 1));
+ NoteCallLocation();
}
bool
@@ -1796,6 +1837,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
S.Diag(getLocationOfByte(CS.getStart()),
diag::warn_format_mix_positional_nonpositional_args)
<< getSpecifierRange(startSpecifier, specifierLen);
+ NoteCallLocation();
return false;
}
}
@@ -1870,19 +1912,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())
+ 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()));
+ NoteCallLocation();
+ }
// 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);
+ NoteCallLocation();
// Continue checking the other format specifiers.
return true;
}
@@ -1939,6 +1984,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
<< getSpecifierRange(startSpecifier, specifierLen)
<< Ex->getSourceRange();
}
+ NoteCallLocation();
}
return true;
@@ -1975,6 +2021,7 @@ void CheckScanfHandler::HandleIncompleteScanList(const char *start,
const char *end) {
S.Diag(getLocationOfByte(end), diag::warn_scanf_scanlist_incomplete)
<< getSpecifierRange(start, end - start);
+ NoteCallLocation();
}
bool CheckScanfHandler::HandleInvalidScanfConversionSpecifier(
@@ -2013,6 +2060,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
S.Diag(getLocationOfByte(CS.getStart()),
diag::warn_format_mix_positional_nonpositional_args)
<< getSpecifierRange(startSpecifier, specifierLen);
+ NoteCallLocation();
return false;
}
}
@@ -2026,6 +2074,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
S.Diag(getLocationOfByte(Amt.getStart()),
diag::warn_scanf_nonzero_width)
<< R << FixItHint::CreateRemoval(R);
+ NoteCallLocation();
}
}
@@ -2053,6 +2102,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
<< getSpecifierRange(startSpecifier, specifierLen)
<< FixItHint::CreateRemoval(getSpecifierRange(LM.getStart(),
LM.getLength()));
+ NoteCallLocation();
}
// The remaining checks depend on the data arguments.
diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index 6b5f7e2..59a7632 100644
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -70,6 +70,8 @@ void check_writeback_specifier()
void check_invalid_specifier(FILE* fp, char *buf)
{
printf("%s%lb%d","unix",10,20); // expected-warning {{invalid conversion specifier 'b'}}
+ const char fmt[] = "%s%lb%d"; // expected-warning {{invalid conversion specifier 'b'}}
+ printf(fmt,"unix",10,20); // expected-note {{format string used here}}
fprintf(fp,"%%%l"); // expected-warning {{incomplete format specifier}}
sprintf(buf,"%%%%%ld%d%d", 1, 2, 3); // expected-warning{{conversion specifies type 'long' but the argument has type 'int'}}
snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning{{conversion specifies type 'long' but the argument has type 'int'}} expected-warning {{invalid conversion specifier ';'}}
@@ -151,6 +153,8 @@ void torture(va_list v8) {
void test10(int x, float f, int i, long long lli) {
printf("%s"); // expected-warning{{more '%' conversions than data arguments}}
+ const char fmt[] = "%s"; // expected-warning{{more '%' conversions than data arguments}}
+ printf(fmt); // expected-note{{format string used here}}
printf("%@", 12); // expected-warning{{invalid conversion specifier '@'}}
printf("\0"); // expected-warning{{format string contains '\0' within the string body}}
printf("xs\0"); // expected-warning{{format string contains '\0' within the string body}}
@@ -386,4 +390,3 @@ void test_suppress_invalid_specifier() {
printf("%@", 12); // no-warning
#pragma clang diagnostic pop
}
-
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
