From: George Karpenkov <ekarpenkov@apple.com>
Date: Tue, 22 Jan 2019 19:51:00 +0000 (+0000)
Subject: [analyzer] Insert notes in RetainCountChecker where our dynamic cast modeling assumes... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=48ff4162635d886fe75b52395825bfcea1c189dd;p=clang

[analyzer] Insert notes in RetainCountChecker where our dynamic cast modeling assumes 'null' output

rdar://47397214

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

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

diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index e86d63804b..a72f09fb3c 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -575,7 +575,6 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
 
   // Helper tag for providing diagnostics: indicate whether dealloc was sent
   // at this location.
-  static CheckerProgramPointTag DeallocSentTag(this, DeallocTagDescription);
   bool DeallocSent = false;
 
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
@@ -903,8 +902,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
       // Assume that output is zero on the other branch.
       NullOutputState = NullOutputState->BindExpr(
           CE, LCtx, C.getSValBuilder().makeNull(), /*Invalidate=*/false);
-
-      C.addTransition(NullOutputState);
+      C.addTransition(NullOutputState, &CastFailTag);
 
       // And on the original branch assume that both input and
       // output are non-zero.
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
index ea2c84be31..8e74f6cbb0 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -260,9 +260,11 @@ class RetainCountChecker
   RefCountBug leakWithinFunction{this, RefCountBug::LeakWithinFunction};
   RefCountBug leakAtReturn{this, RefCountBug::LeakAtReturn};
 
+  CheckerProgramPointTag DeallocSentTag{this, "DeallocSent"};
+  CheckerProgramPointTag CastFailTag{this, "DynamicCastFail"};
+
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
 public:
-  static constexpr const char *DeallocTagDescription = "DeallocSent";
 
   /// Track Objective-C and CoreFoundation objects.
   bool TrackObjCAndCFObjects = false;
@@ -361,6 +363,14 @@ public:
                              CheckerContext &Ctx,
                              ExplodedNode *Pred = nullptr) const;
 
+  const CheckerProgramPointTag &getDeallocSentTag() const {
+    return DeallocSentTag;
+  }
+
+  const CheckerProgramPointTag &getCastFailTag() const {
+    return CastFailTag;
+  }
+
 private:
   /// Perform the necessary checks and state adjustments at the end of the
   /// function.
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 1915b61222..dcb5ca8555 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -66,7 +66,7 @@ StringRef RefCountBug::getDescription() const {
 RefCountBug::RefCountBug(const CheckerBase *Checker, RefCountBugType BT)
     : BugType(Checker, bugTypeToName(BT), categories::MemoryRefCount,
               /*SupressOnSink=*/BT == LeakWithinFunction || BT == LeakAtReturn),
-      BT(BT) {}
+      BT(BT), Checker(Checker) {}
 
 static bool isNumericLiteralExpression(const Expr *E) {
   // FIXME: This set of cases was copied from SemaExprObjC.
@@ -423,6 +423,8 @@ RefCountReportVisitor::VisitNode(const ExplodedNode *N,
                               BugReporterContext &BRC, BugReport &BR) {
 
   const auto &BT = static_cast<const RefCountBug&>(BR.getBugType());
+  const auto *Checker =
+      static_cast<const RetainCountChecker *>(BT.getChecker());
 
   bool IsFreeUnowned = BT.getBugType() == RefCountBug::FreeNotOwned ||
                        BT.getBugType() == RefCountBug::DeallocNotOwned;
@@ -509,8 +511,12 @@ RefCountReportVisitor::VisitNode(const ExplodedNode *N,
   bool DeallocSent = false;
 
   const ProgramPointTag *Tag = N->getLocation().getTag();
-  if (Tag && Tag->getTagDescription().contains(
-                 RetainCountChecker::DeallocTagDescription)) {
+
+  if (Tag == &Checker->getCastFailTag()) {
+    os << "Assuming dynamic cast returns null due to type mismatch";
+  }
+
+  if (Tag == &Checker->getDeallocSentTag()) {
     // We only have summaries attached to nodes after evaluating CallExpr and
     // ObjCMessageExprs.
     const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt();
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
index 8b19d75fb1..e0b53e4387 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
@@ -37,12 +37,18 @@ public:
   };
   RefCountBug(const CheckerBase *checker, RefCountBugType BT);
   StringRef getDescription() const;
+
   RefCountBugType getBugType() const {
     return BT;
   }
 
+  const CheckerBase *getChecker() const {
+    return Checker;
+  }
+
 private:
   RefCountBugType BT;
+  const CheckerBase *Checker;
   static StringRef bugTypeToName(RefCountBugType BT);
 };
 
diff --git a/test/Analysis/osobject-retain-release.cpp b/test/Analysis/osobject-retain-release.cpp
index 24d6cb284c..467b643410 100644
--- a/test/Analysis/osobject-retain-release.cpp
+++ b/test/Analysis/osobject-retain-release.cpp
@@ -488,7 +488,7 @@ unsigned int check_dynamic_cast_no_null_on_orig(OSObject *obj) {
 
 void check_dynamic_cast_null_branch(OSObject *obj) {
   OSArray *arr1 = OSArray::withCapacity(10); // expected-note{{Call to method 'OSArray::withCapacity' returns an OSObject}}
-  OSArray *arr = OSDynamicCast(OSArray, obj);
+  OSArray *arr = OSDynamicCast(OSArray, obj); // expected-note{{Assuming dynamic cast returns null due to type mismatch}}
   if (!arr) // expected-note{{Taking true branch}}
     return; // expected-warning{{Potential leak of an object stored into 'arr1'}}
             // expected-note@-1{{Object leaked}}
@@ -499,6 +499,7 @@ void check_dynamic_cast_null_check() {
   OSArray *arr = OSDynamicCast(OSArray, OSObject::generateObject(1)); // expected-note{{Call to method 'OSObject::generateObject' returns an OSObject}}
     // expected-warning@-1{{Potential leak of an object}}
     // expected-note@-2{{Object leaked}}
+    // expected-note@-3{{Assuming dynamic cast returns null due to type mismatch}}
   if (!arr)
     return;
   arr->release();