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@158887 91177308-0d34-0410-b5e6-96231b3b80d8

8 files changed, 518 insertions, 217 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index a7582a0d28..2c13e0c177 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4723,6 +4723,11 @@ def err_cannot_pass_objc_interface_to_vararg : Error<
   "cannot pass object with interface type %0 by-value through variadic "
   "%select{function|block|method}1">;
 
+def warn_non_pod_vararg_with_format_string : Warning<
+  "cannot pass %select{non-POD|non-trivial}0 object of type %1 to variadic "
+  "function; expected type from format string was %2">,
+  InGroup<DiagGroup<"non-pod-varargs">>, DefaultError;
+
 def warn_cannot_pass_non_pod_arg_to_vararg : Warning<
   "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic"
   " %select{function|block|method|constructor}2; call will abort at runtime">,
@@ -5206,7 +5211,8 @@ def warn_scanf_scanlist_incomplete : Warning<
   "no closing ']' for '%%[' in scanf format string">,
   InGroup<Format>;
 def note_format_string_defined : Note<"format string is defined here">;
- 
+def note_printf_c_str: Note<"did you mean to call the %0 method?">;
+
 def warn_null_arg : Warning<
   "null passed to a callee which requires a non-null argument">,
   InGroup<NonNull>;
@@ -5666,4 +5672,3 @@ def err_module_private_definition : Error<
 }
 
 } // end of sema component.
-
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 6e7f23bd6b..4b54e82d40 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6409,6 +6409,21 @@ public:
     VariadicDoesNotApply
   };
 
+  VariadicCallType getVariadicCallType(FunctionDecl *FDecl,
+                                       const FunctionProtoType *Proto,
+                                       Expr *Fn);
+
+  // Used for determining in which context a type is allowed to be passed to a
+  // vararg function.
+  enum VarArgKind {
+    VAK_Valid,
+    VAK_ValidInCXX11,
+    VAK_Invalid
+  };
+
+  // Determines which VarArgKind fits an expression.
+  VarArgKind isValidVarArgType(const QualType &Ty);
+
   /// GatherArgumentsForCall - Collector argument expressions for various
   /// form of call prototypes.
   bool GatherArgumentsForCall(SourceLocation CallLoc,
@@ -6421,10 +6436,14 @@ public:
                               bool AllowExplicit = false);
 
   // DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but
-  // will warn if the resulting type is not a POD type.
+  // will return ExprError() if the resulting type is not a POD type.
   ExprResult DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
                                               FunctionDecl *FDecl);
 
+  /// Checks to see if the given expression is a valid argument to a variadic
+  /// function, issuing a diagnostic and returning NULL if not.
+  bool variadicArgumentPODCheck(const Expr *E, VariadicCallType CT);
+
   // UsualArithmeticConversions - performs the UsualUnaryConversions on it's
   // operands and then handles various conversions that are common to binary
   // operators (C99 6.3.1.8). If both operands aren't arithmetic, this
@@ -6999,10 +7018,33 @@ private:
                         const ArraySubscriptExpr *ASE=0,
                         bool AllowOnePastEnd=true, bool IndexNegated=false);
   void CheckArrayAccess(const Expr *E);
-  bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);
+  // Used to grab the relevant information from a FormatAttr and a
+  // FunctionDeclaration.
+  struct FormatStringInfo {
+    unsigned FormatIdx;
+    unsigned FirstDataArg;
+    bool HasVAListArg;
+  };
+
+  bool getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
+                           FormatStringInfo *FSI);
+  bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
+                         const FunctionProtoType *Proto);
   bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc, 
                            Expr **Args, unsigned NumArgs);
-  bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall);
+  bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall,
+                      const FunctionProtoType *Proto);
+  void CheckConstructorCall(FunctionDecl *FDecl,
+                            Expr **Args,
+                            unsigned NumArgs,
+                            const FunctionProtoType *Proto,
+                            SourceLocation Loc);
+
+  void checkCall(NamedDecl *FDecl, Expr **Args, unsigned NumArgs,
+                 unsigned NumProtoArgs, bool IsMemberFunction,
+                 SourceLocation Loc, SourceRange Range,
+                 VariadicCallType CallType);
+
 
   bool CheckObjCString(Expr *Arg);
 
@@ -7037,21 +7079,31 @@ private:
     FST_Unknown
   };
   static FormatStringType GetFormatStringType(const FormatAttr *Format);
-  bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs,
-                              bool HasVAListArg, unsigned format_idx,
-                              unsigned firstDataArg, FormatStringType Type,
-                              bool inFunctionCall = true);
+
+  enum StringLiteralCheckType {
+    SLCT_NotALiteral,
+    SLCT_UncheckedLiteral,
+    SLCT_CheckedLiteral
+  };
+
+  StringLiteralCheckType checkFormatStringExpr(const Expr *E,
+                                               Expr **Args, unsigned NumArgs,
+                                               bool HasVAListArg,
+                                               unsigned format_idx,
+                                               unsigned firstDataArg,
+                                               FormatStringType Type,
+                                               bool inFunctionCall = true);
 
   void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,
                          Expr **Args, unsigned NumArgs, bool HasVAListArg,
                          unsigned format_idx, unsigned firstDataArg,
                          FormatStringType Type, bool inFunctionCall);
 
-  void CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);
-  void CheckFormatArguments(const FormatAttr *Format, Expr **Args,
+  bool CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);
+  bool CheckFormatArguments(const FormatAttr *Format, Expr **Args,
                             unsigned NumArgs, bool IsCXXMember,
                             SourceLocation Loc, SourceRange Range);
-  void CheckFormatArguments(Expr **Args, unsigned NumArgs,
+  bool CheckFormatArguments(Expr **Args, unsigned NumArgs,
                             bool HasVAListArg, unsigned format_idx,
                             unsigned firstDataArg, FormatStringType Type,
                             SourceLocation Loc, SourceRange range);
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index fff8d80a47..fe5e8aca3d 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 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   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::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
 
 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) {
-
-    CheckFormatArguments(*i, Args, NumArgs, false, lbrac, 
-                         Method->getSourceRange());
-  }
+  VariadicCallType CallType =
+      Method->isVariadic() ? VariadicMethod : VariadicDoesNotApply;
 
-  // 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 @@ bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall) {
   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 @@ bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) {
   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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
     // 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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
       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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
             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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
               // 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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
             --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 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
     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 @@ Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
 
 /// 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 @@ void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
   // 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 @@ void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
   // 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 @@ void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
     Diag(Args[format_idx]->getLocStart(),
          diag::warn_format_nonliteral)
            << OrigFormatExpr->getSourceRange();
+  return false;
 }
 
 namespace {
@@ -2138,7 +2200,11 @@ public:
   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 @@ public:
                          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 @@ void CheckPrintfHandler::HandleIgnoredFlag(
                          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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
   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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
         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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
 
     // 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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
 
       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);
     }
   }
 
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 2a11c74593..828083527a 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -9028,13 +9028,6 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
   unsigned NumExprs = ExprArgs.size();
   Expr **Exprs = (Expr **)ExprArgs.release();
 
-  for (specific_attr_iterator<NonNullAttr>
-           i = Constructor->specific_attr_begin<NonNullAttr>(),
-           e = Constructor->specific_attr_end<NonNullAttr>(); i != e; ++i) {
-    const NonNullAttr *NonNull = *i;
-    CheckNonNullArguments(NonNull, ExprArgs.get(), ConstructLoc);
-  }
-
   MarkFunctionReferenced(ConstructLoc, Constructor);
   return Owned(CXXConstructExpr::Create(Context, DeclInitType, ConstructLoc,
                                         Constructor, Elidable, Exprs, NumExprs,
@@ -9100,7 +9093,7 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
 bool 
 Sema::CompleteConstructorCall(CXXConstructorDecl *Constructor,
                               MultiExprArg ArgsPtr,
-                              SourceLocation Loc,                                    
+                              SourceLocation Loc,
                               ASTOwningVector<Expr*> &ConvertedArgs,
                               bool AllowExplicit) {
   // FIXME: This duplicates a lot of code from Sema::ConvertArgumentsForCall.
@@ -9128,7 +9121,8 @@ Sema::CompleteConstructorCall(CXXConstructorDecl *Constructor,
 
   DiagnoseSentinelCalls(Constructor, Loc, AllArgs.data(), AllArgs.size());
 
-  // FIXME: Missing call to CheckFunctionCall or equivalent
+  CheckConstructorCall(Constructor, AllArgs.data(), AllArgs.size(),
+                       Proto, Loc);
 
   return Invalid;
 }
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaEx