From d635c5fcc45c952b75743dd2f4c86d6950e4954e Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Sat, 30 Jan 2010 00:49:51 +0000 Subject: [PATCH] Add basic type checking of format string conversion specifiers and their arguments. Thanks to Cristian Draghici for his help with this patch! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@94864 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 7 ++- lib/Sema/SemaChecking.cpp | 69 ++++++++++++++++++++-- test/Sema/format-strings.c | 12 +++- test/Sema/ucn-cstring.c | 4 +- 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index bdd58d6d15..e15bf20f0c 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2449,6 +2449,9 @@ def warn_printf_incomplete_specifier : Warning< "incomplete format specifier">, InGroup; def warn_printf_missing_format_string : Warning< "format string missing">, InGroup; +def warn_printf_conversion_argument_type_mismatch : Warning< + "conversion specifies type %0 but the argument has type %1">, + InGroup; def warn_null_arg : Warning< "null passed to a callee which requires a non-null argument">, InGroup; @@ -2463,10 +2466,10 @@ def warn_printf_asterisk_width_missing_arg : Warning< def warn_printf_asterisk_precision_missing_arg : Warning< "'.*' specified field precision is missing a matching 'int' argument">; def warn_printf_asterisk_width_wrong_type : Warning< - "field width should have type 'int', but argument has type %0">, + "field width should have type %0, but argument has type %1">, InGroup; def warn_printf_asterisk_precision_wrong_type : Warning< - "field precision should have type 'int', but argument has type %0">, + "field precision should have type %0, but argument has type %1">, InGroup; // CHECK: returning address/reference of stack memory diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index f10c8a17fc..1e7641690d 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1081,6 +1081,8 @@ private: unsigned MissingArgDiag, unsigned BadTypeDiag, const char *startSpecifier, unsigned specifierLen); + bool MatchType(QualType A, QualType B, bool ignoreSign); + const Expr *getDataArg(unsigned i) const; }; } @@ -1132,6 +1134,47 @@ const Expr *CheckPrintfHandler::getDataArg(unsigned i) const { return TheCall->getArg(FormatIdx + i); } +bool CheckPrintfHandler::MatchType(QualType A, QualType B, bool ignoreSign) { + A = S.Context.getCanonicalType(A).getUnqualifiedType(); + B = S.Context.getCanonicalType(B).getUnqualifiedType(); + + if (A == B) + return true; + + if (ignoreSign) { + if (const BuiltinType *BT = B->getAs()) { + switch (BT->getKind()) { + default: + return false; + case BuiltinType::Char_S: + case BuiltinType::SChar: + return A == S.Context.UnsignedCharTy; + case BuiltinType::Char_U: + case BuiltinType::UChar: + return A == S.Context.SignedCharTy; + case BuiltinType::Short: + return A == S.Context.UnsignedShortTy; + case BuiltinType::UShort: + return A == S.Context.ShortTy; + case BuiltinType::Int: + return A == S.Context.UnsignedIntTy; + case BuiltinType::UInt: + return A == S.Context.IntTy; + case BuiltinType::Long: + return A == S.Context.UnsignedLongTy; + case BuiltinType::ULong: + return A == S.Context.LongTy; + case BuiltinType::LongLong: + return A == S.Context.UnsignedLongLongTy; + case BuiltinType::ULongLong: + return A == S.Context.LongLongTy; + } + return A == B; + } + } + return false; +} + bool CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned MissingArgDiag, @@ -1156,13 +1199,11 @@ CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt, // doesn't emit a warning for that case. const Expr *Arg = getDataArg(NumConversions); QualType T = Arg->getType(); - const BuiltinType *BT = T->getAs(); - if (!BT || (BT->getKind() != BuiltinType::Int && - BT->getKind() != BuiltinType::UInt)) { + if (!MatchType(T, S.Context.IntTy, true)) { S.Diag(getLocationOfByte(Amt.getStart()), BadTypeDiag) - << T - << getFormatSpecifierRange(startSpecifier, specifierLen) - << Arg->getSourceRange(); + << S.Context.IntTy << T + << getFormatSpecifierRange(startSpecifier, specifierLen) + << Arg->getSourceRange(); // Don't do any more checking. We will just emit // spurious errors. return false; @@ -1234,6 +1275,22 @@ CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier // Don't do any more checking. return false; } + + // Now type check the data expression that matches the + // format specifier. + const Expr *Ex = getDataArg(NumConversions); + const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context); + + if (const QualType *T = ATR.getSpecificType()) { + if (!MatchType(*T, Ex->getType(), true)) { + S.Diag(getLocationOfByte(CS.getStart()), + diag::warn_printf_conversion_argument_type_mismatch) + << *T << Ex->getType() + << getFormatSpecifierRange(startSpecifier, specifierLen) + << Ex->getSourceRange(); + } + return true; + } return true; } diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c index 94fb593730..f1ab868dc7 100644 --- a/test/Sema/format-strings.c +++ b/test/Sema/format-strings.c @@ -71,8 +71,8 @@ void check_invalid_specifier(FILE* fp, char *buf) { printf("%s%lb%d","unix",10,20); // expected-warning {{invalid conversion specifier 'b'}} fprintf(fp,"%%%l"); // expected-warning {{incomplete format specifier}} - sprintf(buf,"%%%%%ld%d%d", 1, 2, 3); // no-warning - snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning {{invalid conversion 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 ';'}} } void check_null_char_string(char* b) @@ -162,3 +162,11 @@ void test10(int x, float f, int i) { printf("%.", x); // expected-warning{{incomplete format 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))); +void test_asl(aslclient asl) { + // Test case from . + asl_log(asl, 0, 3, "Error: %m"); // no-warning + asl_log(asl, 0, 3, "Error: %W"); // expected-warning{{invalid conversion specifier 'W'}} +} diff --git a/test/Sema/ucn-cstring.c b/test/Sema/ucn-cstring.c index f5bf457ed1..ac1d37f186 100644 --- a/test/Sema/ucn-cstring.c +++ b/test/Sema/ucn-cstring.c @@ -5,8 +5,8 @@ int printf(const char *, ...); int main(void) { int a[sizeof("hello \u2192 \u2603 \u2190 world") == 24 ? 1 : -1]; - printf("%s (%d)\n", "hello \u2192 \u2603 \u2190 world", sizeof("hello \u2192 \u2603 \u2190 world")); - printf("%s (%d)\n", "\U00010400\U0001D12B", sizeof("\U00010400\U0001D12B")); + printf("%s (%zd)\n", "hello \u2192 \u2603 \u2190 world", sizeof("hello \u2192 \u2603 \u2190 world")); + printf("%s (%zd)\n", "\U00010400\U0001D12B", sizeof("\U00010400\U0001D12B")); // Some error conditions... printf("%s\n", "\U"); // expected-error{{\u used with no following hex digits}} printf("%s\n", "\U00"); // expected-error{{incomplete universal character name}} -- 2.40.0