r205827 - [analyzer] When checking Foundation method calls, match the selectors exactly.
Jordan Rose
jordan_rose at apple.com
Tue Apr 8 18:39:23 PDT 2014
Author: jrose
Date: Tue Apr 8 20:39:22 2014
New Revision: 205827
URL: http://llvm.org/viewvc/llvm-project?rev=205827&view=rev
Log:
[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
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/NSContainers.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp?rev=205827&r1=205826&r2=205827&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp Tue Apr 8 20:39:22 2014
@@ -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<ObjCArrayLiteral> > {
mutable std::unique_ptr<APIMisuse> BT;
+ mutable llvm::SmallDenseMap<Selector, unsigned, 16> 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(
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(
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(
// If argument is '0', report a warning.
if ((Arg != InvalidArgIndex))
warnIfNilArg(C, msg, Arg, Class, CanBeSubscript);
-
}
void NilArgChecker::checkPostStmt(const ObjCArrayLiteral *AL,
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp?rev=205827&r1=205826&r2=205827&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp Tue Apr 8 20:39:22 2014
@@ -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::checkPostC
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::checkPostO
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;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=205827&r1=205826&r2=205827&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Tue Apr 8 20:39:22 2014
@@ -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<IdentifierInfo*, 10> 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;
}
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h?rev=205827&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h Tue Apr 8 20:39:22 2014
@@ -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<IdentifierInfo*, 10> 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
Modified: cfe/trunk/test/Analysis/NSContainers.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NSContainers.m?rev=205827&r1=205826&r2=205827&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/NSContainers.m (original)
+++ cfe/trunk/test/Analysis/NSContainers.m Tue Apr 8 20:39:22 2014
@@ -284,3 +284,11 @@ void testLiteralsNonNil() {
clang_analyzer_eval(!!@{}); // expected-warning{{TRUE}}
}
+ at interface NSMutableArray (MySafeAdd)
+- (void)addObject:(id)obj safe:(BOOL)safe;
+ at end
+
+void testArrayCategory(NSMutableArray *arr) {
+ [arr addObject:0 safe:1]; // no-warning
+}
+
More information about the cfe-commits
mailing list