diff --git a/clang/include/clang/Analysis/Analyses/PrintfFormatString.h b/clang/include/clang/Analysis/Analyses/PrintfFormatString.h index a5c0b23c2d0f..0153f724a721 100644 --- a/clang/include/clang/Analysis/Analyses/PrintfFormatString.h +++ b/clang/include/clang/Analysis/Analyses/PrintfFormatString.h @@ -73,6 +73,10 @@ public: const char *getStart() const { return Position; } + + llvm::StringRef getCharacters() const { + return llvm::StringRef(getStart(), getLength()); + } bool consumesDataArgument() const { switch (kind) { @@ -232,6 +236,7 @@ public: bool hasPlusPrefix() const { return (bool) HasPlusPrefix; } bool hasAlternativeForm() const { return (bool) HasAlternativeForm; } bool hasLeadingZeros() const { return (bool) HasLeadingZeroes; } + bool hasSpacePrefix() const { return (bool) HasSpacePrefix; } }; class FormatStringHandler { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index aacaf4e42936..84dbaca6de6d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2514,7 +2514,13 @@ def warn_printf_asterisk_width_wrong_type : Warning< def warn_printf_asterisk_precision_wrong_type : Warning< "field precision should have type %0, but argument has type %1">, InGroup; - +def warn_printf_nonsensical_precision: Warning< + "precision used in '%0' conversion specifier (where it has no meaning)">, + InGroup; +def warn_printf_nonsensical_flag: Warning< + "flag '%0' results in undefined behavior in '%1' conversion specifier">, + InGroup; + // 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/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 845102376169..3a77552f4828 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1080,6 +1080,9 @@ private: bool HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned MissingArgDiag, unsigned BadTypeDiag, const char *startSpecifier, unsigned specifierLen); + void HandleFlags(const analyze_printf::FormatSpecifier &FS, + llvm::StringRef flag, llvm::StringRef cspec, + const char *startSpecifier, unsigned specifierLen); bool MatchType(QualType A, QualType B, bool ignoreSign); @@ -1175,6 +1178,16 @@ bool CheckPrintfHandler::MatchType(QualType A, QualType B, bool ignoreSign) { return false; } +void CheckPrintfHandler::HandleFlags(const analyze_printf::FormatSpecifier &FS, + llvm::StringRef flag, + llvm::StringRef cspec, + const char *startSpecifier, + unsigned specifierLen) { + const analyze_printf::ConversionSpecifier &CS = FS.getConversionSpecifier(); + S.Diag(getLocationOfByte(CS.getStart()), diag::warn_printf_nonsensical_flag) + << flag << cspec << getFormatSpecifierRange(startSpecifier, specifierLen); +} + bool CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned MissingArgDiag, @@ -1214,7 +1227,8 @@ CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt, } bool -CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS, +CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier + &FS, const char *startSpecifier, unsigned specifierLen) { @@ -1262,7 +1276,25 @@ CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier // Continue checking the other format specifiers. return true; } - + + if (CS.getKind() == ConversionSpecifier::VoidPtrArg) { + if (FS.getPrecision().getHowSpecified() != OptionalAmount::NotSpecified) + S.Diag(getLocationOfByte(CS.getStart()), + diag::warn_printf_nonsensical_precision) + << CS.getCharacters() + << getFormatSpecifierRange(startSpecifier, specifierLen); + } + if (CS.getKind() == ConversionSpecifier::VoidPtrArg || + CS.getKind() == ConversionSpecifier::CStrArg) { + // FIXME: Instead of using "0", "+", etc., eventually get them from + // the FormatSpecifier. + if (FS.hasLeadingZeros()) + HandleFlags(FS, "0", CS.getCharacters(), startSpecifier, specifierLen); + if (FS.hasPlusPrefix()) + HandleFlags(FS, "+", CS.getCharacters(), startSpecifier, specifierLen); + if (FS.hasSpacePrefix()) + HandleFlags(FS, " ", CS.getCharacters(), startSpecifier, specifierLen); + } // The remaining checks depend on the data arguments. if (HasVAListArg) diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c index 5ce3eb036ebf..1bbc68cd0d5e 100644 --- a/clang/test/Sema/format-strings.c +++ b/clang/test/Sema/format-strings.c @@ -171,6 +171,18 @@ void test10(int x, float f, int i, long long lli) { printf("%f\n", (long double) 1.0); // expected-warning{{conversion specifies type 'double' but the argument has type 'long double'}} } +void test11(void *p, char *s) { + printf("%p", p); // no-warning + printf("%.4p", p); // expected-warning{{precision used in 'p' conversion specifier (where it has no meaning)}} + printf("%+p", p); // expected-warning{{flag '+' results in undefined behavior in 'p' conversion specifier}} + printf("% p", p); // expected-warning{{flag ' ' results in undefined behavior in 'p' conversion specifier}} + printf("%0p", p); // expected-warning{{flag '0' results in undefined behavior in 'p' conversion specifier}} + printf("%s", s); // no-warning + printf("%+s", p); // expected-warning{{flag '+' results in undefined behavior in 's' conversion specifier}} + printf("% s", p); // expected-warning{{flag ' ' results in undefined behavior in 's' conversion specifier}} + printf("%0s", p); // expected-warning{{flag '0' results in undefined behavior in 's' conversion specifier}} +} + typedef struct __aslclient *aslclient; typedef struct __aslmsg *aslmsg; int asl_log(aslclient asl, aslmsg msg, int level, const char *format, ...) __attribute__((__format__ (__printf__, 4, 5)));