scanf format checking: include the buffer length in the fix-it for %s.

Patch by Zach Davis!

llvm-svn: 204300

4 files changed, 48 insertions, 11 deletions

diff --git a/clang/include/clang/Analysis/Analyses/FormatString.h b/clang/include/clang/Analysis/Analyses/FormatString.h
index c9516b50cae..3bffcd3ce00 100644
--- a/clang/include/clang/Analysis/Analyses/FormatString.h
+++ b/clang/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/clang/lib/Analysis/ScanfFormatString.cpp b/clang/lib/Analysis/ScanfFormatString.cpp
index f5ce84fe361..3ff7f0ad2e5 100644
--- a/clang/lib/Analysis/ScanfFormatString.cpp
+++ b/clang/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<EnumType>())
-    QT = ETy->getDecl()->getIntegerType();
-  
+  if (const EnumType *ETy = PT->getAs<EnumType>())
+    PT = ETy->getDecl()->getIntegerType();
+
   const BuiltinType *BT = PT->getAs<BuiltinType>();
   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/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 74ca197b8de..e45f2e53337 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/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/clang/test/Sema/format-strings-fixit.c b/clang/test/Sema/format-strings-fixit.c
index 31274185cbc..d9d7fb3fbad 100644
--- a/clang/test/Sema/format-strings-fixit.c
+++ b/clang/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);