From a5a035243b641e331bf22360a31fa1539bb2d61f Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 9 Apr 2014 01:39:22 +0000 Subject: [PATCH] [analyzer] When checking Foundation method calls, match the selectors exactly. This also includes some infrastructure to make it easier to build multi-argument selectors, rather than trying to use string matching on each piece. There's a bit more setup code, but less cost at runtime. PR18908 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205827 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/BasicObjCFoundationChecks.cpp | 114 +++++++++++------- .../Checkers/NoReturnFunctionChecker.cpp | 33 ++--- .../Checkers/RetainCountChecker.cpp | 16 +-- lib/StaticAnalyzer/Checkers/SelectorExtras.h | 67 ++++++++++ test/Analysis/NSContainers.m | 8 ++ 5 files changed, 162 insertions(+), 76 deletions(-) create mode 100644 lib/StaticAnalyzer/Checkers/SelectorExtras.h diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 4b2ccd43aa..44aa43f998 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -14,6 +14,7 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "SelectorExtras.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -97,6 +98,18 @@ namespace { check::PostStmt > { mutable std::unique_ptr BT; + mutable llvm::SmallDenseMap StringSelectors; + mutable Selector ArrayWithObjectSel; + mutable Selector AddObjectSel; + mutable Selector InsertObjectAtIndexSel; + mutable Selector ReplaceObjectAtIndexWithObjectSel; + mutable Selector SetObjectAtIndexedSubscriptSel; + mutable Selector ArrayByAddingObjectSel; + mutable Selector DictionaryWithObjectForKeySel; + mutable Selector SetObjectForKeySel; + mutable Selector SetObjectForKeyedSubscriptSel; + mutable Selector RemoveObjectForKeySel; + void warnIfNilExpr(const Expr *E, const char *Msg, CheckerContext &C) const; @@ -214,50 +227,62 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, if (Class == FC_NSString) { Selector S = msg.getSelector(); - + if (S.isUnarySelector()) return; - - // FIXME: This is going to be really slow doing these checks with - // lexical comparisons. - - std::string NameStr = S.getAsString(); - StringRef Name(NameStr); - assert(!Name.empty()); - - // FIXME: Checking for initWithFormat: will not work in most cases - // yet because [NSString alloc] returns id, not NSString*. We will - // need support for tracking expected-type information in the analyzer - // to find these errors. - if (Name == "caseInsensitiveCompare:" || - Name == "compare:" || - Name == "compare:options:" || - Name == "compare:options:range:" || - Name == "compare:options:range:locale:" || - Name == "componentsSeparatedByCharactersInSet:" || - Name == "initWithFormat:") { - Arg = 0; + + if (StringSelectors.empty()) { + ASTContext &Ctx = C.getASTContext(); + Selector Sels[] = { + getKeywordSelector(Ctx, "caseInsensitiveCompare", nullptr), + getKeywordSelector(Ctx, "compare", nullptr), + getKeywordSelector(Ctx, "compare", "options", nullptr), + getKeywordSelector(Ctx, "compare", "options", "range", nullptr), + getKeywordSelector(Ctx, "compare", "options", "range", "locale", + nullptr), + getKeywordSelector(Ctx, "componentsSeparatedByCharactersInSet", + nullptr), + getKeywordSelector(Ctx, "initWithFormat", + nullptr), + getKeywordSelector(Ctx, "localizedCaseInsensitiveCompare", nullptr), + getKeywordSelector(Ctx, "localizedCompare", nullptr), + getKeywordSelector(Ctx, "localizedStandardCompare", nullptr), + }; + for (Selector KnownSel : Sels) + StringSelectors[KnownSel] = 0; } + auto I = StringSelectors.find(S); + if (I == StringSelectors.end()) + return; + Arg = I->second; } else if (Class == FC_NSArray) { Selector S = msg.getSelector(); if (S.isUnarySelector()) return; - if (S.getNameForSlot(0).equals("addObject")) { - Arg = 0; - } else if (S.getNameForSlot(0).equals("insertObject") && - S.getNameForSlot(1).equals("atIndex")) { + if (ArrayWithObjectSel.isNull()) { + ASTContext &Ctx = C.getASTContext(); + ArrayWithObjectSel = getKeywordSelector(Ctx, "arrayWithObject", nullptr); + AddObjectSel = getKeywordSelector(Ctx, "addObject", nullptr); + InsertObjectAtIndexSel = + getKeywordSelector(Ctx, "insertObject", "atIndex", nullptr); + ReplaceObjectAtIndexWithObjectSel = + getKeywordSelector(Ctx, "replaceObjectAtIndex", "withObject", nullptr); + SetObjectAtIndexedSubscriptSel = + getKeywordSelector(Ctx, "setObject", "atIndexedSubscript", nullptr); + ArrayByAddingObjectSel = + getKeywordSelector(Ctx, "arrayByAddingObject", nullptr); + } + + if (S == ArrayWithObjectSel || S == AddObjectSel || + S == InsertObjectAtIndexSel || S == ArrayByAddingObjectSel) { Arg = 0; - } else if (S.getNameForSlot(0).equals("replaceObjectAtIndex") && - S.getNameForSlot(1).equals("withObject")) { - Arg = 1; - } else if (S.getNameForSlot(0).equals("setObject") && - S.getNameForSlot(1).equals("atIndexedSubscript")) { + } else if (S == SetObjectAtIndexedSubscriptSel) { Arg = 0; CanBeSubscript = true; - } else if (S.getNameForSlot(0).equals("arrayByAddingObject")) { - Arg = 0; + } else if (S == ReplaceObjectAtIndexWithObjectSel) { + Arg = 1; } } else if (Class == FC_NSDictionary) { Selector S = msg.getSelector(); @@ -265,20 +290,26 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, if (S.isUnarySelector()) return; - if (S.getNameForSlot(0).equals("dictionaryWithObject") && - S.getNameForSlot(1).equals("forKey")) { - Arg = 0; - warnIfNilArg(C, msg, /* Arg */1, Class); - } else if (S.getNameForSlot(0).equals("setObject") && - S.getNameForSlot(1).equals("forKey")) { + if (DictionaryWithObjectForKeySel.isNull()) { + ASTContext &Ctx = C.getASTContext(); + DictionaryWithObjectForKeySel = + getKeywordSelector(Ctx, "dictionaryWithObject", "forKey", nullptr); + SetObjectForKeySel = + getKeywordSelector(Ctx, "setObject", "forKey", nullptr); + SetObjectForKeyedSubscriptSel = + getKeywordSelector(Ctx, "setObject", "forKeyedSubscript", nullptr); + RemoveObjectForKeySel = + getKeywordSelector(Ctx, "removeObjectForKey", nullptr); + } + + if (S == DictionaryWithObjectForKeySel || S == SetObjectForKeySel) { Arg = 0; warnIfNilArg(C, msg, /* Arg */1, Class); - } else if (S.getNameForSlot(0).equals("setObject") && - S.getNameForSlot(1).equals("forKeyedSubscript")) { + } else if (S == SetObjectForKeyedSubscriptSel) { CanBeSubscript = true; Arg = 0; warnIfNilArg(C, msg, /* Arg */1, Class, CanBeSubscript); - } else if (S.getNameForSlot(0).equals("removeObjectForKey")) { + } else if (S == RemoveObjectForKeySel) { Arg = 0; } } @@ -286,7 +317,6 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, // If argument is '0', report a warning. if ((Arg != InvalidArgIndex)) warnIfNilArg(C, msg, Arg, Class, CanBeSubscript); - } void NilArgChecker::checkPostStmt(const ObjCArrayLiteral *AL, diff --git a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp index 0e9cbba350..ba82d1d1d4 100644 --- a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "SelectorExtras.h" #include "clang/AST/Attr.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -28,6 +29,8 @@ namespace { class NoReturnFunctionChecker : public Checker< check::PostCall, check::PostObjCMessage > { + mutable Selector HandleFailureInFunctionSel; + mutable Selector HandleFailureInMethodSel; public: void checkPostCall(const CallEvent &CE, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; @@ -81,24 +84,6 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE, C.generateSink(); } -static bool END_WITH_NULL isMultiArgSelector(const Selector *Sel, ...) { - va_list argp; - va_start(argp, Sel); - - unsigned Slot = 0; - const char *Arg; - while ((Arg = va_arg(argp, const char *))) { - if (!Sel->getNameForSlot(Slot).equals(Arg)) - break; // still need to va_end! - ++Slot; - } - - va_end(argp); - - // We only succeeded if we made it to the end of the argument list. - return (Arg == NULL); -} - void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, CheckerContext &C) const { // Check if the method is annotated with analyzer_noreturn. @@ -135,13 +120,17 @@ void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, default: return; case 4: - if (!isMultiArgSelector(&Sel, "handleFailureInFunction", "file", - "lineNumber", "description", NULL)) + lazyInitKeywordSelector(HandleFailureInFunctionSel, C.getASTContext(), + "handleFailureInFunction", "file", "lineNumber", + "description", nullptr); + if (Sel != HandleFailureInFunctionSel) return; break; case 5: - if (!isMultiArgSelector(&Sel, "handleFailureInMethod", "object", "file", - "lineNumber", "description", NULL)) + lazyInitKeywordSelector(HandleFailureInMethodSel, C.getASTContext(), + "handleFailureInMethod", "object", "file", + "lineNumber", "description", nullptr); + if (Sel != HandleFailureInMethodSel) return; break; } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 9ac993160a..d312b0199a 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -14,6 +14,7 @@ #include "ClangSACheckers.h" #include "AllocationDiagnostics.h" +#include "SelectorExtras.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -675,18 +676,9 @@ private: ObjCMethodSummaries[ObjCSummaryKey(ClsII, S)] = Summ; } - Selector generateSelector(va_list argp) { - SmallVector II; - - while (const char* s = va_arg(argp, const char*)) - II.push_back(&Ctx.Idents.get(s)); - - return Ctx.Selectors.getSelector(II.size(), &II[0]); - } - - void addMethodSummary(IdentifierInfo *ClsII, ObjCMethodSummariesTy& Summaries, - const RetainSummary * Summ, va_list argp) { - Selector S = generateSelector(argp); + void addMethodSummary(IdentifierInfo *ClsII, ObjCMethodSummariesTy &Summaries, + const RetainSummary *Summ, va_list argp) { + Selector S = getKeywordSelector(Ctx, argp); Summaries[ObjCSummaryKey(ClsII, S)] = Summ; } diff --git a/lib/StaticAnalyzer/Checkers/SelectorExtras.h b/lib/StaticAnalyzer/Checkers/SelectorExtras.h new file mode 100644 index 0000000000..4a3ef551c6 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/SelectorExtras.h @@ -0,0 +1,67 @@ +//=== SelectorExtras.h - Helpers for checkers using selectors -----*- C++ -*-=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_SA_CHECKERS_SELECTOREXTRAS +#define LLVM_CLANG_SA_CHECKERS_SELECTOREXTRAS + +#include "clang/AST/ASTContext.h" + +namespace clang { +namespace ento { + +static inline Selector getKeywordSelectorImpl(ASTContext &Ctx, + const char *First, + va_list argp) { + SmallVector II; + II.push_back(&Ctx.Idents.get(First)); + + while (const char *s = va_arg(argp, const char *)) + II.push_back(&Ctx.Idents.get(s)); + + return Ctx.Selectors.getSelector(II.size(), &II[0]); +} + +static inline Selector getKeywordSelector(ASTContext &Ctx, va_list argp) { + const char *First = va_arg(argp, const char *); + assert(First && "keyword selectors must have at least one argument"); + return getKeywordSelectorImpl(Ctx, First, argp); +} + +END_WITH_NULL +static inline Selector getKeywordSelector(ASTContext &Ctx, + const char *First, ...) { + va_list argp; + va_start(argp, First); + Selector result = getKeywordSelectorImpl(Ctx, First, argp); + va_end(argp); + return result; +} + +END_WITH_NULL +static inline void lazyInitKeywordSelector(Selector &Sel, ASTContext &Ctx, + const char *First, ...) { + if (!Sel.isNull()) + return; + va_list argp; + va_start(argp, First); + Sel = getKeywordSelectorImpl(Ctx, First, argp); + va_end(argp); +} + +static inline void lazyInitNullarySelector(Selector &Sel, ASTContext &Ctx, + const char *Name) { + if (!Sel.isNull()) + return; + Sel = GetNullarySelector(Name, Ctx); +} + +} // end namespace ento +} // end namespace clang + +#endif diff --git a/test/Analysis/NSContainers.m b/test/Analysis/NSContainers.m index 3d3603acb3..4b34926456 100644 --- a/test/Analysis/NSContainers.m +++ b/test/Analysis/NSContainers.m @@ -284,3 +284,11 @@ void testLiteralsNonNil() { clang_analyzer_eval(!!@{}); // expected-warning{{TRUE}} } +@interface NSMutableArray (MySafeAdd) +- (void)addObject:(id)obj safe:(BOOL)safe; +@end + +void testArrayCategory(NSMutableArray *arr) { + [arr addObject:0 safe:1]; // no-warning +} + -- 2.40.0