Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h (revision 148179) +++ include/clang/Sema/Sema.h (working copy) @@ -6267,9 +6267,13 @@ bool AllowOnePastEnd=true, bool IndexNegated=false); void CheckArrayAccess(const Expr *E); bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall); + bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc, + Expr **Args, unsigned NumArgs); bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall); bool CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall); + bool CheckablePrintfAttr(const FormatAttr *Format, Expr **Args, + unsigned NumArgs); bool CheckObjCString(Expr *Arg); ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); @@ -6293,13 +6297,13 @@ bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum, llvm::APSInt &Result); - bool SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall, + bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, bool isPrintf, bool inFunctionCall = true); void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr, - const CallExpr *TheCall, bool HasVAListArg, + Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, bool isPrintf, bool inFunctionCall); @@ -6307,10 +6311,14 @@ const Expr * const *ExprArgs, SourceLocation CallSiteLoc); - void CheckPrintfScanfArguments(const CallExpr *TheCall, bool HasVAListArg, + void CheckPrintfScanfArguments(CallExpr *TheCall, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, bool isPrintf); - + void CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs, + bool HasVAListArg, unsigned format_idx, + unsigned firstDataArg, bool isPrintf, + SourceLocation Loc, SourceRange range); + void CheckMemaccessArguments(const CallExpr *Call, FunctionDecl::MemoryFunctionKind CMF, IdentifierInfo *FnName); Index: lib/Sema/SemaExprObjC.cpp =================================================================== --- lib/Sema/SemaExprObjC.cpp (revision 148179) +++ lib/Sema/SemaExprObjC.cpp (working copy) @@ -453,14 +453,12 @@ Args[NumArgs-1]->getLocEnd()); } } - // diagnose nonnull arguments. - for (specific_attr_iterator - i = Method->specific_attr_begin(), - e = Method->specific_attr_end(); i != e; ++i) { - CheckNonNullArguments(*i, Args, lbrac); - } DiagnoseSentinelCalls(Method, lbrac, Args, NumArgs); + + // Do additional checkings on method. + IsError |= CheckObjCMethodCall(Method, lbrac, Args, NumArgs); + return IsError; } Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp (revision 148179) +++ lib/Sema/SemaChecking.cpp (working copy) @@ -49,8 +49,9 @@ /// CheckablePrintfAttr - does a function call have a "printf" attribute /// and arguments that merit checking? bool Sema::CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall) { - if (Format->getType() == "printf") return true; - if (Format->getType() == "printf0") { + StringRef Type = Format->getType(); + if (Type == "printf" || Type == "NSString" || Type == "CFString") return true; + if (Type == "printf0") { // printf0 allows null "format" string; if so don't check format/args unsigned format_idx = Format->getFormatIdx() - 1; // Does the index refer to the implicit object argument? @@ -69,6 +70,23 @@ return false; } +bool Sema::CheckablePrintfAttr(const FormatAttr *Format, Expr **Args, + unsigned NumArgs) { + StringRef Type = Format->getType(); + if (Type == "printf" || Type == "NSString" || Type == "CFString") return true; + if (Type == "printf0") { + // printf0 allows null "format" string; if so don't check format/args + unsigned format_idx = Format->getFormatIdx() - 1; + if (format_idx < NumArgs) { + Expr *Format = Args[format_idx]->IgnoreParenCasts(); + if (!Format->isNullPointerConstant(Context, + Expr::NPC_ValueDependentIsNull)) + return true; + } + } + return false; +} + /// Checks that a call expression's argument count is the desired number. /// This is useful when doing custom type-checking. Returns true on error. static bool checkArgCount(Sema &S, CallExpr *call, unsigned desiredArgCount) { @@ -493,6 +511,34 @@ return false; } +bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac, + Expr **Args, unsigned NumArgs) { + // TODO: Factorize code with CheckFunctionCall + for (specific_attr_iterator + i = Method->specific_attr_begin(), + e = Method->specific_attr_end(); i != e ; ++i) { + + const FormatAttr *Format = *i; + const bool b = Format->getType() == "scanf"; + if (b || CheckablePrintfAttr(Format, Args, NumArgs)) { + bool HasVAListArg = Format->getFirstArg() == 0; + CheckPrintfScanfArguments(Args, NumArgs, HasVAListArg, + Format->getFormatIdx() - 1, + HasVAListArg ? 0 : Format->getFirstArg() - 1, + !b, lbrac, Method->getSourceRange()); + } + } + + // diagnose nonnull arguments. + for (specific_attr_iterator + i = Method->specific_attr_begin(), + e = Method->specific_attr_end(); i != e; ++i) { + CheckNonNullArguments(*i, Args, lbrac); + } + + return false; +} + bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall) { // Printf checking. const FormatAttr *Format = NDecl->getAttr(); @@ -1362,8 +1408,8 @@ } // Handle i > 1 ? "x" : "y", recursively. -bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall, - bool HasVAListArg, +bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args, + unsigned NumArgs, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, bool isPrintf, bool inFunctionCall) { tryAgain: @@ -1376,10 +1422,10 @@ case Stmt::BinaryConditionalOperatorClass: case Stmt::ConditionalOperatorClass: { const AbstractConditionalOperator *C = cast(E); - return SemaCheckStringLiteral(C->getTrueExpr(), TheCall, HasVAListArg, + return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg, format_idx, firstDataArg, isPrintf, inFunctionCall) - && SemaCheckStringLiteral(C->getFalseExpr(), TheCall, HasVAListArg, + && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg, format_idx, firstDataArg, isPrintf, inFunctionCall); } @@ -1427,7 +1473,7 @@ if (isConstant) { if (const Expr *Init = VD->getAnyInitializer()) - return SemaCheckStringLiteral(Init, TheCall, + return SemaCheckStringLiteral(Init, Args, NumArgs, HasVAListArg, format_idx, firstDataArg, isPrintf, /*inFunctionCall*/false); } @@ -1468,7 +1514,7 @@ unsigned ArgIndex = FA->getFormatIdx(); const Expr *Arg = CE->getArg(ArgIndex - 1); - return SemaCheckStringLiteral(Arg, TheCall, HasVAListArg, + return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg, format_idx, firstDataArg, isPrintf, inFunctionCall); } @@ -1488,7 +1534,7 @@ StrE = cast(E); if (StrE) { - CheckFormatString(StrE, E, TheCall, HasVAListArg, format_idx, + CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx, firstDataArg, isPrintf, inFunctionCall); return true; } @@ -1518,12 +1564,9 @@ /// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar /// functions) for correct use of format strings. void -Sema::CheckPrintfScanfArguments(const CallExpr *TheCall, bool HasVAListArg, +Sema::CheckPrintfScanfArguments(CallExpr *TheCall, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, bool isPrintf) { - - const Expr *Fn = TheCall->getCallee(); - // 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. @@ -1539,15 +1582,24 @@ --firstDataArg; } } + + CheckPrintfScanfArguments(TheCall->getArgs(), TheCall->getNumArgs(), + HasVAListArg, format_idx, firstDataArg, isPrintf, + TheCall->getRParenLoc(), + TheCall->getCallee()->getSourceRange()); +} +void Sema::CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs, + bool HasVAListArg, unsigned format_idx, + unsigned firstDataArg, bool isPrintf, + SourceLocation Loc, SourceRange range) { // CHECK: printf/scanf-like function is called with no format string. - if (format_idx >= TheCall->getNumArgs()) { - Diag(TheCall->getRParenLoc(), diag::warn_missing_format_string) - << Fn->getSourceRange(); + if (format_idx >= NumArgs) { + Diag(Loc, diag::warn_missing_format_string) << range; return; } - const Expr *OrigFormatExpr = TheCall->getArg(format_idx)->IgnoreParenCasts(); + const Expr *OrigFormatExpr = Args[format_idx]->IgnoreParenCasts(); // CHECK: format string is not a string literal. // @@ -1561,18 +1613,18 @@ // 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, TheCall, HasVAListArg, format_idx, - firstDataArg, isPrintf)) + if (SemaCheckStringLiteral(OrigFormatExpr, Args, NumArgs, HasVAListArg, + format_idx, firstDataArg, isPrintf)) return; // Literal format string found, check done! // If there are no arguments specified, warn with -Wformat-security, otherwise // warn only with -Wformat-nonliteral. - if (TheCall->getNumArgs() == format_idx+1) - Diag(TheCall->getArg(format_idx)->getLocStart(), + if (NumArgs == format_idx+1) + Diag(Args[format_idx]->getLocStart(), diag::warn_format_nonliteral_noargs) << OrigFormatExpr->getSourceRange(); else - Diag(TheCall->getArg(format_idx)->getLocStart(), + Diag(Args[format_idx]->getLocStart(), diag::warn_format_nonliteral) << OrigFormatExpr->getSourceRange(); } @@ -1588,7 +1640,8 @@ const bool IsObjCLiteral; const char *Beg; // Start of format string. const bool HasVAListArg; - const CallExpr *TheCall; + const Expr * const *Args; + const unsigned NumArgs; unsigned FormatIdx; llvm::BitVector CoveredArgs; bool usesPositionalArgs; @@ -1599,14 +1652,14 @@ const Expr *origFormatExpr, unsigned firstDataArg, unsigned numDataArgs, bool isObjCLiteral, const char *beg, bool hasVAListArg, - const CallExpr *theCall, unsigned formatIdx, - bool inFunctionCall) + Expr **args, unsigned numArgs, + unsigned formatIdx, bool inFunctionCall) : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr), FirstDataArg(firstDataArg), NumDataArgs(numDataArgs), IsObjCLiteral(isObjCLiteral), Beg(beg), HasVAListArg(hasVAListArg), - TheCall(theCall), FormatIdx(formatIdx), + Args(args), NumArgs(numArgs), FormatIdx(formatIdx), usesPositionalArgs(false), atFirstArg(true), inFunctionCall(inFunctionCall) { CoveredArgs.resize(numDataArgs); @@ -1721,7 +1774,7 @@ } const Expr *CheckFormatHandler::getDataArg(unsigned i) const { - return TheCall->getArg(FirstDataArg + i); + return Args[FirstDataArg + i]; } void CheckFormatHandler::DoneProcessing() { @@ -1805,7 +1858,7 @@ bool IsStringLocation, Range StringRange, FixItHint FixIt) { - EmitFormatDiagnostic(S, inFunctionCall, TheCall->getArg(FormatIdx), PDiag, + EmitFormatDiagnostic(S, inFunctionCall, Args[FormatIdx], PDiag, Loc, IsStringLocation, StringRange, FixIt); } @@ -1864,11 +1917,11 @@ const Expr *origFormatExpr, unsigned firstDataArg, unsigned numDataArgs, bool isObjCLiteral, const char *beg, bool hasVAListArg, - const CallExpr *theCall, unsigned formatIdx, - bool inFunctionCall) + Expr **Args, unsigned NumArgs, + unsigned formatIdx, bool inFunctionCall) : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg, numDataArgs, isObjCLiteral, beg, hasVAListArg, - theCall, formatIdx, inFunctionCall) {} + Args, NumArgs, formatIdx, inFunctionCall) {} bool HandleInvalidPrintfConversionSpecifier( @@ -2188,11 +2241,11 @@ const Expr *origFormatExpr, unsigned firstDataArg, unsigned numDataArgs, bool isObjCLiteral, const char *beg, bool hasVAListArg, - const CallExpr *theCall, unsigned formatIdx, - bool inFunctionCall) + Expr **Args, unsigned NumArgs, + unsigned formatIdx, bool inFunctionCall) : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg, numDataArgs, isObjCLiteral, beg, hasVAListArg, - theCall, formatIdx, inFunctionCall) {} + Args, NumArgs, formatIdx, inFunctionCall) {} bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS, const char *startSpecifier, @@ -2335,14 +2388,15 @@ void Sema::CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr, - const CallExpr *TheCall, bool HasVAListArg, - unsigned format_idx, unsigned firstDataArg, - bool isPrintf, bool inFunctionCall) { + Expr **Args, unsigned NumArgs, + bool HasVAListArg, unsigned format_idx, + unsigned firstDataArg, bool isPrintf, + bool inFunctionCall) { // CHECK: is the format string a wide literal? if (!FExpr->isAscii()) { CheckFormatHandler::EmitFormatDiagnostic( - *this, inFunctionCall, TheCall->getArg(format_idx), + *this, inFunctionCall, Args[format_idx], PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(), /*IsStringLocation*/true, OrigFormatExpr->getSourceRange()); return; @@ -2352,12 +2406,12 @@ StringRef StrRef = FExpr->getString(); const char *Str = StrRef.data(); unsigned StrLen = StrRef.size(); - const unsigned numDataArgs = TheCall->getNumArgs() - firstDataArg; + const unsigned numDataArgs = NumArgs - firstDataArg; // CHECK: empty format string? if (StrLen == 0 && numDataArgs > 0) { CheckFormatHandler::EmitFormatDiagnostic( - *this, inFunctionCall, TheCall->getArg(format_idx), + *this, inFunctionCall, Args[format_idx], PDiag(diag::warn_empty_format_string), FExpr->getLocStart(), /*IsStringLocation*/true, OrigFormatExpr->getSourceRange()); return; @@ -2366,7 +2420,7 @@ if (isPrintf) { CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, numDataArgs, isa(OrigFormatExpr), - Str, HasVAListArg, TheCall, format_idx, + Str, HasVAListArg, Args, NumArgs, format_idx, inFunctionCall); if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen, @@ -2376,7 +2430,7 @@ else { CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, numDataArgs, isa(OrigFormatExpr), - Str, HasVAListArg, TheCall, format_idx, + Str, HasVAListArg, Args, NumArgs, format_idx, inFunctionCall); if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen, Index: test/SemaObjC/format-strings-objc.m =================================================================== --- test/SemaObjC/format-strings-objc.m (revision 148179) +++ test/SemaObjC/format-strings-objc.m (working copy) @@ -63,3 +63,22 @@ printf("%p", y); // no-warning } +// - CFString and NSString formats are ignored +extern void MyNSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2))); +extern void MyCFStringCreateWithFormat(CFStringRef format, ...) __attribute__((format(__CFString__, 1, 2))); + +void check_mylog() { + MyNSLog(@"%@"); // expected-warning {{more '%' conversions than data arguments}} + // FIXME: find a way to test CFString too, but I don't know how to create constant CFString. +} + +// PR 10275 - format function attribute isn't checked in Objective-C methods +@interface Foo ++ (id)fooWithFormat:(NSString *)fmt, ... __attribute__((format(__NSString__, 1, 2))); ++ (id)fooWithCStringFormat:(const char *)format, ... __attribute__((format(__printf__, 1, 2))); +@end + +void check_method() { + [Foo fooWithFormat:@"%@"]; // expected-warning {{more '%' conversions than data arguments}} + [Foo fooWithCStringFormat:"%@"]; // expected-warning {{invalid conversion specifier '@'}} +}