r360737 - [analyzer] MIGChecker: Add support for os_ref_retain().

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue May 14 18:19:19 2019
New Revision: 360737

URL: http://llvm.org/viewvc/llvm-project?rev=360737&view=rev
Log:
[analyzer] MIGChecker: Add support for os_ref_retain().

Suppress MIG checker false positives that occur when the programmer increments
the reference count before calling a MIG destructor, and the MIG destructor
literally boils down to decrementing the reference count.

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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
    cfe/trunk/test/Analysis/mig.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp?rev=360737&r1=360736&r2=360737&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Tue May 14 18:19:19 2019
@@ -84,6 +84,8 @@ class MIGChecker : public Checker<check:
 #undef CALL
   };
 
+  CallDescription OsRefRetain{"os_ref_retain", 1};
+
   void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;
 
 public:
@@ -105,10 +107,17 @@ public:
 };
 } // end anonymous namespace
 
+// A flag that says that the programmer has called a MIG destructor
+// for at least one parameter.
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool)
-
-static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) {
-  SymbolRef Sym = V.getAsSymbol();
+// A set of parameters for which the check is suppressed because
+// reference counting is being performed.
+REGISTER_SET_WITH_PROGRAMSTATE(RefCountedParameters, const ParmVarDecl *);
+
+static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C,
+                                         bool IncludeBaseRegions = false) {
+  // TODO: We should most likely always include base regions here.
+  SymbolRef Sym = V.getAsSymbol(IncludeBaseRegions);
   if (!Sym)
     return nullptr;
 
@@ -168,6 +177,19 @@ static bool isInMIGCall(CheckerContext &
 }
 
 void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+  if (Call.isCalled(OsRefRetain)) {
+    // If the code is doing reference counting over the parameter,
+    // it opens up an opportunity for safely calling a destructor function.
+    // TODO: We should still check for over-releases.
+    if (const ParmVarDecl *PVD =
+            getOriginParam(Call.getArgSVal(0), C, /*IncludeBaseRegions=*/true)) {
+      // We never need to clean up the program state because these are
+      // top-level parameters anyway, so they're always live.
+      C.addTransition(C.getState()->add<RefCountedParameters>(PVD));
+    }
+    return;
+  }
+
   if (!isInMIGCall(C))
     return;
 
@@ -178,10 +200,11 @@ void MIGChecker::checkPostCall(const Cal
   if (I == Deallocators.end())
     return;
 
+  ProgramStateRef State = C.getState();
   unsigned ArgIdx = I->second;
   SVal Arg = Call.getArgSVal(ArgIdx);
   const ParmVarDecl *PVD = getOriginParam(Arg, C);
-  if (!PVD)
+  if (!PVD || State->contains<RefCountedParameters>(PVD))
     return;
 
   const NoteTag *T = C.getNoteTag([this, PVD](BugReport &BR) -> std::string {
@@ -193,7 +216,7 @@ void MIGChecker::checkPostCall(const Cal
        << "\' is deallocated";
     return OS.str();
   });
-  C.addTransition(C.getState()->set<ReleasedParameter>(true), T);
+  C.addTransition(State->set<ReleasedParameter>(true), T);
 }
 
 // Returns true if V can potentially represent a "successful" kern_return_t.

Modified: cfe/trunk/test/Analysis/mig.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/mig.mm?rev=360737&r1=360736&r2=360737&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/mig.mm (original)
+++ cfe/trunk/test/Analysis/mig.mm Tue May 14 18:19:19 2019
@@ -19,11 +19,24 @@ typedef struct ipc_port *ipc_port_t;
 typedef unsigned mach_port_t;
 typedef uint32_t UInt32;
 
+struct os_refcnt {};
+typedef struct os_refcnt os_refcnt_t;
+
+struct thread {
+  os_refcnt_t ref_count;
+};
+typedef struct thread *thread_t;
+
 kern_return_t vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 kern_return_t mach_vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 void mig_deallocate(vm_address_t, vm_size_t);
 kern_return_t mach_port_deallocate(ipc_space_t, mach_port_name_t);
 void ipc_port_release(ipc_port_t);
+void thread_deallocate(thread_t);
+
+static void os_ref_retain(struct os_refcnt *rc);
+
+#define thread_reference_internal(thread) os_ref_retain(&(thread)->ref_count);
 
 #define MIG_SERVER_ROUTINE __attribute__((mig_server_routine))
 
@@ -237,3 +250,10 @@ class MyClient: public IOUserClient {
                            // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
   }
 };
+
+MIG_SERVER_ROUTINE
+kern_return_t test_os_ref_retain(thread_t thread) {
+  thread_reference_internal(thread);
+  thread_deallocate(thread);
+  return KERN_ERROR; // no-warning
+}