From 863a5e8b570d4ae0cfe22bf985a89ac33a508593 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Thu, 20 Mar 2014 03:32:39 +0000 Subject: [PATCH] scanf format checking: include the buffer length in the fix-it for %s. Patch by Zach Davis! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@204300 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/FormatString.h | 3 +- lib/Analysis/ScanfFormatString.cpp | 23 +++++++++++---- lib/Sema/SemaChecking.cpp | 5 ++-- test/Sema/format-strings-fixit.c | 28 +++++++++++++++++-- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index c9516b50ca..3bffcd3ce0 100644 --- a/include/clang/Analysis/Analyses/FormatString.h +++ b/include/clang/Analysis/Analyses/FormatString.h @@ -572,7 +572,8 @@ public: ArgType getArgType(ASTContext &Ctx) const; - bool fixType(QualType QT, const LangOptions &LangOpt, ASTContext &Ctx); + bool fixType(QualType QT, QualType RawQT, const LangOptions &LangOpt, + ASTContext &Ctx); void toString(raw_ostream &os) const; diff --git a/lib/Analysis/ScanfFormatString.cpp b/lib/Analysis/ScanfFormatString.cpp index f5ce84fe36..3ff7f0ad2e 100644 --- a/lib/Analysis/ScanfFormatString.cpp +++ b/lib/Analysis/ScanfFormatString.cpp @@ -379,21 +379,23 @@ ArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { return ArgType(); } -bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt, +bool ScanfSpecifier::fixType(QualType QT, QualType RawQT, + const LangOptions &LangOpt, ASTContext &Ctx) { - if (!QT->isPointerType()) - return false; // %n is different from other conversion specifiers; don't try to fix it. if (CS.getKind() == ConversionSpecifier::nArg) return false; + if (!QT->isPointerType()) + return false; + QualType PT = QT->getPointeeType(); // If it's an enum, get its underlying type. - if (const EnumType *ETy = QT->getAs()) - QT = ETy->getDecl()->getIntegerType(); - + if (const EnumType *ETy = PT->getAs()) + PT = ETy->getDecl()->getIntegerType(); + const BuiltinType *BT = PT->getAs(); if (!BT) return false; @@ -405,6 +407,15 @@ bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt, LM.setKind(LengthModifier::AsWideChar); else LM.setKind(LengthModifier::None); + + // If we know the target array length, we can use it as a field width. + if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(RawQT)) { + if (CAT->getSizeModifier() == ArrayType::Normal) + FieldWidth = OptionalAmount(OptionalAmount::Constant, + CAT->getSize().getZExtValue() - 1, + "", 0, false); + + } return true; } diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 74ca197b8d..e45f2e5333 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3518,8 +3518,9 @@ bool CheckScanfHandler::HandleScanfSpecifier( const analyze_format_string::ArgType &AT = FS.getArgType(S.Context); if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) { ScanfSpecifier fixedFS = FS; - bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(), - S.Context); + bool success = fixedFS.fixType(Ex->getType(), + Ex->IgnoreImpCasts()->getType(), + S.getLangOpts(), S.Context); if (success) { // Get the fix string from the fixed format specifier. diff --git a/test/Sema/format-strings-fixit.c b/test/Sema/format-strings-fixit.c index 31274185cb..d9d7fb3fba 100644 --- a/test/Sema/format-strings-fixit.c +++ b/test/Sema/format-strings-fixit.c @@ -14,6 +14,7 @@ typedef __SIZE_TYPE__ size_t; typedef __INTMAX_TYPE__ intmax_t; typedef __UINTMAX_TYPE__ uintmax_t; typedef __PTRDIFF_TYPE__ ptrdiff_t; +typedef __WCHAR_TYPE__ wchar_t; void test() { // Basic types @@ -97,11 +98,14 @@ void test() { int scanf(char const *, ...); -void test2() { +void test2(int intSAParm[static 2]) { char str[100]; + char *vstr = "abc"; + wchar_t wstr[100]; short shortVar; unsigned short uShortVar; int intVar; + int intAVar[2]; unsigned uIntVar; float floatVar; double doubleVar; @@ -114,11 +118,22 @@ void test2() { intmax_t intmaxVar; uintmax_t uIntmaxVar; ptrdiff_t ptrdiffVar; + enum {A, B, C} enumVar; + // Some string types. scanf("%lf", str); + scanf("%lf", vstr); + scanf("%ls", str); + scanf("%f", wstr); // FIXME: wchar_t should resolve to %ls, not %d. + scanf("%s", wstr); // FIXME: wchar_t should resolve to %ls, not %d. + scanf("%ls", str); + + // Some integer types. scanf("%f", &shortVar); scanf("%f", &uShortVar); scanf("%p", &intVar); + scanf("%f", intAVar); + scanf("%f", intSAParm); scanf("%Lf", &uIntVar); scanf("%ld", &floatVar); scanf("%f", &doubleVar); @@ -127,6 +142,7 @@ void test2() { scanf("%f", &uLongVar); scanf("%f", &longLongVar); scanf("%f", &uLongLongVar); + scanf("%d", &enumVar); // FIXME: We ought to fix specifiers for enums. // Some named ints. scanf("%f", &sizeVar); @@ -206,10 +222,17 @@ void test2() { // CHECK: printf("%La", (long double) 42); // CHECK: printf("%LA", (long double) 42); -// CHECK: scanf("%s", str); +// CHECK: scanf("%99s", str); +// CHECK: scanf("%s", vstr); +// CHECK: scanf("%99s", str); +// CHECK: scanf("%d", wstr); +// CHECK: scanf("%d", wstr); +// CHECK: scanf("%99s", str); // CHECK: scanf("%hd", &shortVar); // CHECK: scanf("%hu", &uShortVar); // CHECK: scanf("%d", &intVar); +// CHECK: scanf("%d", intAVar); +// CHECK: scanf("%d", intSAParm); // CHECK: scanf("%u", &uIntVar); // CHECK: scanf("%f", &floatVar); // CHECK: scanf("%lf", &doubleVar); @@ -218,6 +241,7 @@ void test2() { // CHECK: scanf("%lu", &uLongVar); // CHECK: scanf("%lld", &longLongVar); // CHECK: scanf("%llu", &uLongLongVar); +// CHECK: scanf("%d", &enumVar); // CHECK: scanf("%zu", &sizeVar); // CHECK: scanf("%jd", &intmaxVar); // CHECK: scanf("%ju", &uIntmaxVar); -- 2.40.0