r368878 - [Sema][ObjC] Fix a -Wformat false positive with localizedStringForKey
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 14 09:57:11 PDT 2019
Author: epilk
Date: Wed Aug 14 09:57:11 2019
New Revision: 368878
URL: http://llvm.org/viewvc/llvm-project?rev=368878&view=rev
Log:
[Sema][ObjC] Fix a -Wformat false positive with localizedStringForKey
Only honour format_arg attributes on -[NSBundle localizedStringForKey] when its
argument has a format specifier in it, otherwise its likely to just be a key to
fetch localized strings.
Fixes rdar://23622446
Differential revision: https://reviews.llvm.org/D27165
Modified:
cfe/trunk/include/clang/AST/FormatString.h
cfe/trunk/include/clang/Basic/IdentifierTable.h
cfe/trunk/lib/AST/PrintfFormatString.cpp
cfe/trunk/lib/Basic/IdentifierTable.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaObjC/format-strings-objc.m
Modified: cfe/trunk/include/clang/AST/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/FormatString.h?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/FormatString.h (original)
+++ cfe/trunk/include/clang/AST/FormatString.h Wed Aug 14 09:57:11 2019
@@ -748,6 +748,12 @@ bool ParseScanfString(FormatStringHandle
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);
+
} // end analyze_format_string namespace
} // end clang namespace
#endif
Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h Wed Aug 14 09:57:11 2019
@@ -750,6 +750,12 @@ public:
return getIdentifierInfoFlag() == ZeroArg;
}
+ /// If this selector is the specific keyword selector described by Names.
+ bool isKeywordSelector(ArrayRef<StringRef> Names) const;
+
+ /// If this selector is the specific unary selector described by Name.
+ bool isUnarySelector(StringRef Name) const;
+
unsigned getNumArgs() const;
/// Retrieve the identifier at a given position in the selector.
Modified: cfe/trunk/lib/AST/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/PrintfFormatString.cpp?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/lib/AST/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/AST/PrintfFormatString.cpp Wed Aug 14 09:57:11 2019
@@ -463,6 +463,23 @@ bool clang::analyze_format_string::Parse
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.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/IdentifierTable.cpp (original)
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp Wed Aug 14 09:57:11 2019
@@ -412,6 +412,21 @@ public:
} // namespace clang.
+bool Selector::isKeywordSelector(ArrayRef<StringRef> Names) const {
+ assert(!Names.empty() && "must have >= 1 selector slots");
+ if (getNumArgs() != Names.size())
+ return false;
+ for (unsigned I = 0, E = Names.size(); I != E; ++I) {
+ if (getNameForSlot(I) != Names[I])
+ return false;
+ }
+ return true;
+}
+
+bool Selector::isUnarySelector(StringRef Name) const {
+ return isUnarySelector() && getNameForSlot(0) == Name;
+}
+
unsigned Selector::getNumArgs() const {
unsigned IIF = getIdentifierInfoFlag();
if (IIF <= ZeroArg)
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Aug 14 09:57:11 2019
@@ -6575,7 +6575,8 @@ static void CheckFormatString(Sema &S, c
bool inFunctionCall,
Sema::VariadicCallType CallType,
llvm::SmallBitVector &CheckedVarArgs,
- UncoveredArgHandler &UncoveredArg);
+ 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
@@ -6588,7 +6589,8 @@ checkFormatStringExpr(Sema &S, const Exp
Sema::VariadicCallType CallType, bool InFunctionCall,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg,
- llvm::APSInt Offset) {
+ llvm::APSInt Offset,
+ bool IgnoreStringsWithoutSpecifiers = false) {
if (S.isConstantEvaluated())
return SLCT_NotALiteral;
tryAgain:
@@ -6639,17 +6641,17 @@ checkFormatStringExpr(Sema &S, const Exp
Left = checkFormatStringExpr(S, C->getTrueExpr(), Args,
HasVAListArg, format_idx, firstDataArg,
Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
+ 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;
}
@@ -6753,7 +6755,8 @@ checkFormatStringExpr(Sema &S, const Exp
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
StringLiteralCheckType Result = checkFormatStringExpr(
S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
- CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+ CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ IgnoreStringsWithoutSpecifiers);
if (IsFirst) {
CommonResult = Result;
IsFirst = false;
@@ -6771,7 +6774,8 @@ checkFormatStringExpr(Sema &S, const Exp
HasVAListArg, format_idx,
firstDataArg, Type, CallType,
InFunctionCall, CheckedVarArgs,
- UncoveredArg, Offset);
+ UncoveredArg, Offset,
+ IgnoreStringsWithoutSpecifiers);
}
}
}
@@ -6780,12 +6784,28 @@ checkFormatStringExpr(Sema &S, const Exp
}
case Stmt::ObjCMessageExprClass: {
const auto *ME = cast<ObjCMessageExpr>(E);
- if (const auto *ND = ME->getMethodDecl()) {
- if (const auto *FA = ND->getAttr<FormatArgAttr>()) {
+ if (const auto *MD = ME->getMethodDecl()) {
+ if (const auto *FA = MD->getAttr<FormatArgAttr>()) {
+ // As a special case heuristic, if we're using the method -[NSBundle
+ // localizedStringForKey:value:table:], ignore any key strings that lack
+ // format specifiers. The idea is that if the key doesn't have any
+ // format specifiers then its probably just a key to map to the
+ // localized strings. If it does have format specifiers though, then its
+ // likely that the text of the key is the format string in the
+ // programmer's language, and should be checked.
+ const ObjCInterfaceDecl *IFace;
+ if (MD->isInstanceMethod() && (IFace = MD->getClassInterface()) &&
+ IFace->getIdentifier()->isStr("NSBundle") &&
+ MD->getSelector().isKeywordSelector(
+ {"localizedStringForKey", "value", "table"})) {
+ IgnoreStringsWithoutSpecifiers = true;
+ }
+
const Expr *Arg = ME->getArg(FA->getFormatIdx().getASTIndex());
return checkFormatStringExpr(
S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
- CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+ CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ IgnoreStringsWithoutSpecifiers);
}
}
@@ -6809,7 +6829,8 @@ checkFormatStringExpr(Sema &S, const Exp
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;
}
@@ -8500,7 +8521,8 @@ static void CheckFormatString(Sema &S, c
bool inFunctionCall,
Sema::VariadicCallType CallType,
llvm::SmallBitVector &CheckedVarArgs,
- UncoveredArgHandler &UncoveredArg) {
+ UncoveredArgHandler &UncoveredArg,
+ bool IgnoreStringsWithoutSpecifiers) {
// CHECK: is the format string a wide literal?
if (!FExpr->isAscii() && !FExpr->isUTF8()) {
CheckFormatHandler::EmitFormatDiagnostic(
@@ -8521,6 +8543,11 @@ static void CheckFormatString(Sema &S, c
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;
+
// Emit a warning if the string literal is truncated and does not contain an
// embedded null character.
if (TypeSize <= StrRef.size() &&
Modified: cfe/trunk/test/SemaObjC/format-strings-objc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc.m?rev=368878&r1=368877&r2=368878&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
+++ cfe/trunk/test/SemaObjC/format-strings-objc.m Wed Aug 14 09:57:11 2019
@@ -25,7 +25,11 @@ typedef struct _NSZone NSZone;
@protocol NSCoding - (void)encodeWithCoder:(NSCoder *)aCoder; @end
@interface NSObject <NSObject> {} @end
typedef float CGFloat;
- at interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding> - (NSUInteger)length; @end
+ at interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>
+- (NSUInteger)length;
++(instancetype)stringWithFormat:(NSString *)fmt, ...
+ __attribute__((format(__NSString__, 1, 2)));
+ at end
@interface NSSimpleCString : NSString {} @end
@interface NSConstantString : NSSimpleCString @end
extern void *_NSConstantStringClassReference;
@@ -302,3 +306,39 @@ const char *rd23622446(const char *forma
}
@end
+
+ at interface NSBundle : NSObject
+- (NSString *)localizedStringForKey:(NSString *)key
+ value:(nullable NSString *)value
+ table:(nullable NSString *)tableName
+ __attribute__((format_arg(1)));
+
+- (NSString *)someRandomMethod:(NSString *)key
+ value:(nullable NSString *)value
+ table:(nullable NSString *)tableName
+ __attribute__((format_arg(1)));
+ at end
+
+void useLocalizedStringForKey(NSBundle *bndl) {
+ [NSString stringWithFormat:
+ [bndl localizedStringForKey:@"%d" // expected-warning{{more '%' conversions than data arguments}}
+ value:0
+ table:0]];
+ // No warning, @"flerp" doesn't have a format specifier.
+ [NSString stringWithFormat: [bndl localizedStringForKey:@"flerp" value:0 table:0], 43, @"flarp"];
+
+ [NSString stringWithFormat:
+ [bndl localizedStringForKey:@"%f"
+ value:0
+ table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+ [NSString stringWithFormat:
+ [bndl someRandomMethod:@"%f"
+ value:0
+ table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+ [NSString stringWithFormat:
+ [bndl someRandomMethod:@"flerp"
+ value:0
+ table:0], 42]; // expected-warning{{data argument not used by format string}}
+}
More information about the cfe-commits
mailing list