Unrevert r158887, reverted in r158949, along with a fix for the bug which
resulted in it being reverted. A test for that bug was added in r158950.

Original comment:

If an object (such as a std::string) with an appropriate c_str() member function
is passed to a variadic function in a position where a format string indicates
that c_str()'s return type is desired, provide a note suggesting that the user
may have intended to call the c_str() member.

Factor the non-POD-vararg checking out of DefaultVariadicArgumentPromotion and
move it to SemaChecking in order to facilitate this. Factor the call checking
out of function call checking and block call checking, and extend it to cover
constructor calls too.

Patch by Sam Panzer!


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159159 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index b8ed7b5..ef7dc88 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -16,6 +16,7 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Initialization.h"
+#include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Analysis/Analyses/FormatString.h"
 #include "clang/AST/ASTContext.h"
@@ -418,34 +419,91 @@
   return false;
 }
 
-/// CheckFunctionCall - Check a direct function call for various correctness
-/// and safety properties not strictly enforced by the C type system.
-bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
-  // Get the IdentifierInfo* for the called function.
-  IdentifierInfo *FnInfo = FDecl->getIdentifier();
+/// Given a FunctionDecl's FormatAttr, attempts to populate the FomatStringInfo
+/// parameter with the FormatAttr's correct format_idx and firstDataArg.
+/// Returns true when the format fits the function and the FormatStringInfo has
+/// been populated.
+bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
+                               FormatStringInfo *FSI) {
+  FSI->HasVAListArg = Format->getFirstArg() == 0;
+  FSI->FormatIdx = Format->getFormatIdx() - 1;
+  FSI->FirstDataArg = FSI->HasVAListArg ? 0 : Format->getFirstArg() - 1;
 
-  // None of the checks below are needed for functions that don't have
-  // simple names (e.g., C++ conversion functions).
-  if (!FnInfo)
-    return false;
+  // The way the format attribute works in GCC, the implicit this argument
+  // of member functions is counted. However, it doesn't appear in our own
+  // lists, so decrement format_idx in that case.
+  if (IsCXXMember) {
+    if(FSI->FormatIdx == 0)
+      return false;
+    --FSI->FormatIdx;
+    if (FSI->FirstDataArg != 0)
+      --FSI->FirstDataArg;
+  }
+  return true;
+}
 
+/// Handles the checks for format strings, non-POD arguments to vararg
+/// functions, and NULL arguments passed to non-NULL parameters.
+void Sema::checkCall(NamedDecl *FDecl, Expr **Args,
+                     unsigned NumArgs,
+                     unsigned NumProtoArgs,
+                     bool IsMemberFunction,
+                     SourceLocation Loc,
+                     SourceRange Range,
+                     VariadicCallType CallType) {
   // FIXME: This mechanism should be abstracted to be less fragile and
   // more efficient. For example, just map function ids to custom
   // handlers.
 
   // Printf and scanf checking.
+  bool HandledFormatString = false;
   for (specific_attr_iterator<FormatAttr>
-         i = FDecl->specific_attr_begin<FormatAttr>(),
-         e = FDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
-    CheckFormatArguments(*i, TheCall);
-  }
+         I = FDecl->specific_attr_begin<FormatAttr>(),
+         E = FDecl->specific_attr_end<FormatAttr>(); I != E ; ++I)
+    if (CheckFormatArguments(*I, Args, NumArgs, IsMemberFunction, Loc, Range))
+        HandledFormatString = true;
+
+  // Refuse POD arguments that weren't caught by the format string
+  // checks above.
+  if (!HandledFormatString && CallType != VariadicDoesNotApply)
+    for (unsigned ArgIdx = NumProtoArgs; ArgIdx < NumArgs; ++ArgIdx)
+      variadicArgumentPODCheck(Args[ArgIdx], CallType);
 
   for (specific_attr_iterator<NonNullAttr>
-         i = FDecl->specific_attr_begin<NonNullAttr>(),
-         e = FDecl->specific_attr_end<NonNullAttr>(); i != e; ++i) {
-    CheckNonNullArguments(*i, TheCall->getArgs(),
-                          TheCall->getCallee()->getLocStart());
-  }
+         I = FDecl->specific_attr_begin<NonNullAttr>(),
+         E = FDecl->specific_attr_end<NonNullAttr>(); I != E; ++I)
+    CheckNonNullArguments(*I, Args, Loc);
+}
+
+/// CheckConstructorCall - Check a constructor call for correctness and safety
+/// properties not enforced by the C type system.
+void Sema::CheckConstructorCall(FunctionDecl *FDecl, Expr **Args,
+                                unsigned NumArgs,
+                                const FunctionProtoType *Proto,
+                                SourceLocation Loc) {
+  VariadicCallType CallType =
+    Proto->isVariadic() ? VariadicConstructor : VariadicDoesNotApply;
+  checkCall(FDecl, Args, NumArgs, Proto->getNumArgs(),
+            /*IsMemberFunction=*/true, Loc, SourceRange(), CallType);
+}
+
+/// CheckFunctionCall - Check a direct function call for various correctness
+/// and safety properties not strictly enforced by the C type system.
+bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
+                             const FunctionProtoType *Proto) {
+  bool IsMemberFunction = isa<CXXMemberCallExpr>(TheCall);
+  VariadicCallType CallType = getVariadicCallType(FDecl, Proto,
+                                                  TheCall->getCallee());
+  unsigned NumProtoArgs = Proto ? Proto->getNumArgs() : 0;
+  checkCall(FDecl, TheCall->getArgs(), TheCall->getNumArgs(), NumProtoArgs,
+            IsMemberFunction, TheCall->getRParenLoc(),
+            TheCall->getCallee()->getSourceRange(), CallType);
+
+  IdentifierInfo *FnInfo = FDecl->getIdentifier();
+  // None of the checks below are needed for functions that don't have
+  // simple names (e.g., C++ conversion functions).
+  if (!FnInfo)
+    return false;
 
   unsigned CMId = FDecl->getMemoryFunctionKind();
   if (CMId == 0)
@@ -464,25 +522,18 @@
 
 bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac, 
                                Expr **Args, unsigned NumArgs) {
-  for (specific_attr_iterator<FormatAttr>
-       i = Method->specific_attr_begin<FormatAttr>(),
-       e = Method->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+  VariadicCallType CallType =
+      Method->isVariadic() ? VariadicMethod : VariadicDoesNotApply;
 
-    CheckFormatArguments(*i, Args, NumArgs, false, lbrac, 
-                         Method->getSourceRange());
-  }
-
-  // diagnose nonnull arguments.
-  for (specific_attr_iterator<NonNullAttr>
-       i = Method->specific_attr_begin<NonNullAttr>(),
-       e = Method->specific_attr_end<NonNullAttr>(); i != e; ++i) {
-    CheckNonNullArguments(*i, Args, lbrac);
-  }
+  checkCall(Method, Args, NumArgs, Method->param_size(),
+            /*IsMemberFunction=*/false,
+            lbrac, Method->getSourceRange(), CallType);
 
   return false;
 }
 
-bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall) {
+bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall,
+                          const FunctionProtoType *Proto) {
   const VarDecl *V = dyn_cast<VarDecl>(NDecl);
   if (!V)
     return false;
@@ -491,13 +542,15 @@
   if (!Ty->isBlockPointerType())
     return false;
 
-  // format string checking.
-  for (specific_attr_iterator<FormatAttr>
-       i = NDecl->specific_attr_begin<FormatAttr>(),
-       e = NDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
-    CheckFormatArguments(*i, TheCall);
-  }
+  VariadicCallType CallType = 
+      Proto && Proto->isVariadic() ? VariadicBlock : VariadicDoesNotApply ;
+  unsigned NumProtoArgs = Proto ? Proto->getNumArgs() : 0;
 
+  checkCall(NDecl, TheCall->getArgs(), TheCall->getNumArgs(),
+            NumProtoArgs, /*IsMemberFunction=*/false,
+            TheCall->getRParenLoc(),
+            TheCall->getCallee()->getSourceRange(), CallType);
+  
   return false;
 }
 
@@ -1501,14 +1554,18 @@
   return false;
 }
 
-// Handle i > 1 ? "x" : "y", recursively.
-bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
-                                  unsigned NumArgs, bool HasVAListArg,
-                                  unsigned format_idx, unsigned firstDataArg,
-                                  FormatStringType Type, bool inFunctionCall) {
+// Determine if an expression is a string literal or constant string.
+// If this function returns false on the arguments to a function expecting a
+// format string, we will usually need to emit a warning.
+// True string literals are then checked by CheckFormatString.
+Sema::StringLiteralCheckType
+Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
+                            unsigned NumArgs, bool HasVAListArg,
+                            unsigned format_idx, unsigned firstDataArg,
+                            FormatStringType Type, bool inFunctionCall) {
  tryAgain:
   if (E->isTypeDependent() || E->isValueDependent())
-    return false;
+    return SLCT_NotALiteral;
 
   E = E->IgnoreParenCasts();
 
@@ -1517,18 +1574,26 @@
     // The behavior of printf and friends in this case is implementation
     // dependent.  Ideally if the format string cannot be null then
     // it should have a 'nonnull' attribute in the function prototype.
-    return true;
+    return SLCT_CheckedLiteral;
 
   switch (E->getStmtClass()) {
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::ConditionalOperatorClass: {
-    const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
-    return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg,
-                                  format_idx, firstDataArg, Type,
-                                  inFunctionCall)
-       && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg,
-                                 format_idx, firstDataArg, Type,
-                                 inFunctionCall);
+    // The expression is a literal if both sub-expressions were, and it was
+    // completely checked only if both sub-expressions were checked.
+    const AbstractConditionalOperator *C =
+        cast<AbstractConditionalOperator>(E);
+    StringLiteralCheckType Left =
+        checkFormatStringExpr(C->getTrueExpr(), Args, NumArgs,
+                              HasVAListArg, format_idx, firstDataArg,
+                              Type, inFunctionCall);
+    if (Left == SLCT_NotALiteral)
+      return SLCT_NotALiteral;
+    StringLiteralCheckType Right =
+        checkFormatStringExpr(C->getFalseExpr(), Args, NumArgs,
+                              HasVAListArg, format_idx, firstDataArg,
+                              Type, inFunctionCall);
+    return Left < Right ? Left : Right;
   }
 
   case Stmt::ImplicitCastExprClass: {
@@ -1541,13 +1606,13 @@
       E = src;
       goto tryAgain;
     }
-    return false;
+    return SLCT_NotALiteral;
 
   case Stmt::PredefinedExprClass:
     // While __func__, etc., are technically not string literals, they
     // cannot contain format specifiers and thus are not a security
     // liability.
-    return true;
+    return SLCT_UncheckedLiteral;
       
   case Stmt::DeclRefExprClass: {
     const DeclRefExpr *DR = cast<DeclRefExpr>(E);
@@ -1576,9 +1641,10 @@
             if (InitList->isStringLiteralInit())
               Init = InitList->getInit(0)->IgnoreParenImpCasts();
           }
-          return SemaCheckStringLiteral(Init, Args, NumArgs,
-                                        HasVAListArg, format_idx, firstDataArg,
-                                        Type, /*inFunctionCall*/false);
+          return checkFormatStringExpr(Init, Args, NumArgs,
+                                       HasVAListArg, format_idx,
+                                       firstDataArg, Type,
+                                       /*inFunctionCall*/false);
         }
       }
 
@@ -1612,14 +1678,14 @@
               // We can't pass a 'scanf' string to a 'printf' function.
               if (PVIndex == PVFormat->getFormatIdx() &&
                   Type == GetFormatStringType(PVFormat))
-                return true;
+                return SLCT_UncheckedLiteral;
             }
           }
         }
       }
     }
 
-    return false;
+    return SLCT_NotALiteral;
   }
 
   case Stmt::CallExprClass:
@@ -1633,22 +1699,22 @@
             --ArgIndex;
         const Expr *Arg = CE->getArg(ArgIndex - 1);
 
-        return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
-                                      format_idx, firstDataArg, Type,
-                                      inFunctionCall);
+        return checkFormatStringExpr(Arg, Args, NumArgs,
+                                     HasVAListArg, format_idx, firstDataArg,
+                                     Type, inFunctionCall);
       } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
           const Expr *Arg = CE->getArg(0);
-          return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
-                                        format_idx, firstDataArg, Type,
-                                        inFunctionCall);
+          return checkFormatStringExpr(Arg, Args, NumArgs,
+                                       HasVAListArg, format_idx,
+                                       firstDataArg, Type, inFunctionCall);
         }
       }
     }
 
-    return false;
+    return SLCT_NotALiteral;
   }
   case Stmt::ObjCStringLiteralClass:
   case Stmt::StringLiteralClass: {
@@ -1662,14 +1728,14 @@
     if (StrE) {
       CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx,
                         firstDataArg, Type, inFunctionCall);
-      return true;
+      return SLCT_CheckedLiteral;
     }
 
-    return false;
+    return SLCT_NotALiteral;
   }
 
   default:
-    return false;
+    return SLCT_NotALiteral;
   }
 }
 
@@ -1700,42 +1766,34 @@
 
 /// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar
 /// functions) for correct use of format strings.
-void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
-  bool IsCXXMember = false;
-  // The way the format attribute works in GCC, the implicit this argument
-  // of member functions is counted. However, it doesn't appear in our own
-  // lists, so decrement format_idx in that case.
-  IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
-  CheckFormatArguments(Format, TheCall->getArgs(), TheCall->getNumArgs(),
-                       IsCXXMember, TheCall->getRParenLoc(), 
-                       TheCall->getCallee()->getSourceRange());
+/// Returns true if a format string has been fully checked.
+bool Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
+  bool IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
+  return CheckFormatArguments(Format, TheCall->getArgs(),
+                              TheCall->getNumArgs(),
+                              IsCXXMember, TheCall->getRParenLoc(), 
+                              TheCall->getCallee()->getSourceRange());
 }
 
-void Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
+bool Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
                                 unsigned NumArgs, bool IsCXXMember,
                                 SourceLocation Loc, SourceRange Range) {
-  bool HasVAListArg = Format->getFirstArg() == 0;
-  unsigned format_idx = Format->getFormatIdx() - 1;
-  unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1;
-  if (IsCXXMember) {
-    if (format_idx == 0)
-      return;
-    --format_idx;
-    if(firstDataArg != 0)
-      --firstDataArg;
-  }
-  CheckFormatArguments(Args, NumArgs, HasVAListArg, format_idx,
-                       firstDataArg, GetFormatStringType(Format), Loc, Range);
+  FormatStringInfo FSI;
+  if (getFormatStringInfo(Format, IsCXXMember, &FSI))
+    return CheckFormatArguments(Args, NumArgs, FSI.HasVAListArg, FSI.FormatIdx,
+                                FSI.FirstDataArg, GetFormatStringType(Format),
+                                Loc, Range);
+  return false;
 }
 
-void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
+bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
                                 bool HasVAListArg, unsigned format_idx,
                                 unsigned firstDataArg, FormatStringType Type,
                                 SourceLocation Loc, SourceRange Range) {
   // CHECK: printf/scanf-like function is called with no format string.
   if (format_idx >= NumArgs) {
     Diag(Loc, diag::warn_missing_format_string) << Range;
-    return;
+    return false;
   }
 
   const Expr *OrigFormatExpr = Args[format_idx]->IgnoreParenCasts();
@@ -1752,14 +1810,17 @@
   // C string (e.g. "%d")
   // ObjC string uses the same format specifiers as C string, so we can use
   // the same format string checking logic for both ObjC and C strings.
-  if (SemaCheckStringLiteral(OrigFormatExpr, Args, NumArgs, HasVAListArg,
-                             format_idx, firstDataArg, Type))
-    return;  // Literal format string found, check done!
+  StringLiteralCheckType CT =
+      checkFormatStringExpr(OrigFormatExpr, Args, NumArgs, HasVAListArg,
+                            format_idx, firstDataArg, Type);
+  if (CT != SLCT_NotALiteral)
+    // Literal format string found, check done!
+    return CT == SLCT_CheckedLiteral;
 
   // Strftime is particular as it always uses a single 'time' argument,
   // so it is safe to pass a non-literal string.
   if (Type == FST_Strftime)
-    return;
+    return false;
 
   // Do not emit diag when the string param is a macro expansion and the
   // format is either NSString or CFString. This is a hack to prevent
@@ -1767,7 +1828,7 @@
   // which are usually used in place of NS and CF string literals.
   if (Type == FST_NSString &&
       SourceMgr.isInSystemMacro(Args[format_idx]->getLocStart()))
-    return;
+    return false;
 
   // If there are no arguments specified, warn with -Wformat-security, otherwise
   // warn only with -Wformat-nonliteral.
@@ -1779,6 +1840,7 @@
     Diag(Args[format_idx]->getLocStart(),
          diag::warn_format_nonliteral)
            << OrigFormatExpr->getSourceRange();
+  return false;
 }
 
 namespace {
@@ -2138,7 +2200,11 @@
   bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
                              const char *startSpecifier,
                              unsigned specifierLen);
-  
+  bool checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
+                       const char *StartSpecifier,
+                       unsigned SpecifierLen,
+                       const Expr *E);
+
   bool HandleAmount(const analyze_format_string::OptionalAmount &Amt, unsigned k,
                     const char *startSpecifier, unsigned specifierLen);
   void HandleInvalidAmount(const analyze_printf::PrintfSpecifier &FS,
@@ -2152,6 +2218,9 @@
                          const analyze_printf::OptionalFlag &ignoredFlag,
                          const analyze_printf::OptionalFlag &flag,
                          const char *startSpecifier, unsigned specifierLen);
+  bool checkForCStrMembers(const analyze_printf::ArgTypeResult &ATR,
+                           const Expr *E, const CharSourceRange &CSR);
+
 };  
 }
 
@@ -2269,6 +2338,64 @@
                          getSpecifierRange(ignoredFlag.getPosition(), 1)));
 }
 
+// Determines if the specified is a C++ class or struct containing
+// a member with the specified name and kind (e.g. a CXXMethodDecl named
+// "c_str()").
+template<typename MemberKind>
+static llvm::SmallPtrSet<MemberKind*, 1>
+CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
+  const RecordType *RT = Ty->getAs<RecordType>();
+  llvm::SmallPtrSet<MemberKind*, 1> Results;
+
+  if (!RT)
+    return Results;
+  const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
+  if (!RD)
+    return Results;
+
+  LookupResult R(S, &S.PP.getIdentifierTable().get(Name), SourceLocation(),
+                 Sema::LookupMemberName);
+
+  // We just need to include all members of the right kind turned up by the
+  // filter, at this point.
+  if (S.LookupQualifiedName(R, RT->getDecl()))
+    for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+      NamedDecl *decl = (*I)->getUnderlyingDecl();
+      if (MemberKind *FK = dyn_cast<MemberKind>(decl))
+        Results.insert(FK);
+    }
+  return Results;
+}
+
+// Check if a (w)string was passed when a (w)char* was needed, and offer a
+// better diagnostic if so. ATR is assumed to be valid.
+// Returns true when a c_str() conversion method is found.
+bool CheckPrintfHandler::checkForCStrMembers(
+    const analyze_printf::ArgTypeResult &ATR, const Expr *E,
+    const CharSourceRange &CSR) {
+  typedef llvm::SmallPtrSet<CXXMethodDecl*, 1> MethodSet;
+
+  MethodSet Results =
+      CXXRecordMembersNamed<CXXMethodDecl>("c_str", S, E->getType());
+
+  for (MethodSet::iterator MI = Results.begin(), ME = Results.end();
+       MI != ME; ++MI) {
+    const CXXMethodDecl *Method = *MI;
+    if (Method->getNumParams() == 0 &&
+          ATR.matchesType(S.Context, Method->getResultType())) {
+      // FIXME: Suggest parens if the expression needs them.
+      SourceLocation EndLoc =
+          S.getPreprocessor().getLocForEndOfToken(E->getLocEnd());
+      S.Diag(E->getLocStart(), diag::note_printf_c_str)
+          << "c_str()"
+          << FixItHint::CreateInsertion(EndLoc, ".c_str()");
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool
 CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
                                             &FS,
@@ -2396,20 +2523,30 @@
   if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
     return false;
 
+  return checkFormatExpr(FS, startSpecifier, specifierLen,
+                         getDataArg(argIndex));
+}
+
+bool
+CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
+                                    const char *StartSpecifier,
+                                    unsigned SpecifierLen,
+                                    const Expr *E) {
+  using namespace analyze_format_string;
+  using namespace analyze_printf;
   // Now type check the data expression that matches the
   // format specifier.
-  const Expr *Ex = getDataArg(argIndex);
   const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context,
                                                            ObjCContext);
-  if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
+  if (ATR.isValid() && !ATR.matchesType(S.Context, E->getType())) {
     // Look through argument promotions for our error message's reported type.
     // This includes the integral and floating promotions, but excludes array
     // and function pointer decay; seeing that an argument intended to be a
     // string has type 'char [6]' is probably more confusing than 'char *'.
-    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex)) {
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
       if (ICE->getCastKind() == CK_IntegralCast ||
           ICE->getCastKind() == CK_FloatingCast) {
-        Ex = ICE->getSubExpr();
+        E = ICE->getSubExpr();
 
         // Check if we didn't match because of an implicit cast from a 'char'
         // or 'short' to an 'int'.  This is done because printf is a varargs
@@ -2417,7 +2554,7 @@
         if (ICE->getType() == S.Context.IntTy ||
             ICE->getType() == S.Context.UnsignedIntTy) {
           // All further checking is done on the subexpression.
-          if (ATR.matchesType(S.Context, Ex->getType()))
+          if (ATR.matchesType(S.Context, E->getType()))
             return true;
         }
       }
@@ -2425,7 +2562,7 @@
 
     // We may be able to offer a FixItHint if it is a supported type.
     PrintfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(),
+    bool success = fixedFS.fixType(E->getType(), S.getLangOpts(),
                                    S.Context, ObjCContext);
 
     if (success) {
@@ -2436,24 +2573,38 @@
 
       EmitFormatDiagnostic(
         S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
-          << Ex->getSourceRange(),
-        Ex->getLocStart(),
+          << ATR.getRepresentativeTypeName(S.Context) << E->getType()
+          << E->getSourceRange(),
+        E->getLocStart(),
         /*IsStringLocation*/false,
-        getSpecifierRange(startSpecifier, specifierLen),
+        getSpecifierRange(StartSpecifier, SpecifierLen),
         FixItHint::CreateReplacement(
-          getSpecifierRange(startSpecifier, specifierLen),
+          getSpecifierRange(StartSpecifier, SpecifierLen),
           os.str()));
-    }
-    else {
-      EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
-          << getSpecifierRange(startSpecifier, specifierLen)
-          << Ex->getSourceRange(),
-        Ex->getLocStart(),
-        /*IsStringLocation*/false,
-        getSpecifierRange(startSpecifier, specifierLen));
+    } else {
+      const CharSourceRange &CSR = getSpecifierRange(StartSpecifier,
+                                                     SpecifierLen);
+      // Since the warning for passing non-POD types to variadic functions
+      // was deferred until now, we emit a warning for non-POD
+      // arguments here.
+      if (S.isValidVarArgType(E->getType()) == Sema::VAK_Invalid) {
+        EmitFormatDiagnostic(
+          S.PDiag(diag::warn_non_pod_vararg_with_format_string)
+            << S.getLangOpts().CPlusPlus0x
+            << E->getType()
+            << ATR.getRepresentativeTypeName(S.Context)
+            << CSR
+            << E->getSourceRange(),
+          E->getLocStart(), /*IsStringLocation*/false, CSR);
+
+        checkForCStrMembers(ATR, E, CSR);
+      } else
+        EmitFormatDiagnostic(
+          S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+            << ATR.getRepresentativeTypeName(S.Context) << E->getType()
+            << CSR
+            << E->getSourceRange(),
+          E->getLocStart(), /*IsStringLocation*/false, CSR);
     }
   }