[cfe-commits] r108757 - in /cfe/trunk: include/clang/Analysis/Analyses/FormatString.h lib/Sema/SemaChecking.cpp test/Sema/format-strings-scanf.c test/Sema/format-strings.c

[Ted Kremenek]

Author: kremenek
Date: Mon Jul 19 17:01:06 2010
New Revision: 108757

URL: http://llvm.org/viewvc/llvm-project?rev=108757&view=rev
Log:
Don't warn when a '%%' or '%*d' (scanf) is used in a format string with positional arguments, since
these don't actually consume an argument.

Modified:
    cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings-scanf.c
    cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=108757&r1=108756&r2=108757&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Mon Jul 19 17:01:06 2010
@@ -220,17 +220,14 @@
   void setUsesPositionalArg() { UsesPositionalArg = true; }
 
   void setArgIndex(unsigned i) {
-      // assert(CS.consumesDataArgument());
     argIndex = i;
   }
 
   unsigned getArgIndex() const {
-      //assert(CS.consumesDataArgument());
     return argIndex;
   }
 
   unsigned getPositionalArgIndex() const {
-      //assert(CS.consumesDataArgument());
     return argIndex + 1;
   }
 
@@ -402,12 +399,16 @@
   const OptionalAmount &getPrecision() const {
     return Precision;
   }
+  
+  bool consumesDataArgument() const {
+    return CS.consumesDataArgument();
+  }
 
-    /// \brief Returns the builtin type that a data argument
-    /// paired with this format specifier should have.  This method
-    /// will return null if the format specifier does not have
-    /// a matching data argument or the matching argument matches
-    /// more than one type.
+  /// \brief Returns the builtin type that a data argument
+  /// paired with this format specifier should have.  This method
+  /// will return null if the format specifier does not have
+  /// a matching data argument or the matching argument matches
+  /// more than one type.
   ArgTypeResult getArgType(ASTContext &Ctx) const;
 
   const OptionalFlag &isLeftJustified() const { return IsLeftJustified; }

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=108757&r1=108756&r2=108757&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jul 19 17:01:06 2010
@@ -1434,16 +1434,18 @@
   using namespace analyze_printf;  
   const ConversionSpecifier &CS = FS.getConversionSpecifier();
 
-  if (atFirstArg) {
-    atFirstArg = false;
-    usesPositionalArgs = FS.usesPositionalArg();
-  }
-  else if (usesPositionalArgs != FS.usesPositionalArg()) {
-    // Cannot mix-and-match positional and non-positional arguments.
-    S.Diag(getLocationOfByte(CS.getStart()),
-           diag::warn_format_mix_positional_nonpositional_args)
-      << getSpecifierRange(startSpecifier, specifierLen);
-    return false;
+  if (FS.consumesDataArgument()) {
+    if (atFirstArg) {
+        atFirstArg = false;
+        usesPositionalArgs = FS.usesPositionalArg();
+    }
+    else if (usesPositionalArgs != FS.usesPositionalArg()) {
+      // Cannot mix-and-match positional and non-positional arguments.
+      S.Diag(getLocationOfByte(CS.getStart()),
+             diag::warn_format_mix_positional_nonpositional_args)
+        << getSpecifierRange(startSpecifier, specifierLen);
+      return false;
+    }
   }
 
   // First check if the field width, precision, and conversion specifier
@@ -1653,18 +1655,20 @@
 
   const ConversionSpecifier &CS = FS.getConversionSpecifier();
 
-  // FIXME: Handle case where '%' and '*' don't consume an argument.
-  // This needs to be done for the printf case as well.
-  if (atFirstArg) {
-    atFirstArg = false;
-    usesPositionalArgs = FS.usesPositionalArg();
-  }
-  else if (usesPositionalArgs != FS.usesPositionalArg()) {
-    // Cannot mix-and-match positional and non-positional arguments.
-    S.Diag(getLocationOfByte(CS.getStart()),
-           diag::warn_format_mix_positional_nonpositional_args)
-      << getSpecifierRange(startSpecifier, specifierLen);
-    return false;
+  // Handle case where '%' and '*' don't consume an argument.  These shouldn't
+  // be used to decide if we are using positional arguments consistently.
+  if (FS.consumesDataArgument()) {
+    if (atFirstArg) {
+      atFirstArg = false;
+      usesPositionalArgs = FS.usesPositionalArg();
+    }
+    else if (usesPositionalArgs != FS.usesPositionalArg()) {
+      // Cannot mix-and-match positional and non-positional arguments.
+      S.Diag(getLocationOfByte(CS.getStart()),
+             diag::warn_format_mix_positional_nonpositional_args)
+        << getSpecifierRange(startSpecifier, specifierLen);
+      return false;
+    }
   }
   
   // Check if the field with is non-zero.

Modified: cfe/trunk/test/Sema/format-strings-scanf.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-scanf.c?rev=108757&r1=108756&r2=108757&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-scanf.c (original)
+++ cfe/trunk/test/Sema/format-strings-scanf.c Mon Jul 19 17:01:06 2010
@@ -16,4 +16,11 @@
   unsigned short s_x;
   scanf ("%" "hu" "\n", &s_x); // no-warning
   scanf("%y", i); // expected-warning{{invalid conversion specifier 'y'}}
+  scanf("%%"); // no-warning
+  scanf("%%%1$d", i); // no-warning
+  scanf("%1$d%%", i); // no-warning
+  scanf("%d", i, i); // expected-warning{{data argument not used by format string}}
+  scanf("%*d", i); // // expected-warning{{data argument not used by format string}}
+  scanf("%*d", i); // // expected-warning{{data argument not used by format string}}
+  scanf("%*d%1$d", i); // no-warning
 }

Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=108757&r1=108756&r2=108757&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Mon Jul 19 17:01:06 2010
@@ -239,6 +239,8 @@
   printf("%1$2.2d", (int) 2); // no-warning
   printf("%2$*1$.2d", (int) 2, (int) 3); // no-warning
   printf("%2$*8$d", (int) 2, (int) 3); // expected-warning{{specified field width is missing a matching 'int' argument}}
+  printf("%%%1$d", (int) 2); // no-warning
+  printf("%1$d%%", (int) 2); // no-warning
 }
 
 // PR 6697 - Handle format strings where the data argument is not adjacent to the format string