From ef6fa17f79523b7547dbf5b2dc5c049b26f687af Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Tue, 29 Mar 2016 17:35:02 +0000 Subject: [PATCH] [Sema] Handle UTF-8 invalid format string specifiers Improve invalid format string specifier handling by printing out invalid specifiers characters with \x, \u and \U. Previously clang would print gargabe whenever the character is unprintable. Example, before: NSLog(@"%\u25B9"); => warning: invalid conversion specifier ' [-Wformat-invalid-specifier] after: NSLog(@"%\u25B9"); => warning: invalid conversion specifier '\u25b9' [-Wformat-invalid-specifier] Differential Revision: http://reviews.llvm.org/D18296 rdar://problem/24672159 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@264752 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/FormatString.h | 8 +--- lib/Analysis/FormatString.cpp | 23 ++++++++++ lib/Analysis/FormatStringParsing.h | 8 +++- lib/Analysis/PrintfFormatString.cpp | 7 ++- lib/Analysis/ScanfFormatString.cpp | 11 +++-- lib/Sema/SemaChecking.cpp | 43 ++++++++++++++++--- test/Sema/format-strings-scanf.c | 8 ++++ test/Sema/format-strings.c | 8 ++++ test/SemaObjC/format-strings-objc.m | 8 ++++ 9 files changed, 106 insertions(+), 18 deletions(-) diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index 4471311a33..a593e9853e 100644 --- a/include/clang/Analysis/Analyses/FormatString.h +++ b/include/clang/Analysis/Analyses/FormatString.h @@ -210,6 +210,7 @@ public: unsigned getLength() const { return EndScanList ? EndScanList - Position : 1; } + void setEndScanList(const char *pos) { EndScanList = pos; } bool isIntArg() const { return (kind >= IntArgBeg && kind <= IntArgEnd) || kind == FreeBSDrArg || kind == FreeBSDyArg; } @@ -413,11 +414,6 @@ public: bool isObjCArg() const { return kind >= ObjCBeg && kind <= ObjCEnd; } bool isDoubleArg() const { return kind >= DoubleArgBeg && kind <= DoubleArgEnd; } - unsigned getLength() const { - // Conversion specifiers currently only are represented by - // single characters, but we be flexible. - return 1; - } static bool classof(const analyze_format_string::ConversionSpecifier *CS) { return CS->isPrintfKind(); @@ -546,8 +542,6 @@ public: ScanfConversionSpecifier(const char *pos, Kind k) : ConversionSpecifier(false, pos, k) {} - void setEndScanList(const char *pos) { EndScanList = pos; } - static bool classof(const analyze_format_string::ConversionSpecifier *CS) { return !CS->isPrintfKind(); } diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp index 1c42ec0e87..badc71021a 100644 --- a/lib/Analysis/FormatString.cpp +++ b/lib/Analysis/FormatString.cpp @@ -15,6 +15,7 @@ #include "FormatStringParsing.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" +#include "llvm/Support/ConvertUTF.h" using clang::analyze_format_string::ArgType; using clang::analyze_format_string::FormatStringHandler; @@ -260,6 +261,28 @@ clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS, return true; } +bool clang::analyze_format_string::ParseUTF8InvalidSpecifier( + const char *SpecifierBegin, const char *FmtStrEnd, unsigned &Len) { + if (SpecifierBegin + 1 >= FmtStrEnd) + return false; + + const UTF8 *SB = reinterpret_cast(SpecifierBegin + 1); + const UTF8 *SE = reinterpret_cast(FmtStrEnd); + const char FirstByte = *SB; + + // If the invalid specifier is a multibyte UTF-8 string, return the + // total length accordingly so that the conversion specifier can be + // properly updated to reflect a complete UTF-8 specifier. + unsigned NumBytes = getNumBytesForUTF8(FirstByte); + if (NumBytes == 1) + return false; + if (SB + NumBytes > SE) + return false; + + Len = NumBytes + 1; + return true; +} + //===----------------------------------------------------------------------===// // Methods on ArgType. //===----------------------------------------------------------------------===// diff --git a/lib/Analysis/FormatStringParsing.h b/lib/Analysis/FormatStringParsing.h index e1652964b8..8463fcec5b 100644 --- a/lib/Analysis/FormatStringParsing.h +++ b/lib/Analysis/FormatStringParsing.h @@ -46,7 +46,13 @@ bool ParseArgPosition(FormatStringHandler &H, /// FormatSpecifier& argument, and false otherwise. bool ParseLengthModifier(FormatSpecifier &FS, const char *&Beg, const char *E, const LangOptions &LO, bool IsScanf = false); - + +/// Returns true if the invalid specifier in \p SpecifierBegin is a UTF-8 +/// string; check that it won't go further than \p FmtStrEnd and write +/// up the total size in \p Len. +bool ParseUTF8InvalidSpecifier(const char *SpecifierBegin, + const char *FmtStrEnd, unsigned &Len); + template class SpecifierResult { T FS; const char *Start; diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp index f0976bce97..fb5df61c5e 100644 --- a/lib/Analysis/PrintfFormatString.cpp +++ b/lib/Analysis/PrintfFormatString.cpp @@ -312,8 +312,13 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H, argIndex++; if (k == ConversionSpecifier::InvalidSpecifier) { + unsigned Len = I - Start; + if (ParseUTF8InvalidSpecifier(Start, E, Len)) { + CS.setEndScanList(Start + Len); + FS.setConversionSpecifier(CS); + } // Assume the conversion takes one argument. - return !H.HandleInvalidPrintfConversionSpecifier(FS, Start, I - Start); + return !H.HandleInvalidPrintfConversionSpecifier(FS, Start, Len); } return PrintfSpecifierResult(Start, FS); } diff --git a/lib/Analysis/ScanfFormatString.cpp b/lib/Analysis/ScanfFormatString.cpp index d484d8e828..82b038864c 100644 --- a/lib/Analysis/ScanfFormatString.cpp +++ b/lib/Analysis/ScanfFormatString.cpp @@ -79,7 +79,7 @@ static ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H, unsigned &argIndex, const LangOptions &LO, const TargetInfo &Target) { - + using namespace clang::analyze_format_string; using namespace clang::analyze_scanf; const char *I = Beg; const char *Start = nullptr; @@ -210,10 +210,15 @@ static ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H, // FIXME: '%' and '*' doesn't make sense. Issue a warning. // FIXME: 'ConsumedSoFar' and '*' doesn't make sense. - + if (k == ScanfConversionSpecifier::InvalidSpecifier) { + unsigned Len = I - Beg; + if (ParseUTF8InvalidSpecifier(Beg, E, Len)) { + CS.setEndScanList(Beg + Len); + FS.setConversionSpecifier(CS); + } // Assume the conversion takes one argument. - return !H.HandleInvalidScanfConversionSpecifier(FS, Beg, I - Beg); + return !H.HandleInvalidScanfConversionSpecifier(FS, Beg, Len); } return ScanfSpecifierResult(Start, FS); } diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index cc261e0596..062041e377 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -36,6 +36,8 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallString.h" +#include "llvm/Support/Format.h" +#include "llvm/Support/Locale.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/raw_ostream.h" #include @@ -3976,12 +3978,41 @@ CheckFormatHandler::HandleInvalidConversionSpecifier(unsigned argIndex, // gibberish when trying to match arguments. keepGoing = false; } - - EmitFormatDiagnostic(S.PDiag(diag::warn_format_invalid_conversion) - << StringRef(csStart, csLen), - Loc, /*IsStringLocation*/true, - getSpecifierRange(startSpec, specifierLen)); - + + StringRef Specifier(csStart, csLen); + + // If the specifier in non-printable, it could be the first byte of a UTF-8 + // sequence. In that case, print the UTF-8 code point. If not, print the byte + // hex value. + std::string CodePointStr; + if (!llvm::sys::locale::isPrint(*csStart)) { + UTF32 CodePoint; + const UTF8 **B = reinterpret_cast(&csStart); + const UTF8 *E = + reinterpret_cast(csStart + csLen); + ConversionResult Result = + llvm::convertUTF8Sequence(B, E, &CodePoint, strictConversion); + + if (Result != conversionOK) { + unsigned char FirstChar = *csStart; + CodePoint = (UTF32)FirstChar; + } + + llvm::raw_string_ostream OS(CodePointStr); + if (CodePoint < 256) + OS << "\\x" << llvm::format("%02x", CodePoint); + else if (CodePoint <= 0xFFFF) + OS << "\\u" << llvm::format("%04x", CodePoint); + else + OS << "\\U" << llvm::format("%08x", CodePoint); + OS.flush(); + Specifier = CodePointStr; + } + + EmitFormatDiagnostic( + S.PDiag(diag::warn_format_invalid_conversion) << Specifier, Loc, + /*IsStringLocation*/ true, getSpecifierRange(startSpec, specifierLen)); + return keepGoing; } diff --git a/test/Sema/format-strings-scanf.c b/test/Sema/format-strings-scanf.c index 7a92842b24..ee2be0e646 100644 --- a/test/Sema/format-strings-scanf.c +++ b/test/Sema/format-strings-scanf.c @@ -183,3 +183,11 @@ void check_conditional_literal(char *s, int *i) { scanf(i ? "%d" : "%d", i, s); // expected-warning{{data argument not used}} scanf(i ? "%s" : "%d", s); // expected-warning{{format specifies type 'int *'}} } + +void testInvalidNoPrintable(int *a) { + scanf("%\u25B9", a); // expected-warning {{invalid conversion specifier '\u25b9'}} + scanf("%\xE2\x96\xB9", a); // expected-warning {{invalid conversion specifier '\u25b9'}} + scanf("%\U00010348", a); // expected-warning {{invalid conversion specifier '\U00010348'}} + scanf("%\xF0\x90\x8D\x88", a); // expected-warning {{invalid conversion specifier '\U00010348'}} + scanf("%\xe2", a); // expected-warning {{invalid conversion specifier '\xe2'}} +} diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c index 5559710c60..253aa57bec 100644 --- a/test/Sema/format-strings.c +++ b/test/Sema/format-strings.c @@ -642,6 +642,14 @@ void test_qualifiers(volatile int *vip, const int *cip, printf("%n", (cip_t)0); // expected-warning{{format specifies type 'int *' but the argument has type 'cip_t' (aka 'const int *')}} } +void testInvalidNoPrintable() { + printf("%\u25B9"); // expected-warning {{invalid conversion specifier '\u25b9'}} + printf("%\xE2\x96\xB9"); // expected-warning {{invalid conversion specifier '\u25b9'}} + printf("%\U00010348"); // expected-warning {{invalid conversion specifier '\U00010348'}} + printf("%\xF0\x90\x8D\x88"); // expected-warning {{invalid conversion specifier '\U00010348'}} + printf("%\xe2"); // expected-warning {{invalid conversion specifier '\xe2'}} +} + #pragma GCC diagnostic ignored "-Wformat-nonliteral" #pragma GCC diagnostic warning "-Wformat-security" // diff --git a/test/SemaObjC/format-strings-objc.m b/test/SemaObjC/format-strings-objc.m index a1ebf03f8e..2ac68cd0d2 100644 --- a/test/SemaObjC/format-strings-objc.m +++ b/test/SemaObjC/format-strings-objc.m @@ -265,3 +265,11 @@ void testObjCModifierFlags() { NSLog(@"%2$[tt]@ %1$[tt]s", @"Foo", @"Bar"); // expected-warning {{object format flags cannot be used with 's' conversion specifier}} } +// Test Objective-C invalid no printable specifiers +void testObjcInvalidNoPrintable(int *a) { + NSLog(@"%\u25B9"); // expected-warning {{invalid conversion specifier '\u25b9'}} + NSLog(@"%\xE2\x96\xB9"); // expected-warning {{invalid conversion specifier '\u25b9'}} + NSLog(@"%\U00010348"); // expected-warning {{invalid conversion specifier '\U00010348'}} + NSLog(@"%\xF0\x90\x8D\x88"); // expected-warning {{invalid conversion specifier '\U00010348'}} + NSLog(@"%\xe2"); // expected-warning {{input conversion stopped}} expected-warning {{invalid conversion specifier '\xe2'}} +} -- 2.40.0