maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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

llvm-svn: 368878
This commit is contained in:
Erik Pilkington 2019-08-14 16:57:11 +00:00
parent 4ae5efbe66
commit aa3855694f
6 changed files with 127 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clang/include/clang/AST/FormatString.h
View File

@ -748,6 +748,12 @@ bool ParseScanfString(FormatStringHandler &H,
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

6
clang/include/clang/Basic/IdentifierTable.h
View File

@ -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.

17
clang/lib/AST/PrintfFormatString.cpp
View File

@ -463,6 +463,23 @@ bool clang::analyze_format_string::ParseFormatStringHasSArg(const char *I,
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.
//===----------------------------------------------------------------------===//

15
clang/lib/Basic/IdentifierTable.cpp
View File

@ -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)

57
clang/lib/Sema/SemaChecking.cpp
View File

@ -6575,7 +6575,8 @@ static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
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 Expr *E, ArrayRef<const Expr *> Args,
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 Expr *E, ArrayRef<const Expr *> Args,
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 Expr *E, ArrayRef<const Expr *> Args,
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 Expr *E, ArrayRef<const Expr *> Args,
HasVAListArg, format_idx,
firstDataArg, Type, CallType,
InFunctionCall, CheckedVarArgs,
UncoveredArg, Offset);
UncoveredArg, Offset,
IgnoreStringsWithoutSpecifiers);
}
}
}
@ -6780,12 +6784,28 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
}
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 Expr *E, ArrayRef<const Expr *> Args,
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, const FormatStringLiteral *FExpr,
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, const FormatStringLiteral *FExpr,
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() &&

42
clang/test/SemaObjC/format-strings-objc.m
View File

@ -25,7 +25,11 @@ typedef struct _NSZone NSZone;
@protocol NSCoding - (void)encodeWithCoder:(NSCoder *)aCoder; @end
@interface NSObject <NSObject> {} @end
typedef float CGFloat;
@interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding> - (NSUInteger)length; @end
@interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>
- (NSUInteger)length;
+(instancetype)stringWithFormat:(NSString *)fmt, ...
__attribute__((format(__NSString__, 1, 2)));
@end
@interface NSSimpleCString : NSString {} @end
@interface NSConstantString : NSSimpleCString @end
extern void *_NSConstantStringClassReference;
@ -302,3 +306,39 @@ const char *rd23622446(const char *format) {
}
@end
@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)));
@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}}
}