From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Wed, 29 Aug 2018 22:39:20 +0000 (+0000)
Subject: [analyzer] CFRetainReleaseChecker: Don't check C++ methods with the same name.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=19ada6cb9bf6b625bfd756e126d50e7418ead4dc;p=clang

[analyzer] CFRetainReleaseChecker: Don't check C++ methods with the same name.

Don't try to understand what's going on when there's a C++ method called eg.
CFRetain().

Refactor the checker a bit, to use more modern APIs.

Differential Revision: https://reviews.llvm.org/D50866


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340982 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 70ce41ac89..a21e10d948 100644
--- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -36,6 +36,7 @@
 
 using namespace clang;
 using namespace ento;
+using namespace llvm;
 
 namespace {
 class APIMisuse : public BugType {
@@ -531,93 +532,59 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE,
 //===----------------------------------------------------------------------===//
 
 namespace {
-class CFRetainReleaseChecker : public Checker< check::PreStmt<CallExpr> > {
-  mutable std::unique_ptr<APIMisuse> BT;
-  mutable IdentifierInfo *Retain, *Release, *MakeCollectable, *Autorelease;
+class CFRetainReleaseChecker : public Checker<check::PreCall> {
+  mutable APIMisuse BT{this, "null passed to CF memory management function"};
+  CallDescription CFRetain{"CFRetain", 1},
+                  CFRelease{"CFRelease", 1},
+                  CFMakeCollectable{"CFMakeCollectable", 1},
+                  CFAutorelease{"CFAutorelease", 1};
 
 public:
-  CFRetainReleaseChecker()
-      : Retain(nullptr), Release(nullptr), MakeCollectable(nullptr),
-        Autorelease(nullptr) {}
-  void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
-void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
+void CFRetainReleaseChecker::checkPreCall(const CallEvent &Call,
                                           CheckerContext &C) const {
-  // If the CallExpr doesn't have exactly 1 argument just give up checking.
-  if (CE->getNumArgs() != 1)
+  // TODO: Make this check part of CallDescription.
+  if (!Call.isGlobalCFunction())
     return;
 
-  ProgramStateRef state = C.getState();
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
-    return;
-
-  if (!BT) {
-    ASTContext &Ctx = C.getASTContext();
-    Retain = &Ctx.Idents.get("CFRetain");
-    Release = &Ctx.Idents.get("CFRelease");
-    MakeCollectable = &Ctx.Idents.get("CFMakeCollectable");
-    Autorelease = &Ctx.Idents.get("CFAutorelease");
-    BT.reset(new APIMisuse(
-        this, "null passed to CF memory management function"));
-  }
-
   // Check if we called CFRetain/CFRelease/CFMakeCollectable/CFAutorelease.
-  const IdentifierInfo *FuncII = FD->getIdentifier();
-  if (!(FuncII == Retain || FuncII == Release || FuncII == MakeCollectable ||
-        FuncII == Autorelease))
+  if (!(Call.isCalled(CFRetain) || Call.isCalled(CFRelease) ||
+        Call.isCalled(CFMakeCollectable) || Call.isCalled(CFAutorelease)))
     return;
 
-  // FIXME: The rest of this just checks that the argument is non-null.
-  // It should probably be refactored and combined with NonNullParamChecker.
-
   // Get the argument's value.
-  const Expr *Arg = CE->getArg(0);
-  SVal ArgVal = C.getSVal(Arg);
+  SVal ArgVal = Call.getArgSVal(0);
   Optional<DefinedSVal> DefArgVal = ArgVal.getAs<DefinedSVal>();
   if (!DefArgVal)
     return;
 
-  // Get a NULL value.
-  SValBuilder &svalBuilder = C.getSValBuilder();
-  DefinedSVal zero =
-      svalBuilder.makeZeroVal(Arg->getType()).castAs<DefinedSVal>();
-
-  // Make an expression asserting that they're equal.
-  DefinedOrUnknownSVal ArgIsNull = svalBuilder.evalEQ(state, zero, *DefArgVal);
-
-  // Are they equal?
-  ProgramStateRef stateTrue, stateFalse;
-  std::tie(stateTrue, stateFalse) = state->assume(ArgIsNull);
+  // Is it null?
+  ProgramStateRef state = C.getState();
+  ProgramStateRef stateNonNull, stateNull;
+  std::tie(stateNonNull, stateNull) = state->assume(*DefArgVal);
 
-  if (stateTrue && !stateFalse) {
-    ExplodedNode *N = C.generateErrorNode(stateTrue);
+  if (!stateNonNull) {
+    ExplodedNode *N = C.generateErrorNode(stateNull);
     if (!N)
       return;
 
-    const char *description;
-    if (FuncII == Retain)
-      description = "Null pointer argument in call to CFRetain";
-    else if (FuncII == Release)
-      description = "Null pointer argument in call to CFRelease";
-    else if (FuncII == MakeCollectable)
-      description = "Null pointer argument in call to CFMakeCollectable";
-    else if (FuncII == Autorelease)
-      description = "Null pointer argument in call to CFAutorelease";
-    else
-      llvm_unreachable("impossible case");
+    SmallString<64> Str;
+    raw_svector_ostream OS(Str);
+    OS << "Null pointer argument in call to "
+       << cast<FunctionDecl>(Call.getDecl())->getName();
 
-    auto report = llvm::make_unique<BugReport>(*BT, description, N);
-    report->addRange(Arg->getSourceRange());
-    bugreporter::trackNullOrUndefValue(N, Arg, *report);
+    auto report = llvm::make_unique<BugReport>(BT, OS.str(), N);
+    report->addRange(Call.getArgSourceRange(0));
+    bugreporter::trackNullOrUndefValue(N, Call.getArgExpr(0), *report);
     C.emitReport(std::move(report));
     return;
   }
 
   // From here on, we know the argument is non-null.
-  C.addTransition(stateFalse);
+  C.addTransition(stateNonNull);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/test/Analysis/retain-release.mm b/test/Analysis/retain-release.mm
index ac83c1a48e..5dc8f857d8 100644
--- a/test/Analysis/retain-release.mm
+++ b/test/Analysis/retain-release.mm
@@ -470,3 +470,18 @@ void* IOBSDNameMatching();
 void rdar33832412() {
   void* x = IOBSDNameMatching(); // no-warning
 }
+
+
+namespace member_CFRetains {
+class Foo {
+public:
+  void CFRetain(const Foo &) {}
+  void CFRetain(int) {}
+};
+
+void bar() {
+  Foo foo;
+  foo.CFRetain(foo); // no-warning
+  foo.CFRetain(0); // no-warning
+}
+}