[llvm-branch-commits] [cfe-tag] r97681 - in /cfe/tags/Apple/clang/clang/tools/clang: include/clang/Analysis/Analyses/PrintfFormatString.h include/clang/Basic/DiagnosticSemaKinds.td lib/Analysis/PrintfFormatString.cpp lib/Basic/Version.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings.c

Ted Kremenek kremenek at apple.com
Wed Mar 3 15:42:15 PST 2010


Author: kremenek
Date: Wed Mar  3 17:42:15 2010
New Revision: 97681

URL: http://llvm.org/viewvc/llvm-project?rev=97681&view=rev
Log:
Merge in:

97248
97297
97298
97318
97620
97625

Fixes:

<rdar://problem/7440963>
<rdar://problem/7663667>
<rdar://problem/7697748>
<rdar://problem/7700339>


Modified:
    cfe/tags/Apple/clang/clang/tools/clang/include/clang/Analysis/Analyses/PrintfFormatString.h
    cfe/tags/Apple/clang/clang/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/tags/Apple/clang/clang/tools/clang/lib/Analysis/PrintfFormatString.cpp
    cfe/tags/Apple/clang/clang/tools/clang/lib/Basic/Version.cpp
    cfe/tags/Apple/clang/clang/tools/clang/lib/Sema/SemaChecking.cpp
    cfe/tags/Apple/clang/clang/tools/clang/test/Sema/format-strings.c

Modified: cfe/tags/Apple/clang/clang/tools/clang/include/clang/Analysis/Analyses/PrintfFormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/include/clang/Analysis/Analyses/PrintfFormatString.h?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/include/clang/Analysis/Analyses/PrintfFormatString.h (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/include/clang/Analysis/Analyses/PrintfFormatString.h Wed Mar  3 17:42:15 2010
@@ -75,11 +75,14 @@
    VoidPtrArg,    // 'p'
    OutIntPtrArg,  // 'n'
    PercentArg,    // '%'
-    // Objective-C specific specifiers.
+   // MacOS X unicode extensions.
+   CArg, // 'C'
+   UnicodeStrArg, // 'S'
+   // Objective-C specific specifiers.
    ObjCObjArg,    // '@'
-    // GlibC specific specifiers.
+   // GlibC specific specifiers.
    PrintErrno,    // 'm'
-    // Specifier ranges.
+   // Specifier ranges.
    IntArgBeg = dArg,
    IntArgEnd = iArg,
    UIntArgBeg = oArg,
@@ -147,20 +150,27 @@
 
 class OptionalAmount {
 public:
-  enum HowSpecified { NotSpecified, Constant, Arg };
+  enum HowSpecified { NotSpecified, Constant, Arg, Invalid };
 
-  OptionalAmount(HowSpecified h, const char *st)
-    : start(st), hs(h), amt(0) {}
+  OptionalAmount(HowSpecified h, unsigned i, const char *st)
+    : start(st), hs(h), amt(i) {}
 
-  OptionalAmount()
-    : start(0), hs(NotSpecified), amt(0) {}
+  OptionalAmount(bool b = true)
+    : start(0), hs(b ? NotSpecified : Invalid), amt(0) {}
 
-  OptionalAmount(unsigned i, const char *st)
-    : start(st), hs(Constant), amt(i) {}
+  bool isInvalid() const {
+    return hs == Invalid;
+  }
 
   HowSpecified getHowSpecified() const { return hs; }
+
   bool hasDataArgument() const { return hs == Arg; }
 
+  unsigned getArgIndex() const {
+    assert(hasDataArgument());
+    return amt;
+  }
+
   unsigned getConstantAmount() const {
     assert(hs == Constant);
     return amt;
@@ -185,14 +195,19 @@
   unsigned HasSpacePrefix : 1;
   unsigned HasAlternativeForm : 1;
   unsigned HasLeadingZeroes : 1;
-  unsigned flags : 5;
+  /// Positional arguments, an IEEE extension:
+  ///  IEEE Std 1003.1, 2004 Edition
+  ///  http://www.opengroup.org/onlinepubs/009695399/functions/printf.html
+  unsigned UsesPositionalArg : 1;
+  unsigned argIndex;
   ConversionSpecifier CS;
   OptionalAmount FieldWidth;
   OptionalAmount Precision;
 public:
   FormatSpecifier() : LM(None),
     IsLeftJustified(0), HasPlusPrefix(0), HasSpacePrefix(0),
-    HasAlternativeForm(0), HasLeadingZeroes(0) {}
+    HasAlternativeForm(0), HasLeadingZeroes(0), UsesPositionalArg(0),
+    argIndex(0) {}
 
   static FormatSpecifier Parse(const char *beg, const char *end);
 
@@ -208,6 +223,17 @@
   void setHasSpacePrefix() { HasSpacePrefix = 1; }
   void setHasAlternativeForm() { HasAlternativeForm = 1; }
   void setHasLeadingZeros() { HasLeadingZeroes = 1; }
+  void setUsesPositionalArg() { UsesPositionalArg = 1; }
+
+  void setArgIndex(unsigned i) {
+    assert(CS.consumesDataArgument());
+    argIndex = i;
+  }
+
+  unsigned getArgIndex() const {
+    assert(CS.consumesDataArgument());
+    return argIndex;
+  }
 
   // Methods for querying the format specifier.
 
@@ -247,8 +273,11 @@
   bool hasAlternativeForm() const { return (bool) HasAlternativeForm; }
   bool hasLeadingZeros() const { return (bool) HasLeadingZeroes; }
   bool hasSpacePrefix() const { return (bool) HasSpacePrefix; }
+  bool usesPositionalArg() const { return (bool) UsesPositionalArg; }
 };
 
+enum PositionContext { FieldWidthPos = 0, PrecisionPos = 1 };
+
 class FormatStringHandler {
 public:
   FormatStringHandler() {}
@@ -259,10 +288,15 @@
 
   virtual void HandleNullChar(const char *nullCharacter) {}
 
-  virtual void
+  virtual void HandleInvalidPosition(const char *startPos, unsigned posLen,
+                                     PositionContext p) {}
+
+  virtual void HandleZeroPosition(const char *startPos, unsigned posLen) {}
+
+  virtual bool
     HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
                                      const char *startSpecifier,
-                                     unsigned specifierLen) {}
+                                     unsigned specifierLen) { return true; }
 
   virtual bool HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
                                      const char *startSpecifier,

Modified: cfe/tags/Apple/clang/clang/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar  3 17:42:15 2010
@@ -2492,8 +2492,8 @@
   InGroup<FormatSecurity>;
 def warn_printf_insufficient_data_args : Warning<
   "more '%%' conversions than data arguments">, InGroup<Format>;
-def warn_printf_too_many_data_args : Warning<
-  "more data arguments than format specifiers">, InGroup<FormatExtraArgs>;
+def warn_printf_data_arg_not_used : Warning<
+  "data argument not used by format string">, InGroup<FormatExtraArgs>;
 def warn_printf_invalid_conversion : Warning<
   "invalid conversion specifier '%0'">, InGroup<Format>;
 def warn_printf_incomplete_specifier : Warning<
@@ -2503,6 +2503,15 @@
 def warn_printf_conversion_argument_type_mismatch : Warning<
   "conversion specifies type %0 but the argument has type %1">,
   InGroup<Format>;
+def warn_printf_zero_positional_specifier : Warning<
+  "position arguments in format strings start counting at 1 (not 0)">,
+  InGroup<Format>;
+def warn_printf_invalid_positional_specifier : Warning<
+  "invalid position specified for %select{field width|field precision}0">,
+  InGroup<Format>;
+def warn_printf_mix_positional_nonpositional_args : Warning<
+  "cannot mix positional and non-positional arguments in format string">,
+  InGroup<Format>;
 def warn_null_arg : Warning<
   "null passed to a callee which requires a non-null argument">,
   InGroup<NonNull>;
@@ -2512,15 +2521,10 @@
   "format string should not be a wide string">, InGroup<Format>;
 def warn_printf_format_string_contains_null_char : Warning<
   "format string contains '\\0' within the string body">, InGroup<Format>;
-def warn_printf_asterisk_width_missing_arg : Warning<
-  "'*' specified field width is missing a matching 'int' argument">;
-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 %0, but argument has type %1">,
-  InGroup<Format>;
-def warn_printf_asterisk_precision_wrong_type : Warning<
-  "field precision should have type %0, but argument has type %1">,
+def warn_printf_asterisk_missing_arg : Warning<
+  "'%select{*|.*}0' specified field %select{width|precision}0 is missing a matching 'int' argument">;
+def warn_printf_asterisk_wrong_type : Warning<
+  "field %select{width|precision}0 should have type %1, but argument has type %2">,
   InGroup<Format>;
 def warn_printf_nonsensical_precision: Warning<
   "precision used in '%0' conversion specifier (where it has no meaning)">,
@@ -2528,7 +2532,7 @@
 def warn_printf_nonsensical_flag: Warning<
   "flag '%0' results in undefined behavior in '%1' conversion specifier">,
   InGroup<Format>;
-  
+
 // CHECK: returning address/reference of stack memory
 def warn_ret_stack_addr : Warning<
   "address of stack memory associated with local variable %0 returned">;

Modified: cfe/tags/Apple/clang/clang/tools/clang/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/lib/Analysis/PrintfFormatString.cpp?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/lib/Analysis/PrintfFormatString.cpp Wed Mar  3 17:42:15 2010
@@ -15,10 +15,12 @@
 #include "clang/Analysis/Analyses/PrintfFormatString.h"
 #include "clang/AST/ASTContext.h"
 
-using clang::analyze_printf::FormatSpecifier;
-using clang::analyze_printf::OptionalAmount;
 using clang::analyze_printf::ArgTypeResult;
+using clang::analyze_printf::FormatSpecifier;
 using clang::analyze_printf::FormatStringHandler;
+using clang::analyze_printf::OptionalAmount;
+using clang::analyze_printf::PositionContext;
+
 using namespace clang;
 
 namespace {
@@ -66,24 +68,19 @@
   const char *I = Beg;
   UpdateOnReturn <const char*> UpdateBeg(Beg, I);
 
-  bool foundDigits = false;
   unsigned accumulator = 0;
+  bool hasDigits = false;
 
   for ( ; I != E; ++I) {
     char c = *I;
     if (c >= '0' && c <= '9') {
-      foundDigits = true;
+      hasDigits = true;
       accumulator += (accumulator * 10) + (c - '0');
       continue;
     }
 
-    if (foundDigits)
-      return OptionalAmount(accumulator, Beg);
-
-    if (c == '*') {
-      ++I;
-      return OptionalAmount(OptionalAmount::Arg, Beg);
-    }
+    if (hasDigits)
+      return OptionalAmount(OptionalAmount::Constant, accumulator, Beg);
 
     break;
   }
@@ -91,9 +88,129 @@
   return OptionalAmount();
 }
 
+static OptionalAmount ParseNonPositionAmount(const char *&Beg, const char *E,
+                                             unsigned &argIndex) {
+  if (*Beg == '*') {
+    ++Beg;
+    return OptionalAmount(OptionalAmount::Arg, argIndex++, Beg);
+  }
+
+  return ParseAmount(Beg, E);
+}
+
+static OptionalAmount ParsePositionAmount(FormatStringHandler &H,
+                                          const char *Start,
+                                          const char *&Beg, const char *E,
+                                          PositionContext p) {
+  if (*Beg == '*') {
+    const char *I = Beg + 1;
+    const OptionalAmount &Amt = ParseAmount(I, E);
+
+    if (Amt.getHowSpecified() == OptionalAmount::NotSpecified) {
+      H.HandleInvalidPosition(Beg, I - Beg, p);
+      return OptionalAmount(false);
+    }
+
+    if (I== E) {
+      // No more characters left?
+      H.HandleIncompleteFormatSpecifier(Start, E - Start);
+      return OptionalAmount(false);
+    }
+
+    assert(Amt.getHowSpecified() == OptionalAmount::Constant);
+
+    if (*I == '$') {
+      // Special case: '*0$', since this is an easy mistake.
+      if (Amt.getConstantAmount() == 0) {
+        H.HandleZeroPosition(Beg, I - Beg + 1);
+        return OptionalAmount(false);
+      }
+
+      const char *Tmp = Beg;
+      Beg = ++I;
+
+      return OptionalAmount(OptionalAmount::Arg, Amt.getConstantAmount() - 1,
+                            Tmp);
+    }
+
+    H.HandleInvalidPosition(Beg, I - Beg, p);
+    return OptionalAmount(false);
+  }
+
+  return ParseAmount(Beg, E);
+}
+
+static bool ParsePrecision(FormatStringHandler &H, FormatSpecifier &FS,
+                           const char *Start, const char *&Beg, const char *E,
+                           unsigned *argIndex) {
+  if (argIndex) {
+    FS.setPrecision(ParseNonPositionAmount(Beg, E, *argIndex));
+  }
+  else {
+    const OptionalAmount Amt = ParsePositionAmount(H, Start, Beg, E,
+                                                  analyze_printf::PrecisionPos);
+    if (Amt.isInvalid())
+      return true;
+    FS.setPrecision(Amt);
+  }
+  return false;
+}
+
+static bool ParseFieldWidth(FormatStringHandler &H, FormatSpecifier &FS,
+                            const char *Start, const char *&Beg, const char *E,
+                            unsigned *argIndex) {
+  // FIXME: Support negative field widths.
+  if (argIndex) {
+    FS.setFieldWidth(ParseNonPositionAmount(Beg, E, *argIndex));
+  }
+  else {
+    const OptionalAmount Amt = ParsePositionAmount(H, Start, Beg, E,
+                                                 analyze_printf::FieldWidthPos);
+    if (Amt.isInvalid())
+      return true;
+    FS.setFieldWidth(Amt);
+  }
+  return false;
+}
+
+
+static bool ParseArgPosition(FormatStringHandler &H,
+                             FormatSpecifier &FS, const char *Start,
+                             const char *&Beg, const char *E) {
+
+  using namespace clang::analyze_printf;
+  const char *I = Beg;
+
+  const OptionalAmount &Amt = ParseAmount(I, E);
+
+  if (I == E) {
+    // No more characters left?
+    H.HandleIncompleteFormatSpecifier(Start, E - Start);
+    return true;
+  }
+
+  if (Amt.getHowSpecified() == OptionalAmount::Constant && *(I++) == '$') {
+    // Special case: '%0$', since this is an easy mistake.
+    if (Amt.getConstantAmount() == 0) {
+      H.HandleZeroPosition(Start, I - Start);
+      return true;
+    }
+
+    FS.setArgIndex(Amt.getConstantAmount() - 1);
+    FS.setUsesPositionalArg();
+    // Update the caller's pointer if we decided to consume
+    // these characters.
+    Beg = I;
+    return false;
+  }
+
+  return false;
+}
+
 static FormatSpecifierResult ParseFormatSpecifier(FormatStringHandler &H,
                                                   const char *&Beg,
-                                                  const char *E) {
+                                                  const char *E,
+                                                  unsigned &argIndex) {
 
   using namespace clang::analyze_printf;
 
@@ -126,6 +243,14 @@
   }
 
   FormatSpecifier FS;
+  if (ParseArgPosition(H, FS, Start, I, E))
+    return true;
+
+  if (I == E) {
+    // No more characters left?
+    H.HandleIncompleteFormatSpecifier(Start, E - Start);
+    return true;
+  }
 
   // Look for flags (if any).
   bool hasMore = true;
@@ -149,7 +274,9 @@
   }
 
   // Look for the field width (if any).
-  FS.setFieldWidth(ParseAmount(I, E));
+  if (ParseFieldWidth(H, FS, Start, I, E,
+                      FS.usesPositionalArg() ? 0 : &argIndex))
+    return true;
 
   if (I == E) {
     // No more characters left?
@@ -165,7 +292,9 @@
       return true;
     }
 
-    FS.setPrecision(ParseAmount(I, E));
+    if (ParsePrecision(H, FS, Start, I, E,
+                       FS.usesPositionalArg() ? 0 : &argIndex))
+      return true;
 
     if (I == E) {
       // No more characters left?
@@ -214,44 +343,53 @@
     default:
       break;
     // C99: 7.19.6.1 (section 8).
-    case 'd': k = ConversionSpecifier::dArg; break;
-    case 'i': k = ConversionSpecifier::iArg; break;
-    case 'o': k = ConversionSpecifier::oArg; break;
-    case 'u': k = ConversionSpecifier::uArg; break;
-    case 'x': k = ConversionSpecifier::xArg; break;
-    case 'X': k = ConversionSpecifier::XArg; break;
-    case 'f': k = ConversionSpecifier::fArg; break;
-    case 'F': k = ConversionSpecifier::FArg; break;
-    case 'e': k = ConversionSpecifier::eArg; break;
+    case '%': k = ConversionSpecifier::PercentArg;   break;
+    case 'A': k = ConversionSpecifier::AArg; break;
     case 'E': k = ConversionSpecifier::EArg; break;
-    case 'g': k = ConversionSpecifier::gArg; break;
+    case 'F': k = ConversionSpecifier::FArg; break;
     case 'G': k = ConversionSpecifier::GArg; break;
+    case 'X': k = ConversionSpecifier::XArg; break;
     case 'a': k = ConversionSpecifier::aArg; break;
-    case 'A': k = ConversionSpecifier::AArg; break;
     case 'c': k = ConversionSpecifier::IntAsCharArg; break;
-    case 's': k = ConversionSpecifier::CStrArg;      break;
-    case 'p': k = ConversionSpecifier::VoidPtrArg;   break;
+    case 'd': k = ConversionSpecifier::dArg; break;
+    case 'e': k = ConversionSpecifier::eArg; break;
+    case 'f': k = ConversionSpecifier::fArg; break;
+    case 'g': k = ConversionSpecifier::gArg; break;
+    case 'i': k = ConversionSpecifier::iArg; break;
     case 'n': k = ConversionSpecifier::OutIntPtrArg; break;
-    case '%': k = ConversionSpecifier::PercentArg;   break;
+    case 'o': k = ConversionSpecifier::oArg; break;
+    case 'p': k = ConversionSpecifier::VoidPtrArg;   break;
+    case 's': k = ConversionSpecifier::CStrArg;      break;
+    case 'u': k = ConversionSpecifier::uArg; break;
+    case 'x': k = ConversionSpecifier::xArg; break;
+    // Mac OS X (unicode) specific
+    case 'C': k = ConversionSpecifier::CArg; break;
+    case 'S': k = ConversionSpecifier::UnicodeStrArg; break;
     // Objective-C.
     case '@': k = ConversionSpecifier::ObjCObjArg; break;
     // Glibc specific.
     case 'm': k = ConversionSpecifier::PrintErrno; break;
   }
-  FS.setConversionSpecifier(ConversionSpecifier(conversionPosition, k));
+  ConversionSpecifier CS(conversionPosition, k);
+  FS.setConversionSpecifier(CS);
+  if (CS.consumesDataArgument() && !FS.usesPositionalArg())
+    FS.setArgIndex(argIndex++);
 
   if (k == ConversionSpecifier::InvalidSpecifier) {
-    H.HandleInvalidConversionSpecifier(FS, Beg, I - Beg);
-    return false; // Keep processing format specifiers.
+    // Assume the conversion takes one argument.
+    return !H.HandleInvalidConversionSpecifier(FS, Beg, I - Beg);
   }
   return FormatSpecifierResult(Start, FS);
 }
 
 bool clang::analyze_printf::ParseFormatString(FormatStringHandler &H,
                        const char *I, const char *E) {
+
+  unsigned argIndex = 0;
+
   // Keep looking for a format specifier until we have exhausted the string.
   while (I != E) {
-    const FormatSpecifierResult &FSR = ParseFormatSpecifier(H, I, E);
+    const FormatSpecifierResult &FSR = ParseFormatSpecifier(H, I, E, argIndex);
     // Did a fail-stop error of any kind occur when parsing the specifier?
     // If so, don't do any more processing.
     if (FSR.shouldStop())
@@ -345,8 +483,10 @@
     if (!PT)
       return false;
 
-    QualType pointeeTy = PT->getPointeeType();
-    return pointeeTy == C.WCharTy;
+    QualType pointeeTy =
+      C.getCanonicalType(PT->getPointeeType()).getUnqualifiedType();
+
+    return pointeeTy == C.getWCharType();
   }
 
   return false;
@@ -359,7 +499,7 @@
   if (K == CStrTy)
     return C.getPointerType(C.CharTy);
   if (K == WCStrTy)
-    return C.getPointerType(C.WCharTy);
+    return C.getPointerType(C.getWCharType());
   if (K == ObjCPointerTy)
     return C.ObjCBuiltinIdTy;
 
@@ -426,9 +566,17 @@
     return Ctx.DoubleTy;
   }
 
-  if (CS.getKind() == ConversionSpecifier::CStrArg)
-    return ArgTypeResult(LM == AsWideChar ? ArgTypeResult::WCStrTy
-                                          : ArgTypeResult::CStrTy);
+  switch (CS.getKind()) {
+    case ConversionSpecifier::CStrArg:
+      return ArgTypeResult(LM == AsWideChar ? ArgTypeResult::WCStrTy                                            : ArgTypeResult::CStrTy);
+    case ConversionSpecifier::UnicodeStrArg:
+      // FIXME: This appears to be Mac OS X specific.
+      return ArgTypeResult::WCStrTy;
+    case ConversionSpecifier::CArg:
+      return Ctx.WCharTy;
+    default:
+      break;
+  }
 
   // FIXME: Handle other cases.
   return ArgTypeResult();

Modified: cfe/tags/Apple/clang/clang/tools/clang/lib/Basic/Version.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/lib/Basic/Version.cpp?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/lib/Basic/Version.cpp (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/lib/Basic/Version.cpp Wed Mar  3 17:42:15 2010
@@ -40,15 +40,15 @@
 }
 
 std::string getClangRevision() {
-#ifndef SVN_REVISION
-  // Subversion was not available at build time?
-  return "";
-#else
-  std::string revision;
-  llvm::raw_string_ostream OS(revision);
-  OS << strtol(SVN_REVISION, 0, 10);
-  return revision;
+#ifdef SVN_REVISION
+  if (SVN_REVISION[0] != '\0') {
+    std::string revision;
+    llvm::raw_string_ostream OS(revision);
+    OS << strtol(SVN_REVISION, 0, 10);
+    return revision;
+  }
 #endif
+  return "";
 }
 
 std::string getClangFullRepositoryVersion() {

Modified: cfe/tags/Apple/clang/clang/tools/clang/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/lib/Sema/SemaChecking.cpp?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/lib/Sema/SemaChecking.cpp (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/lib/Sema/SemaChecking.cpp Wed Mar  3 17:42:15 2010
@@ -1043,13 +1043,15 @@
   Sema &S;
   const StringLiteral *FExpr;
   const Expr *OrigFormatExpr;
-  unsigned NumConversions;
   const unsigned NumDataArgs;
   const bool IsObjCLiteral;
   const char *Beg; // Start of format string.
   const bool HasVAListArg;
   const CallExpr *TheCall;
   unsigned FormatIdx;
+  llvm::BitVector CoveredArgs;
+  bool usesPositionalArgs;
+  bool atFirstArg;
 public:
   CheckPrintfHandler(Sema &s, const StringLiteral *fexpr,
                      const Expr *origFormatExpr,
@@ -1057,21 +1059,31 @@
                      const char *beg, bool hasVAListArg,
                      const CallExpr *theCall, unsigned formatIdx)
     : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr),
-      NumConversions(0), NumDataArgs(numDataArgs),
+      NumDataArgs(numDataArgs),
       IsObjCLiteral(isObjCLiteral), Beg(beg),
       HasVAListArg(hasVAListArg),
-      TheCall(theCall), FormatIdx(formatIdx) {}
+      TheCall(theCall), FormatIdx(formatIdx),
+      usesPositionalArgs(false), atFirstArg(true) {
+        CoveredArgs.resize(numDataArgs);
+        CoveredArgs.reset();
+      }
 
   void DoneProcessing();
 
   void HandleIncompleteFormatSpecifier(const char *startSpecifier,
                                        unsigned specifierLen);
 
-  void
+  bool
   HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
                                    const char *startSpecifier,
                                    unsigned specifierLen);
 
+  virtual void HandleInvalidPosition(const char *startSpecifier,
+                                     unsigned specifierLen,
+                                     analyze_printf::PositionContext p);
+
+  virtual void HandleZeroPosition(const char *startPos, unsigned posLen);
+
   void HandleNullChar(const char *nullCharacter);
 
   bool HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1083,9 +1095,8 @@
                                       unsigned specifierLen);
   SourceLocation getLocationOfByte(const char *x);
 
-  bool HandleAmount(const analyze_printf::OptionalAmount &Amt,
-                    unsigned MissingArgDiag, unsigned BadTypeDiag,
-          const char *startSpecifier, unsigned specifierLen);
+  bool HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned k,
+                    const char *startSpecifier, unsigned specifierLen);
   void HandleFlags(const analyze_printf::FormatSpecifier &FS,
                    llvm::StringRef flag, llvm::StringRef cspec,
                    const char *startSpecifier, unsigned specifierLen);
@@ -1116,18 +1127,50 @@
     << getFormatSpecifierRange(startSpecifier, specifierLen);
 }
 
-void CheckPrintfHandler::
+void
+CheckPrintfHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
+                                          analyze_printf::PositionContext p) {
+  SourceLocation Loc = getLocationOfByte(startPos);
+  S.Diag(Loc, diag::warn_printf_invalid_positional_specifier)
+    << (unsigned) p << getFormatSpecifierRange(startPos, posLen);
+}
+
+void CheckPrintfHandler::HandleZeroPosition(const char *startPos,
+                                            unsigned posLen) {
+  SourceLocation Loc = getLocationOfByte(startPos);
+  S.Diag(Loc, diag::warn_printf_zero_positional_specifier)
+    << getFormatSpecifierRange(startPos, posLen);
+}
+
+bool CheckPrintfHandler::
 HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
                                  const char *startSpecifier,
                                  unsigned specifierLen) {
 
-  ++NumConversions;
+  unsigned argIndex = FS.getArgIndex();
+  bool keepGoing = true;
+  if (argIndex < NumDataArgs) {
+    // Consider the argument coverered, even though the specifier doesn't
+    // make sense.
+    CoveredArgs.set(argIndex);
+  }
+  else {
+    // If argIndex exceeds the number of data arguments we
+    // don't issue a warning because that is just a cascade of warnings (and
+    // they may have intended '%%' anyway). We don't want to continue processing
+    // the format string after this point, however, as we will like just get
+    // gibberish when trying to match arguments.
+    keepGoing = false;
+  }
+
   const analyze_printf::ConversionSpecifier &CS =
     FS.getConversionSpecifier();
   SourceLocation Loc = getLocationOfByte(CS.getStart());
   S.Diag(Loc, diag::warn_printf_invalid_conversion)
       << llvm::StringRef(CS.getStart(), CS.getLength())
       << getFormatSpecifierRange(startSpecifier, specifierLen);
+
+  return keepGoing;
 }
 
 void CheckPrintfHandler::HandleNullChar(const char *nullCharacter) {
@@ -1138,7 +1181,7 @@
 }
 
 const Expr *CheckPrintfHandler::getDataArg(unsigned i) const {
-  return TheCall->getArg(FormatIdx + i);
+  return TheCall->getArg(FormatIdx + i + 1);
 }
 
 
@@ -1155,17 +1198,16 @@
 
 bool
 CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt,
-                                 unsigned MissingArgDiag,
-                                 unsigned BadTypeDiag,
-                                 const char *startSpecifier,
+                                 unsigned k, const char *startSpecifier,
                                  unsigned specifierLen) {
 
   if (Amt.hasDataArgument()) {
-    ++NumConversions;
     if (!HasVAListArg) {
-      if (NumConversions > NumDataArgs) {
-        S.Diag(getLocationOfByte(Amt.getStart()), MissingArgDiag)
-          << getFormatSpecifierRange(startSpecifier, specifierLen);
+      unsigned argIndex = Amt.getArgIndex();
+      if (argIndex >= NumDataArgs) {
+        S.Diag(getLocationOfByte(Amt.getStart()),
+               diag::warn_printf_asterisk_missing_arg)
+          << k << getFormatSpecifierRange(startSpecifier, specifierLen);
         // Don't do any more checking.  We will just emit
         // spurious errors.
         return false;
@@ -1175,14 +1217,17 @@
       // Although not in conformance with C99, we also allow the argument to be
       // an 'unsigned int' as that is a reasonably safe case.  GCC also
       // doesn't emit a warning for that case.
-      const Expr *Arg = getDataArg(NumConversions);
+      CoveredArgs.set(argIndex);
+      const Expr *Arg = getDataArg(argIndex);
       QualType T = Arg->getType();
 
       const analyze_printf::ArgTypeResult &ATR = Amt.getArgType(S.Context);
       assert(ATR.isValid());
 
       if (!ATR.matchesType(S.Context, T)) {
-        S.Diag(getLocationOfByte(Amt.getStart()), BadTypeDiag)
+        S.Diag(getLocationOfByte(Amt.getStart()),
+               diag::warn_printf_asterisk_wrong_type)
+          << k
           << ATR.getRepresentativeType(S.Context) << T
           << getFormatSpecifierRange(startSpecifier, specifierLen)
           << Arg->getSourceRange();
@@ -1201,32 +1246,31 @@
                                           const char *startSpecifier,
                                           unsigned specifierLen) {
 
-  using namespace analyze_printf;
+  using namespace analyze_printf;  
   const ConversionSpecifier &CS = FS.getConversionSpecifier();
 
-  // First check if the field width, precision, and conversion specifier
-  // have matching data arguments.
-  if (!HandleAmount(FS.getFieldWidth(),
-                    diag::warn_printf_asterisk_width_missing_arg,
-                    diag::warn_printf_asterisk_width_wrong_type,
-          startSpecifier, specifierLen)) {
+  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_printf_mix_positional_nonpositional_args)
+      << getFormatSpecifierRange(startSpecifier, specifierLen);
     return false;
   }
 
-  if (!HandleAmount(FS.getPrecision(),
-                    diag::warn_printf_asterisk_precision_missing_arg,
-                    diag::warn_printf_asterisk_precision_wrong_type,
-          startSpecifier, specifierLen)) {
+  // First check if the field width, precision, and conversion specifier
+  // have matching data arguments.
+  if (!HandleAmount(FS.getFieldWidth(), /* field width */ 0,
+                    startSpecifier, specifierLen)) {
     return false;
   }
 
-  // Check for using an Objective-C specific conversion specifier
-  // in a non-ObjC literal.
-  if (!IsObjCLiteral && CS.isObjCArg()) {
-    HandleInvalidConversionSpecifier(FS, startSpecifier, specifierLen);
-
-    // Continue checking the other format specifiers.
-    return true;
+  if (!HandleAmount(FS.getPrecision(), /* precision */ 1,
+                    startSpecifier, specifierLen)) {
+    return false;
   }
 
   if (!CS.consumesDataArgument()) {
@@ -1235,7 +1279,20 @@
     return true;
   }
 
-  ++NumConversions;
+  // Consume the argument.
+  unsigned argIndex = FS.getArgIndex();
+  if (argIndex < NumDataArgs) {
+    // The check to see if the argIndex is valid will come later.
+    // We set the bit here because we may exit early from this
+    // function if we encounter some other error.
+    CoveredArgs.set(argIndex);
+  }
+
+  // Check for using an Objective-C specific conversion specifier
+  // in a non-ObjC literal.
+  if (!IsObjCLiteral && CS.isObjCArg()) {
+    return HandleInvalidConversionSpecifier(FS, startSpecifier, specifierLen);
+  }
 
   // Are we using '%n'?  Issue a warning about this being
   // a possible security issue.
@@ -1269,7 +1326,7 @@
   if (HasVAListArg)
     return true;
 
-  if (NumConversions > NumDataArgs) {
+  if (argIndex >= NumDataArgs) {
     S.Diag(getLocationOfByte(CS.getStart()),
            diag::warn_printf_insufficient_data_args)
       << getFormatSpecifierRange(startSpecifier, specifierLen);
@@ -1279,7 +1336,7 @@
 
   // Now type check the data expression that matches the
   // format specifier.
-  const Expr *Ex = getDataArg(NumConversions);
+  const Expr *Ex = getDataArg(argIndex);
   const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context);
   if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
     // Check if we didn't match because of an implicit cast from a 'char'
@@ -1303,10 +1360,17 @@
 void CheckPrintfHandler::DoneProcessing() {
   // Does the number of data arguments exceed the number of
   // format conversions in the format string?
-  if (!HasVAListArg && NumConversions < NumDataArgs)
-    S.Diag(getDataArg(NumConversions+1)->getLocStart(),
-           diag::warn_printf_too_many_data_args)
-      << getFormatStringRange();
+  if (!HasVAListArg) {
+    // Find any arguments that weren't covered.
+    CoveredArgs.flip();
+    signed notCoveredArg = CoveredArgs.find_first();
+    if (notCoveredArg >= 0) {
+      assert((unsigned)notCoveredArg < NumDataArgs);
+      S.Diag(getDataArg((unsigned) notCoveredArg)->getLocStart(),
+             diag::warn_printf_data_arg_not_used)
+        << getFormatStringRange();
+    }
+  }
 }
 
 void Sema::CheckPrintfString(const StringLiteral *FExpr,

Modified: cfe/tags/Apple/clang/clang/tools/clang/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/tags/Apple/clang/clang/tools/clang/test/Sema/format-strings.c?rev=97681&r1=97680&r2=97681&view=diff
==============================================================================
--- cfe/tags/Apple/clang/clang/tools/clang/test/Sema/format-strings.c (original)
+++ cfe/tags/Apple/clang/clang/tools/clang/test/Sema/format-strings.c Wed Mar  3 17:42:15 2010
@@ -55,7 +55,7 @@
   printf(i == 1 ? "yes" : "no"); // no-warning
   printf(i == 0 ? (i == 1 ? "yes" : "no") : "dont know"); // no-warning
   printf(i == 0 ? (i == 1 ? s : "no") : "dont know"); // expected-warning{{format string is not a string literal}}
-  printf("yes" ?: "no %d", 1); // expected-warning{{more data arguments than format specifiers}}
+  printf("yes" ?: "no %d", 1); // expected-warning{{data argument not used by format string}}
 }
 
 void check_writeback_specifier()
@@ -145,6 +145,7 @@
 }
 
 void test10(int x, float f, int i, long long lli) {
+  printf("%s"); // expected-warning{{more '%' conversions than data arguments}}
   printf("%@", 12); // expected-warning{{invalid conversion specifier '@'}}
   printf("\0"); // expected-warning{{format string contains '\0' within the string body}}
   printf("xs\0"); // expected-warning{{format string contains '\0' within the string body}}
@@ -155,7 +156,7 @@
   printf("%**\n"); // expected-warning{{invalid conversion specifier '*'}}
   printf("%n", &i); // expected-warning{{use of '%n' in format string discouraged (potentially insecure)}}
   printf("%d%d\n", x); // expected-warning{{more '%' conversions than data arguments}}
-  printf("%d\n", x, x); // expected-warning{{more data arguments than format specifiers}}
+  printf("%d\n", x, x); // expected-warning{{data argument not used by format string}}
   printf("%W%d%Z\n", x, x, x); // expected-warning{{invalid conversion specifier 'W'}} expected-warning{{invalid conversion specifier 'Z'}}
   printf("%"); // expected-warning{{incomplete format specifier}}
   printf("%.d", x); // no-warning
@@ -169,6 +170,8 @@
   printf("%d", (long long) 10); // expected-warning{{conversion specifies type 'int' but the argument has type 'long long'}}
   printf("%Lf\n", (long double) 1.0); // no-warning
   printf("%f\n", (long double) 1.0); // expected-warning{{conversion specifies type 'double' but the argument has type 'long double'}}
+  // The man page says that a zero precision is okay.
+  printf("%.0Lf", (long double) 1.0); // no-warning
 } 
 
 void test11(void *p, char *s) {
@@ -204,3 +207,34 @@
 // <rdar://problem/7595366>
 typedef enum { A } int_t;
 void f0(int_t x) { printf("%d\n", x); }
+
+// Unicode test cases.  These are possibly specific to Mac OS X.  If so, they should
+// eventually be moved into a separate test.
+typedef __WCHAR_TYPE__ wchar_t;
+
+void test_unicode_conversions(wchar_t *s) {
+  printf("%S", s); // no-warning
+  printf("%s", s); // expected-warning{{conversion specifies type 'char *' but the argument has type 'wchar_t *'}}
+  printf("%C", s[0]); // no-warning
+  printf("%c", s[0]);
+  // FIXME: This test reports inconsistent results. On Windows, '%C' expects
+  // 'unsigned short'.
+  // printf("%C", 10);
+  // FIXME: we report the expected type as 'int*' instead of 'wchar_t*'
+  printf("%S", "hello"); // expected-warning{{but the argument has type 'char *'}}
+}
+
+// Mac OS X supports positional arguments in format strings.
+// This is an IEEE extension (IEEE Std 1003.1).
+// FIXME: This is probably not portable everywhere.
+void test_positional_arguments() {
+  printf("%0$", (int)2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+  printf("%1$*0$d", (int) 2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+  printf("%1$d", (int) 2); // no-warning
+  printf("%1$d", (int) 2, 2); // expected-warning{{data argument not used by format string}}
+  printf("%1$d%1$f", (int) 2); // expected-warning{{conversion specifies type 'double' but the argument has type 'int'}}
+  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}}
+}
+





More information about the llvm-branch-commits mailing list