4 files changed, 44 insertions, 30 deletions

diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h
index f79300df9f..d76e4f7ad0 100644
--- a/include/clang/Analysis/Analyses/FormatString.h
+++ b/include/clang/Analysis/Analyses/FormatString.h
@@ -220,17 +220,14 @@ public:
   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 @@ public:
   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; }
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index f36a9ed938..72939e6e59 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1434,16 +1434,18 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
   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 @@ bool CheckScanfHandler::HandleScanfSpecifier(
 
   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.
diff --git a/test/Sema/format-strings-scanf.c b/test/Sema/format-strings-scanf.c
index 5d2a7a7a9b..68552b3e24 100644
--- a/test/Sema/format-strings-scanf.c
+++ b/test/Sema/format-strings-scanf.c
@@ -16,4 +16,11 @@ void test(const char *s, int *i) {
   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
 }
diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index c6dee6801e..080e4dbeb5 100644
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -239,6 +239,8 @@ void test_positional_arguments() {
   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