Thanks, this should be fixed in r159290.<br><br><div class="gmail_quote">On Wed, Jun 27, 2012 at 11:31 AM, David Dean <span dir="ltr"><<a href="mailto:david_dean@apple.com" target="_blank">david_dean@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Some of the gcc test suite tests are failing after this commit.<br>
<br>
You should be able to reproduce with:<br>
<br>
gcc -m64 -DSKIP_DECIMAL_FLOAT -c -o c_compat_x_alt.o \<br>
    clang-tests/gcc-4_2-testsuite/src/gcc.dg/compat/fnptr-by-value-1_x.c<br>
gcc -m64 -DSKIP_DECIMAL_FLOAT -c -o c_compat_y_alt.o \<br>
    clang-tests/gcc-4_2-testsuite/src/gcc.dg/compat/fnptr-by-value-1_y.c<br>
clang -m64 -DSKIP_DECIMAL_FLOAT -c -o c_compat_main_tst.o \<br>
    clang-tests/gcc-4_2-testsuite/src/gcc.dg/compat/fnptr-by-value-1_main.c<br>
clang -m64 -DSKIP_DECIMAL_FLOAT -c -o c_compat_x_tst.o \<br>
    clang-tests/gcc-4_2-testsuite/src/gcc.dg/compat/fnptr-by-value-1_x.c<br>
clang -m64 -DSKIP_DECIMAL_FLOAT -c -o c_compat_y_tst.o \<br>
    clang-tests/gcc-4_2-testsuite/src/gcc.dg/compat/fnptr-by-value-1_y.c<br>
clang c_compat_main_tst.o c_compat_x_tst.o c_compat_y_alt.o -m64 \<br>
    -Wl,-multiply_defined,suppress -lm -o gcc-dg-compat-fnptr-by-value-1-02<br>
./gcc-dg-compat-fnptr-by-value-1-02<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On 25 Jun 2012, at 1:30 PM, Richard Smith wrote:<br>
<br>
> Author: rsmith<br>
> Date: Mon Jun 25 15:30:08 2012<br>
> New Revision: 159159<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=159159&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=159159&view=rev</a><br>
> Log:<br>
> Unrevert r158887, reverted in r158949, along with a fix for the bug which<br>
> resulted in it being reverted. A test for that bug was added in r158950.<br>
><br>
> Original comment:<br>
><br>
> If an object (such as a std::string) with an appropriate c_str() member function<br>
> is passed to a variadic function in a position where a format string indicates<br>
> that c_str()'s return type is desired, provide a note suggesting that the user<br>
> may have intended to call the c_str() member.<br>
><br>
> Factor the non-POD-vararg checking out of DefaultVariadicArgumentPromotion and<br>
> move it to SemaChecking in order to facilitate this. Factor the call checking<br>
> out of function call checking and block call checking, and extend it to cover<br>
> constructor calls too.<br>
><br>
> Patch by Sam Panzer!<br>
><br>
> Added:<br>
>    cfe/trunk/test/SemaCXX/printf-block.cpp<br>
>      - copied unchanged from r158948, cfe/trunk/test/SemaCXX/printf-block.cpp<br>
>    cfe/trunk/test/SemaCXX/printf-cstr.cpp<br>
>      - copied unchanged from r158948, cfe/trunk/test/SemaCXX/printf-cstr.cpp<br>
> Modified:<br>
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
>    cfe/trunk/include/clang/Sema/Sema.h<br>
>    cfe/trunk/lib/Sema/SemaChecking.cpp<br>
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
>    cfe/trunk/lib/Sema/SemaExpr.cpp<br>
>    cfe/trunk/lib/Sema/SemaOverload.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=159159&r1=159158&r2=159159&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=159159&r1=159158&r2=159159&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jun 25 15:30:08 2012<br>
> @@ -4725,6 +4725,11 @@<br>
>   "cannot pass object with interface type %0 by-value through variadic "<br>
>   "%select{function|block|method}1">;<br>
><br>
> +def warn_non_pod_vararg_with_format_string : Warning<<br>
> +  "cannot pass %select{non-POD|non-trivial}0 object of type %1 to variadic "<br>
> +  "function; expected type from format string was %2">,<br>
> +  InGroup<DiagGroup<"non-pod-varargs">>, DefaultError;<br>
> +<br>
> def warn_cannot_pass_non_pod_arg_to_vararg : Warning<<br>
>   "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic"<br>
>   " %select{function|block|method|constructor}2; call will abort at runtime">,<br>
> @@ -5208,7 +5213,8 @@<br>
>   "no closing ']' for '%%[' in scanf format string">,<br>
>   InGroup<Format>;<br>
> def note_format_string_defined : Note<"format string is defined here">;<br>
> -<br>
> +def note_printf_c_str: Note<"did you mean to call the %0 method?">;<br>
> +<br>
> def warn_null_arg : Warning<<br>
>   "null passed to a callee which requires a non-null argument">,<br>
>   InGroup<NonNull>;<br>
> @@ -5675,4 +5681,3 @@<br>
> } // end of documentation issue category<br>
><br>
> } // end of sema component.<br>
> -<br>
><br>
> Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=159159&r1=159158&r2=159159&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=159159&r1=159158&r2=159159&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
> +++ cfe/trunk/include/clang/Sema/Sema.h Mon Jun 25 15:30:08 2012<br>
> @@ -6408,6 +6408,21 @@<br>
>     VariadicDoesNotApply<br>
>   };<br>
><br>
> +  VariadicCallType getVariadicCallType(FunctionDecl *FDecl,<br>
> +                                       const FunctionProtoType *Proto,<br>
> +                                       Expr *Fn);<br>
> +<br>
> +  // Used for determining in which context a type is allowed to be passed to a<br>
> +  // vararg function.<br>
> +  enum VarArgKind {<br>
> +    VAK_Valid,<br>
> +    VAK_ValidInCXX11,<br>
> +    VAK_Invalid<br>
> +  };<br>
> +<br>
> +  // Determines which VarArgKind fits an expression.<br>
> +  VarArgKind isValidVarArgType(const QualType &Ty);<br>
> +<br>
>   /// GatherArgumentsForCall - Collector argument expressions for various<br>
>   /// form of call prototypes.<br>
>   bool GatherArgumentsForCall(SourceLocation CallLoc,<br>
> @@ -6420,10 +6435,14 @@<br>
>                               bool AllowExplicit = false);<br>
><br>
>   // DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but<br>
> -  // will warn if the resulting type is not a POD type.<br>
> +  // will return ExprError() if the resulting type is not a POD type.<br>
>   ExprResult DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,<br>
>                                               FunctionDecl *FDecl);<br>
><br>
> +  /// Checks to see if the given expression is a valid argument to a variadic<br>
> +  /// function, issuing a diagnostic and returning NULL if not.<br>
> +  bool variadicArgumentPODCheck(const Expr *E, VariadicCallType CT);<br>
> +<br>
>   // UsualArithmeticConversions - performs the UsualUnaryConversions on it's<br>
>   // operands and then handles various conversions that are common to binary<br>
>   // operators (C99 6.3.1.8). If both operands aren't arithmetic, this<br>
> @@ -6998,10 +7017,33 @@<br>
>                         const ArraySubscriptExpr *ASE=0,<br>
>                         bool AllowOnePastEnd=true, bool IndexNegated=false);<br>
>   void CheckArrayAccess(const Expr *E);<br>
> -  bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);<br>
> +  // Used to grab the relevant information from a FormatAttr and a<br>
> +  // FunctionDeclaration.<br>
> +  struct FormatStringInfo {<br>
> +    unsigned FormatIdx;<br>
> +    unsigned FirstDataArg;<br>
> +    bool HasVAListArg;<br>
> +  };<br>
> +<br>
> +  bool getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,<br>
> +                           FormatStringInfo *FSI);<br>
> +  bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,<br>
> +                         const FunctionProtoType *Proto);<br>
>   bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,<br>
>                            Expr **Args, unsigned NumArgs);<br>
> -  bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall);<br>
> +  bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall,<br>
> +                      const FunctionProtoType *Proto);<br>
> +  void CheckConstructorCall(FunctionDecl *FDecl,<br>
> +                            Expr **Args,<br>
> +                            unsigned NumArgs,<br>
> +                            const FunctionProtoType *Proto,<br>
> +                            SourceLocation Loc);<br>
> +<br>
> +  void checkCall(NamedDecl *FDecl, Expr **Args, unsigned NumArgs,<br>
> +                 unsigned NumProtoArgs, bool IsMemberFunction,<br>
> +                 SourceLocation Loc, SourceRange Range,<br>
> +                 VariadicCallType CallType);<br>
> +<br>
><br>
>   bool CheckObjCString(Expr *Arg);<br>
><br>
> @@ -7036,21 +7078,31 @@<br>
>     FST_Unknown<br>
>   };<br>
>   static FormatStringType GetFormatStringType(const FormatAttr *Format);<br>
> -  bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs,<br>
> -                              bool HasVAListArg, unsigned format_idx,<br>
> -                              unsigned firstDataArg, FormatStringType Type,<br>
> -                              bool inFunctionCall = true);<br>
> +<br>
> +  enum StringLiteralCheckType {<br>
> +    SLCT_NotALiteral,<br>
> +    SLCT_UncheckedLiteral,<br>
> +    SLCT_CheckedLiteral<br>
> +  };<br>
> +<br>
> +  StringLiteralCheckType checkFormatStringExpr(const Expr *E,<br>
> +                                               Expr **Args, unsigned NumArgs,<br>
> +                                               bool HasVAListArg,<br>
> +                                               unsigned format_idx,<br>
> +                                               unsigned firstDataArg,<br>
> +                                               FormatStringType Type,<br>
> +                                               bool inFunctionCall = true);<br>
><br>
>   void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,<br>
>                          Expr **Args, unsigned NumArgs, bool HasVAListArg,<br>
>                          unsigned format_idx, unsigned firstDataArg,<br>
>                          FormatStringType Type, bool inFunctionCall);<br>
><br>
> -  void CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);<br>
> -  void CheckFormatArguments(const FormatAttr *Format, Expr **Args,<br>
> +  bool CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);<br>
> +  bool CheckFormatArguments(const FormatAttr *Format, Expr **Args,<br>
>                             unsigned NumArgs, bool IsCXXMember,<br>
>                             SourceLocation Loc, SourceRange Range);<br>
> -  void CheckFormatArguments(Expr **Args, unsigned NumArgs,<br>
> +  bool CheckFormatArguments(Expr **Args, unsigned NumArgs,<br>
>                             bool HasVAListArg, unsigned format_idx,<br>
>                             unsigned firstDataArg, FormatStringType Type,<br>
>                             SourceLocation Loc, SourceRange range);<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=159159&r1=159158&r2=159159&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=159159&r1=159158&r2=159159&view=diff</a><br>

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

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jun 25 15:30:08 2012<br>
> @@ -9028,13 +9028,6 @@<br>
>   unsigned NumExprs = ExprArgs.size();<br>
>   Expr **Exprs = (Expr **)ExprArgs.release();<br>
><br>
> -  for (specific_attr_iterator<NonNullAttr><br>
> -           i = Constructor->specific_attr_begin<NonNullAttr>(),<br>
> -           e = Constructor->specific_attr_end<NonNullAttr>(); i != e; ++i) {<br>
> -    const NonNullAttr *NonNull = *i;<br>
> -    CheckNonNullArguments(NonNull, ExprArgs.get(), ConstructLoc);<br>
> -  }<br>
> -<br>
>   MarkFunctionReferenced(ConstructLoc, Constructor);<br>
>   return Owned(CXXConstructExpr::Create(Context, DeclInitType, ConstructLoc,<br>
>                                         Constructor, Elidable, Exprs, NumExprs,<br>
> @@ -9100,7 +9093,7 @@<br>
> bool<br>
> Sema::CompleteConstructorCall(CXXConstructorDecl *Constructor,<br>
>                               MultiExprArg ArgsPtr,<br>
> -                              SourceLocation Loc,<br>
> +                              SourceLocation Loc,<br>
>                               ASTOwningVector<Expr*> &ConvertedArgs,<br>
>                               bool AllowExplicit) {<br>
>   // FIXME: This duplicates a lot of code from Sema::ConvertArgumentsForCall.<br>
> @@ -9128,7 +9121,8 @@<br>
><br>
>   DiagnoseSentinelCalls(Constructor, Loc, AllArgs.data(), AllArgs.size());<br>
><br>
> -  // FIXME: Missing call to CheckFunctionCall or equivalent<br>
> +  CheckConstructorCall(Constructor, AllArgs.data(), AllArgs.size(),<br>
> +                       Proto, Loc);<br>
><br>
>   return Invalid;<br>
> }<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=159159&r1=159158&r2=159159&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=159159&r1=159158&r2=159159&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jun 25 15:30:08 2012<br>
> @@ -598,12 +598,59 @@<br>
>   return Owned(E);<br>
> }<br>
><br>
> +/// Determine the degree of POD-ness for an expression.<br>
> +/// Incomplete types are considered POD, since this check can be performed<br>
> +/// when we're in an unevaluated context.<br>
> +Sema::VarArgKind Sema::isValidVarArgType(const QualType &Ty) {<br>
> +  if (Ty->isIncompleteType() || Ty.isCXX98PODType(Context))<br>
> +    return VAK_Valid;<br>
> +  // C++0x [expr.call]p7:<br>
> +  //   Passing a potentially-evaluated argument of class type (Clause 9)<br>
> +  //   having a non-trivial copy constructor, a non-trivial move constructor,<br>
> +  //   or a non-trivial destructor, with no corresponding parameter,<br>
> +  //   is conditionally-supported with implementation-defined semantics.<br>
> +<br>
> +  if (getLangOpts().CPlusPlus0x && !Ty->isDependentType())<br>
> +    if (CXXRecordDecl *Record = Ty->getAsCXXRecordDecl())<br>
> +      if (Record->hasTrivialCopyConstructor() &&<br>
> +          Record->hasTrivialMoveConstructor() &&<br>
> +          Record->hasTrivialDestructor())<br>
> +        return VAK_ValidInCXX11;<br>
> +<br>
> +  if (getLangOpts().ObjCAutoRefCount && Ty->isObjCLifetimeType())<br>
> +    return VAK_Valid;<br>
> +  return VAK_Invalid;<br>
> +}<br>
> +<br>
> +bool Sema::variadicArgumentPODCheck(const Expr *E, VariadicCallType CT) {<br>
> +  // Don't allow one to pass an Objective-C interface to a vararg.<br>
> +  const QualType & Ty = E->getType();<br>
> +<br>
> +  // Complain about passing non-POD types through varargs.<br>
> +  switch (isValidVarArgType(Ty)) {<br>
> +  case VAK_Valid:<br>
> +    break;<br>
> +  case VAK_ValidInCXX11:<br>
> +    DiagRuntimeBehavior(E->getLocStart(), 0,<br>
> +        PDiag(diag::warn_cxx98_compat_pass_non_pod_arg_to_vararg)<br>
> +        << E->getType() << CT);<br>
> +    break;<br>
> +  case VAK_Invalid:<br>
> +    return DiagRuntimeBehavior(E->getLocStart(), 0,<br>
> +                   PDiag(diag::warn_cannot_pass_non_pod_arg_to_vararg)<br>
> +                   << getLangOpts().CPlusPlus0x << Ty << CT);<br>
> +  }<br>
> +  // c++ rules are enforced elsewhere.<br>
> +  return false;<br>
> +}<br>
> +<br>
> /// DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but<br>
> /// will warn if the resulting type is not a POD type, and rejects ObjC<br>
> /// interfaces passed by value.<br>
> ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,<br>
>                                                   FunctionDecl *FDecl) {<br>
> -  if (const BuiltinType *PlaceholderTy = E->getType()->getAsPlaceholderType()) {<br>
> +  const QualType &Ty = E->getType();<br>
> +  if (const BuiltinType *PlaceholderTy = Ty->getAsPlaceholderType()) {<br>
>     // Strip the unbridged-cast placeholder expression off, if applicable.<br>
>     if (PlaceholderTy->getKind() == BuiltinType::ARCUnbridgedCast &&<br>
>         (CT == VariadicMethod ||<br>
> @@ -624,77 +671,44 @@<br>
>     return ExprError();<br>
>   E = ExprRes.take();<br>
><br>
> -  // Don't allow one to pass an Objective-C interface to a vararg.<br>
> -  if (E->getType()->isObjCObjectType() &&<br>
> +  if (Ty->isObjCObjectType() &&<br>
>     DiagRuntimeBehavior(E->getLocStart(), 0,<br>
>                         PDiag(diag::err_cannot_pass_objc_interface_to_vararg)<br>
> -                          << E->getType() << CT))<br>
> +                          << Ty << CT))<br>
>     return ExprError();<br>
><br>
> -  // Complain about passing non-POD types through varargs. However, don't<br>
> -  // perform this check for incomplete types, which we can get here when we're<br>
> -  // in an unevaluated context.<br>
> -  if (!E->getType()->isIncompleteType() &&<br>
> -      !E->getType().isCXX98PODType(Context)) {<br>
> -    // C++0x [expr.call]p7:<br>
> -    //   Passing a potentially-evaluated argument of class type (Clause 9)<br>
> -    //   having a non-trivial copy constructor, a non-trivial move constructor,<br>
> -    //   or a non-trivial destructor, with no corresponding parameter,<br>
> -    //   is conditionally-supported with implementation-defined semantics.<br>
> -    bool TrivialEnough = false;<br>
> -    if (getLangOpts().CPlusPlus0x && !E->getType()->isDependentType())  {<br>
> -      if (CXXRecordDecl *Record = E->getType()->getAsCXXRecordDecl()) {<br>
> -        if (Record->hasTrivialCopyConstructor() &&<br>
> -            Record->hasTrivialMoveConstructor() &&<br>
> -            Record->hasTrivialDestructor()) {<br>
> -          DiagRuntimeBehavior(E->getLocStart(), 0,<br>
> -            PDiag(diag::warn_cxx98_compat_pass_non_pod_arg_to_vararg)<br>
> -              << E->getType() << CT);<br>
> -          TrivialEnough = true;<br>
> -        }<br>
> -      }<br>
> -    }<br>
> +  // Diagnostics regarding non-POD argument types are<br>
> +  // emitted along with format string checking in Sema::CheckFunctionCall().<br>
> +  if (isValidVarArgType(Ty) == VAK_Invalid) {<br>
> +    // Turn this into a trap.<br>
> +    CXXScopeSpec SS;<br>
> +    SourceLocation TemplateKWLoc;<br>
> +    UnqualifiedId Name;<br>
> +    Name.setIdentifier(PP.getIdentifierInfo("__builtin_trap"),<br>
> +                       E->getLocStart());<br>
> +    ExprResult TrapFn = ActOnIdExpression(TUScope, SS, TemplateKWLoc,<br>
> +                                          Name, true, false);<br>
> +    if (TrapFn.isInvalid())<br>
> +      return ExprError();<br>
><br>
> -    if (!TrivialEnough &&<br>
> -        getLangOpts().ObjCAutoRefCount &&<br>
> -        E->getType()->isObjCLifetimeType())<br>
> -      TrivialEnough = true;<br>
> -<br>
> -    if (TrivialEnough) {<br>
> -      // Nothing to diagnose. This is okay.<br>
> -    } else if (DiagRuntimeBehavior(E->getLocStart(), 0,<br>
> -                          PDiag(diag::warn_cannot_pass_non_pod_arg_to_vararg)<br>
> -                            << getLangOpts().CPlusPlus0x << E->getType()<br>
> -                            << CT)) {<br>
> -      // Turn this into a trap.<br>
> -      CXXScopeSpec SS;<br>
> -      SourceLocation TemplateKWLoc;<br>
> -      UnqualifiedId Name;<br>
> -      Name.setIdentifier(PP.getIdentifierInfo("__builtin_trap"),<br>
> -                         E->getLocStart());<br>
> -      ExprResult TrapFn = ActOnIdExpression(TUScope, SS, TemplateKWLoc, Name,<br>
> -                                            true, false);<br>
> -      if (TrapFn.isInvalid())<br>
> -        return ExprError();<br>
> +    ExprResult Call = ActOnCallExpr(TUScope, TrapFn.get(),<br>
> +                                    E->getLocStart(), MultiExprArg(),<br>
> +                                    E->getLocEnd());<br>
> +    if (Call.isInvalid())<br>
> +      return ExprError();<br>
><br>
> -      ExprResult Call = ActOnCallExpr(TUScope, TrapFn.get(), E->getLocStart(),<br>
> -                                      MultiExprArg(), E->getLocEnd());<br>
> -      if (Call.isInvalid())<br>
> -        return ExprError();<br>
> -<br>
> -      ExprResult Comma = ActOnBinOp(TUScope, E->getLocStart(), tok::comma,<br>
> -                                    Call.get(), E);<br>
> -      if (Comma.isInvalid())<br>
> -        return ExprError();<br>
> -      E = Comma.get();<br>
> -    }<br>
> +    ExprResult Comma = ActOnBinOp(TUScope, E->getLocStart(), tok::comma,<br>
> +                                  Call.get(), E);<br>
> +    if (Comma.isInvalid())<br>
> +      return ExprError();<br>
> +    return Comma.get();<br>
>   }<br>
> -  // c++ rules are enforced elsewhere.<br>
> +<br>
>   if (!getLangOpts().CPlusPlus &&<br>
>       RequireCompleteType(E->getExprLoc(), E->getType(),<br>
>                           diag::err_call_incomplete_argument))<br>
>     return ExprError();<br>
> -<br>
> +<br>
>   return Owned(E);<br>
> }<br>
><br>
> @@ -3431,6 +3445,25 @@<br>
>   return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param));<br>
> }<br>
><br>
> +<br>
> +Sema::VariadicCallType<br>
> +Sema::getVariadicCallType(FunctionDecl *FDecl, const FunctionProtoType *Proto,<br>
> +                          Expr *Fn) {<br>
> +  if (Proto && Proto->isVariadic()) {<br>
> +    if (dyn_cast_or_null<CXXConstructorDecl>(FDecl))<br>
> +      return VariadicConstructor;<br>
> +    else if (Fn && Fn->getType()->isBlockPointerType())<br>
> +      return VariadicBlock;<br>
> +    else if (FDecl) {<br>
> +      if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(FDecl))<br>
> +        if (Method->isInstance())<br>
> +          return VariadicMethod;<br>
> +    }<br>
> +    return VariadicFunction;<br>
> +  }<br>
> +  return VariadicDoesNotApply;<br>
> +}<br>
> +<br>
> /// ConvertArgumentsForCall - Converts the arguments specified in<br>
> /// Args/NumArgs to the parameter types of the function FDecl with<br>
> /// function prototype Proto. Call is the call expression itself, and<br>
> @@ -3522,12 +3555,8 @@<br>
>     }<br>
>   }<br>
>   SmallVector<Expr *, 8> AllArgs;<br>
> -  VariadicCallType CallType =<br>
> -    Proto->isVariadic() ? VariadicFunction : VariadicDoesNotApply;<br>
> -  if (Fn->getType()->isBlockPointerType())<br>
> -    CallType = VariadicBlock; // Block<br>
> -  else if (isa<MemberExpr>(Fn))<br>
> -    CallType = VariadicMethod;<br>
> +  VariadicCallType CallType = getVariadicCallType(FDecl, Proto, Fn);<br>
> +<br>
>   Invalid = GatherArgumentsForCall(Call->getLocStart(), FDecl,<br>
>                                    Proto, 0, Args, NumArgs, AllArgs, CallType);<br>
>   if (Invalid)<br>
> @@ -3616,7 +3645,6 @@<br>
><br>
>   // If this is a variadic call, handle args passed through "...".<br>
>   if (CallType != VariadicDoesNotApply) {<br>
> -<br>
>     // Assume that extern "C" functions with variadic arguments that<br>
>     // return __unknown_anytype aren't *really* variadic.<br>
>     if (Proto->getResultType() == Context.UnknownAnyTy &&<br>
> @@ -3954,7 +3982,8 @@<br>
>   TheCall->setType(FuncT->getCallResultType(Context));<br>
>   TheCall->setValueKind(Expr::getValueKindForType(FuncT->getResultType()));<br>
><br>
> -  if (const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(FuncT)) {<br>
> +  const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(FuncT);<br>
> +  if (Proto) {<br>
>     if (ConvertArgumentsForCall(TheCall, Fn, FDecl, Proto, Args, NumArgs,<br>
>                                 RParenLoc, IsExecConfig))<br>
>       return ExprError();<br>
> @@ -3966,8 +3995,7 @@<br>
>       // on our knowledge of the function definition.<br>
>       const FunctionDecl *Def = 0;<br>
>       if (FDecl->hasBody(Def) && NumArgs != Def->param_size()) {<br>
> -        const FunctionProtoType *Proto<br>
> -          = Def->getType()->getAs<FunctionProtoType>();<br>
> +        Proto = Def->getType()->getAs<FunctionProtoType>();<br>
>         if (!Proto || !(Proto->isVariadic() && NumArgs >= Def->param_size()))<br>
>           Diag(RParenLoc, diag::warn_call_wrong_number_of_arguments)<br>
>             << (NumArgs > Def->param_size()) << FDecl << Fn->getSourceRange();<br>
> @@ -4025,13 +4053,13 @@<br>
><br>
>   // Do special checking on direct calls to functions.<br>
>   if (FDecl) {<br>
> -    if (CheckFunctionCall(FDecl, TheCall))<br>
> +    if (CheckFunctionCall(FDecl, TheCall, Proto))<br>
>       return ExprError();<br>
><br>
>     if (BuiltinID)<br>
>       return CheckBuiltinFunctionCall(BuiltinID, TheCall);<br>
>   } else if (NDecl) {<br>
> -    if (CheckBlockCall(NDecl, TheCall))<br>
> +    if (CheckBlockCall(NDecl, TheCall, Proto))<br>
>       return ExprError();<br>
>   }<br>
><br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=159159&r1=159158&r2=159159&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=159159&r1=159158&r2=159159&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon Jun 25 15:30:08 2012<br>
> @@ -10698,7 +10698,7 @@<br>
><br>
>   DiagnoseSentinelCalls(Method, LParenLoc, Args, NumArgs);<br>
><br>
> -  if (CheckFunctionCall(Method, TheCall))<br>
> +  if (CheckFunctionCall(Method, TheCall, Proto))<br>
>     return ExprError();<br>
><br>
>   if ((isa<CXXConstructorDecl>(CurContext) ||<br>
> @@ -11006,7 +11006,7 @@<br>
><br>
>   DiagnoseSentinelCalls(Method, LParenLoc, Args, NumArgs);<br>
><br>
> -  if (CheckFunctionCall(Method, TheCall))<br>
> +  if (CheckFunctionCall(Method, TheCall, Proto))<br>
>     return true;<br>
><br>
>   return MaybeBindToTemporary(TheCall);<br>
> @@ -11186,7 +11186,7 @@<br>
>   if (CheckCallReturnType(FD->getResultType(), UDSuffixLoc, UDL, FD))<br>
>     return ExprError();<br>
><br>
> -  if (CheckFunctionCall(FD, UDL))<br>
> +  if (CheckFunctionCall(FD, UDL, NULL))<br>
>     return ExprError();<br>
><br>
>   return MaybeBindToTemporary(UDL);<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">-David<br>
<br>
<br>
</font></span></blockquote></div><br>