arphaman updated this revision to Diff 81950.
arphaman marked an inline comment as done.
arphaman added a comment.
The updated patch renames the attribute to
`loads_format_specifier_value_using_key` and addresses Aaron's comments
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__((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}}
+}
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: 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: 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<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())
@@ -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<LoadsFormatSpecifierValueUsingKeyAttr>()) {
+ 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);
}
}
}
@@ -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<LoadsFormatSpecifierValueUsingKeyAttr>()) {
+ 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<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();
@@ -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/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,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: 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/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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
