arphaman created this revision.
arphaman added reviewers: aaron.ballman, manmanren.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.
This patch adds a new ``format_dynamic_key_arg``. It's intended to be used
instead of ``format_arg`` for methods/functions that load the formatting string
from some external source based on the given key value. The attribute tells
Clang that it has to avoid ``-Wformat`` warnings when key strings have no
formatting specifiers. Otherwise it tells Clang that it can assume that the key
strings use the same formatting layout as the external formatting string values
(i.e. Clang can perform the normal ``-Wformat`` related checks).
This attribute is useful for certain Objective-C methods like
``NSBundle.localizedStringForKey`` that load the values dynamically based on a
specified key value.
Repository:
rL LLVM
https://reviews.llvm.org/D27165
Files:
include/clang/Analysis/Analyses/FormatString.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/Analysis/PrintfFormatString.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaDeclAttr.cpp
test/Sema/attr-format_arg.c
test/SemaObjC/format-strings-objc.m
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__((format_dynamic_key_arg(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}}
+}
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__((format_dynamic_key_arg(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__((format_dynamic_key_arg("foo"))); // expected-error{{'format_dynamic_key_arg' attribute requires parameter 1 to be an integer constant}}
+const char *formatKeyArgError2(const char *format)
+ __attribute__((format_dynamic_key_arg(0))); // expected-error{{'format_dynamic_key_arg' attribute parameter 1 is out of bounds}}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2750,7 +2750,8 @@
/// 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) {
+static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr,
+ bool IsDynamicKey = false) {
Expr *IdxExpr = Attr.getArgAsExpr(0);
uint64_t Idx;
if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, IdxExpr, Idx))
@@ -2786,6 +2787,12 @@
llvm::APSInt Val;
IdxExpr->EvaluateAsInt(Val, S.Context);
+ if (IsDynamicKey) {
+ D->addAttr(::new (S.Context) FormatDynamicKeyArgAttr(
+ Attr.getRange(), S.Context, Val.getZExtValue(),
+ Attr.getAttributeSpellingListIndex()));
+ return;
+ }
D->addAttr(::new (S.Context)
FormatArgAttr(Attr.getRange(), S.Context, Val.getZExtValue(),
Attr.getAttributeSpellingListIndex()));
@@ -5621,6 +5628,9 @@
case AttributeList::AT_FormatArg:
handleFormatArgAttr(S, D, Attr);
break;
+ case AttributeList::AT_FormatDynamicKeyArg:
+ handleFormatArgAttr(S, D, Attr, /*IsDynamicKey=*/true);
+ break;
case AttributeList::AT_CUDAGlobal:
handleGlobalAttr(S, D, Attr);
break;
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -4401,30 +4401,24 @@
};
} // end anonymous namespace
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
- const Expr *OrigFormatExpr,
- ArrayRef<const Expr *> 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<const Expr *> 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<const Expr *> 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<const Expr *> 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())
@@ -4468,20 +4462,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;
}
@@ -4531,11 +4524,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);
}
}
@@ -4587,20 +4580,29 @@
--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<FormatDynamicKeyArgAttr>()) {
+ unsigned ArgIndex = FKA->getFormatIdx();
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(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<FunctionDecl>(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);
}
}
}
@@ -4615,7 +4617,15 @@
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<FormatDynamicKeyArgAttr>()) {
+ 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);
}
}
@@ -4639,7 +4649,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;
}
@@ -6283,25 +6294,12 @@
return true;
}
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
- const Expr *OrigFormatExpr,
- ArrayRef<const Expr *> 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<const Expr *> 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();
@@ -6313,6 +6311,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/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: include/clang/Basic/AttrDocs.td
===================================================================
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -1751,6 +1751,20 @@
}];
}
+def FormatDynamicKeyArgDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``format_dynamic_key_arg`` attribute indicates that a function accepts a
+key string that corresponds to some external ``printf`` or ``scanf``-like format
+string and returns that format string.
+
+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: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -858,6 +858,14 @@
let Documentation = [Undocumented];
}
+def FormatDynamicKeyArg : InheritableAttr {
+ let Spellings = [GCC<"format_dynamic_key_arg">];
+ let Args = [IntArgument<"FormatIdx">];
+ let Subjects = SubjectList<[ObjCMethod, HasFunctionProto], WarnDiag,
+ "ExpectedFunctionWithProtoType">;
+ let Documentation = [FormatDynamicKeyArgDocs];
+}
+
def GNUInline : InheritableAttr {
let Spellings = [GCC<"gnu_inline">];
let Subjects = SubjectList<[Function]>;
Index: include/clang/Analysis/Analyses/FormatString.h
===================================================================
--- include/clang/Analysis/Analyses/FormatString.h
+++ include/clang/Analysis/Analyses/FormatString.h
@@ -685,6 +685,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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
