Index: include/clang/Analysis/Analyses/FormatString.h =================================================================== --- include/clang/Analysis/Analyses/FormatString.h +++ include/clang/Analysis/Analyses/FormatString.h @@ -687,6 +687,12 @@ bool ParseFormatStringHasSArg(const char *beg, const char *end, const LangOptions &LO, const TargetInfo &Target); +/// Return true if the given string has at least one formatting specifier. +bool parseFormatStringHasFormattingSpecifiers(const char *Begin, + const char *End, + const LangOptions &LO, + const TargetInfo &Target); + bool ParseScanfString(FormatStringHandler &H, const char *beg, const char *end, const LangOptions &LO, const TargetInfo &Target); Index: include/clang/Basic/Attr.td =================================================================== --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -858,6 +858,14 @@ let Documentation = [Undocumented]; } +def LoadsFormatSpecifierValueUsingKey : InheritableAttr { + let Spellings = [GNU<"loads_format_specifier_value_using_key">]; + let Args = [IntArgument<"FormatIdx">]; + let Subjects = SubjectList<[ObjCMethod, HasFunctionProto], WarnDiag, + "ExpectedFunctionWithProtoType">; + let Documentation = [LoadsFormatSpecifierValueUsingKeyDocs]; +} + def GNUInline : InheritableAttr { let Spellings = [GCC<"gnu_inline">]; let Subjects = SubjectList<[Function]>; Index: include/clang/Basic/AttrDocs.td =================================================================== --- include/clang/Basic/AttrDocs.td +++ include/clang/Basic/AttrDocs.td @@ -1751,6 +1751,21 @@ }]; } +def LoadsFormatSpecifierValueUsingKeyDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +The ``loads_format_specifier_value_using_key`` attribute indicates that a +function accepts a key string that corresponds to some external ``printf`` or +``scanf``-like format string value, loads the value that corresponds to the +given key and returns that value. + +If a key string has formatting specifiers, Clang assumes that its formatting +layout corresponds to the formatting layout that's used in the external string +value, and performs ``-Wformat`` warning related checks using the formatting +specifiers found in the key string. + }]; +} + def AlignValueDocs : Documentation { let Category = DocCatType; let Content = [{ Index: lib/Analysis/PrintfFormatString.cpp =================================================================== --- lib/Analysis/PrintfFormatString.cpp +++ lib/Analysis/PrintfFormatString.cpp @@ -420,6 +420,23 @@ return false; } +bool clang::analyze_format_string::parseFormatStringHasFormattingSpecifiers( + const char *Begin, const char *End, const LangOptions &LO, + const TargetInfo &Target) { + unsigned ArgIndex = 0; + // Keep looking for a formatting specifier until we have exhausted the string. + FormatStringHandler H; + while (Begin != End) { + const PrintfSpecifierResult &FSR = + ParsePrintfSpecifier(H, Begin, End, ArgIndex, LO, Target, false, false); + if (FSR.shouldStop()) + break; + if (FSR.hasValue()) + return true; + } + return false; +} + //===----------------------------------------------------------------------===// // Methods on PrintfSpecifier. //===----------------------------------------------------------------------===// Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -4404,30 +4404,24 @@ }; } // end anonymous namespace -static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr, - const Expr *OrigFormatExpr, - ArrayRef Args, - bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, - Sema::FormatStringType Type, - bool inFunctionCall, - Sema::VariadicCallType CallType, - llvm::SmallBitVector &CheckedVarArgs, - UncoveredArgHandler &UncoveredArg); +static void CheckFormatString( + Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr, + ArrayRef Args, bool HasVAListArg, unsigned format_idx, + unsigned firstDataArg, Sema::FormatStringType Type, bool inFunctionCall, + Sema::VariadicCallType CallType, llvm::SmallBitVector &CheckedVarArgs, + UncoveredArgHandler &UncoveredArg, bool IgnoreStringsWithoutSpecifiers); // 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. -static StringLiteralCheckType -checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, - bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, Sema::FormatStringType Type, - Sema::VariadicCallType CallType, bool InFunctionCall, - llvm::SmallBitVector &CheckedVarArgs, - UncoveredArgHandler &UncoveredArg, - llvm::APSInt Offset) { - tryAgain: +static StringLiteralCheckType checkFormatStringExpr( + Sema &S, const Expr *E, ArrayRef Args, bool HasVAListArg, + unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type, + Sema::VariadicCallType CallType, bool InFunctionCall, + llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg, + llvm::APSInt Offset, bool IgnoreStringsWithoutSpecifiers = false) { +tryAgain: assert(Offset.isSigned() && "invalid offset"); if (E->isTypeDependent() || E->isValueDependent()) @@ -4471,20 +4465,19 @@ if (!CheckLeft) Left = SLCT_UncheckedLiteral; else { - Left = checkFormatStringExpr(S, C->getTrueExpr(), Args, - HasVAListArg, format_idx, firstDataArg, - Type, CallType, InFunctionCall, - CheckedVarArgs, UncoveredArg, Offset); + Left = checkFormatStringExpr(S, C->getTrueExpr(), Args, HasVAListArg, + format_idx, firstDataArg, Type, CallType, + InFunctionCall, CheckedVarArgs, UncoveredArg, + Offset, IgnoreStringsWithoutSpecifiers); if (Left == SLCT_NotALiteral || !CheckRight) { return Left; } } - StringLiteralCheckType Right = - checkFormatStringExpr(S, C->getFalseExpr(), Args, - HasVAListArg, format_idx, firstDataArg, - Type, CallType, InFunctionCall, CheckedVarArgs, - UncoveredArg, Offset); + StringLiteralCheckType Right = checkFormatStringExpr( + S, C->getFalseExpr(), Args, HasVAListArg, format_idx, firstDataArg, + Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + IgnoreStringsWithoutSpecifiers); return (CheckLeft && Left < Right) ? Left : Right; } @@ -4534,11 +4527,11 @@ if (InitList->isStringLiteralInit()) Init = InitList->getInit(0)->IgnoreParenImpCasts(); } - return checkFormatStringExpr(S, Init, Args, - HasVAListArg, format_idx, + return checkFormatStringExpr(S, Init, Args, HasVAListArg, format_idx, firstDataArg, Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs, - UncoveredArg, Offset); + UncoveredArg, Offset, + IgnoreStringsWithoutSpecifiers); } } @@ -4590,20 +4583,30 @@ --ArgIndex; const Expr *Arg = CE->getArg(ArgIndex - 1); - return checkFormatStringExpr(S, Arg, Args, - HasVAListArg, format_idx, firstDataArg, - Type, CallType, InFunctionCall, - CheckedVarArgs, UncoveredArg, Offset); + return checkFormatStringExpr( + S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type, + CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + IgnoreStringsWithoutSpecifiers); + } else if (const auto *FKA = + ND->getAttr()) { + unsigned ArgIndex = FKA->getFormatIdx(); + if (const CXXMethodDecl *MD = dyn_cast(ND)) + if (MD->isInstance()) + --ArgIndex; + const Expr *Arg = CE->getArg(ArgIndex - 1); + return checkFormatStringExpr( + S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type, + CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + /*IgnoreStringsWithoutSpecifiers=*/true); } else if (const FunctionDecl *FD = dyn_cast(ND)) { unsigned BuiltinID = FD->getBuiltinID(); if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString || BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) { const Expr *Arg = CE->getArg(0); - return checkFormatStringExpr(S, Arg, Args, - HasVAListArg, format_idx, - firstDataArg, Type, CallType, - InFunctionCall, CheckedVarArgs, - UncoveredArg, Offset); + return checkFormatStringExpr( + S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type, + CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + IgnoreStringsWithoutSpecifiers); } } } @@ -4618,7 +4621,16 @@ const Expr *Arg = ME->getArg(ArgIndex - 1); return checkFormatStringExpr( S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type, - CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset); + CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + IgnoreStringsWithoutSpecifiers); + } else if (const auto *FKA = + ND->getAttr()) { + unsigned ArgIndex = FKA->getFormatIdx(); + const Expr *Arg = ME->getArg(ArgIndex - 1); + return checkFormatStringExpr( + S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type, + CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, + /*IgnoreStringsWithoutSpecifiers=*/true); } } @@ -4642,7 +4654,8 @@ FormatStringLiteral FStr(StrE, Offset.sextOrTrunc(64).getSExtValue()); CheckFormatString(S, &FStr, E, Args, HasVAListArg, format_idx, firstDataArg, Type, InFunctionCall, CallType, - CheckedVarArgs, UncoveredArg); + CheckedVarArgs, UncoveredArg, + IgnoreStringsWithoutSpecifiers); return SLCT_CheckedLiteral; } @@ -6286,25 +6299,12 @@ return true; } -static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr, - const Expr *OrigFormatExpr, - ArrayRef Args, - bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, - Sema::FormatStringType Type, - bool inFunctionCall, - Sema::VariadicCallType CallType, - llvm::SmallBitVector &CheckedVarArgs, - UncoveredArgHandler &UncoveredArg) { - // CHECK: is the format string a wide literal? - if (!FExpr->isAscii() && !FExpr->isUTF8()) { - CheckFormatHandler::EmitFormatDiagnostic( - S, inFunctionCall, Args[format_idx], - S.PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(), - /*IsStringLocation*/true, OrigFormatExpr->getSourceRange()); - return; - } - +static void CheckFormatString( + Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr, + ArrayRef Args, bool HasVAListArg, unsigned format_idx, + unsigned firstDataArg, Sema::FormatStringType Type, bool inFunctionCall, + Sema::VariadicCallType CallType, llvm::SmallBitVector &CheckedVarArgs, + UncoveredArgHandler &UncoveredArg, bool IgnoreStringsWithoutSpecifiers) { // Str - The format string. NOTE: this is NOT null-terminated! StringRef StrRef = FExpr->getString(); const char *Str = StrRef.data(); @@ -6316,6 +6316,20 @@ size_t StrLen = std::min(std::max(TypeSize, size_t(1)) - 1, StrRef.size()); const unsigned numDataArgs = Args.size() - firstDataArg; + if (IgnoreStringsWithoutSpecifiers && + !analyze_format_string::parseFormatStringHasFormattingSpecifiers( + Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo())) + return; + + // CHECK: is the format string a wide literal? + if (!FExpr->isAscii() && !FExpr->isUTF8()) { + CheckFormatHandler::EmitFormatDiagnostic( + S, inFunctionCall, Args[format_idx], + S.PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(), + /*IsStringLocation*/ true, OrigFormatExpr->getSourceRange()); + return; + } + // Emit a warning if the string literal is truncated and does not contain an // embedded null character. if (TypeSize <= StrRef.size() && Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -2748,13 +2748,14 @@ Attr.getAttributeSpellingListIndex())); } -/// Handle __attribute__((format_arg((idx)))) attribute based on -/// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html -static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr) { +/// Checks a format_arg or loads_format_specifier_value_using_key attribute +/// and returns true if it has any errors. +static bool checkFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr, + llvm::APSInt &Val) { Expr *IdxExpr = Attr.getArgAsExpr(0); uint64_t Idx; if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, IdxExpr, Idx)) - return; + return true; // Make sure the format string is really a string. QualType Ty = getFunctionOrMethodParamType(D, Idx); @@ -2767,7 +2768,7 @@ S.Diag(Attr.getLoc(), diag::err_format_attribute_not) << "a string type" << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0); - return; + return true; } Ty = getFunctionOrMethodResultType(D); if (!isNSStringType(Ty, S.Context) && @@ -2777,20 +2778,38 @@ S.Diag(Attr.getLoc(), diag::err_format_attribute_result_not) << (NotNSStringTy ? "string type" : "NSString") << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0); - return; + return true; } // We cannot use the Idx returned from checkFunctionOrMethodParameterIndex // because that has corrected for the implicit this parameter, and is zero- // based. The attribute expects what the user wrote explicitly. - llvm::APSInt Val; IdxExpr->EvaluateAsInt(Val, S.Context); + return false; +} +/// Handle __attribute__((format_arg((idx)))) attribute based on +/// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html +static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr) { + llvm::APSInt Val; + if (checkFormatArgAttr(S, D, Attr, Val)) + return; D->addAttr(::new (S.Context) FormatArgAttr(Attr.getRange(), S.Context, Val.getZExtValue(), Attr.getAttributeSpellingListIndex())); } +static void +handleLoadsFormatSpecifierValueUsingKeyAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + llvm::APSInt Val; + if (checkFormatArgAttr(S, D, Attr, Val)) + return; + D->addAttr(::new (S.Context) LoadsFormatSpecifierValueUsingKeyAttr( + Attr.getRange(), S.Context, Val.getZExtValue(), + Attr.getAttributeSpellingListIndex())); +} + enum FormatAttrKind { CFStringFormat, NSStringFormat, @@ -5615,6 +5634,9 @@ case AttributeList::AT_FormatArg: handleFormatArgAttr(S, D, Attr); break; + case AttributeList::AT_LoadsFormatSpecifierValueUsingKey: + handleLoadsFormatSpecifierValueUsingKeyAttr(S, D, Attr); + break; case AttributeList::AT_CUDAGlobal: handleGlobalAttr(S, D, Attr); break; Index: test/Sema/attr-format_arg.c =================================================================== --- test/Sema/attr-format_arg.c +++ test/Sema/attr-format_arg.c @@ -11,3 +11,19 @@ printf(f("%d"), 123); printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}} } + +const char *loadFormattingValue(const char *key) __attribute__((loads_format_specifier_value_using_key(1))); + +void testFormatDynamicKeyArg() { + // Don't warn when the key has no formatting specifiers + printf(loadFormattingValue("key"), "foo"); + + // Warn normally when the key has formatting specifiers + printf(loadFormattingValue("%d"), 123); + printf(loadFormattingValue("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}} +} + +const char *formatKeyArgError1(const char *format) + __attribute__((loads_format_specifier_value_using_key("foo"))); // expected-error{{'loads_format_specifier_value_using_key' attribute requires parameter 1 to be an integer constant}} +const char *formatKeyArgError2(const char *format) + __attribute__((loads_format_specifier_value_using_key(0))); // expected-error{{'loads_format_specifier_value_using_key' attribute parameter 1 is out of bounds}} Index: test/SemaObjC/format-strings-objc.m =================================================================== --- test/SemaObjC/format-strings-objc.m +++ test/SemaObjC/format-strings-objc.m @@ -302,3 +302,21 @@ } @end + +@interface FormatDynamicKeyArg + +- (NSString *)load:(NSString *)key __attribute__((loads_format_specifier_value_using_key(1))); + +@end + +void testFormatDynamicKeyArg(FormatDynamicKeyArg *m) { + // Don't warn when the key has no formatting specifiers + NSLog([m load:@"key"], @"foo"); + NSLog([m load:@""]); + + // Warn normally when the key has formatting specifiers + NSLog([m load:@"correct %d"], 2); + NSLog([m load:@"warn %d"], "off"); // expected-warning {{format specifies type 'int' but the argument has type 'char *'}} + NSLog([m load:@"%@ %@"], @"name"); // expected-warning {{more '%' conversions than data arguments}} + NSLog([m load:@"Warn again %@"], @"name", @"void"); // expected-warning {{data argument not used by format string}} +}