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

[NoQ]

Summary

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.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp

118–119

This is probably a source of false negatives; i'll investigate it separately.

[dcoughlin]

LGTM. Thanks!

clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp

#define CALL(required_args, deallocated_arg, ...)                              \
       // defined on class 'IOUserClient' that takes exactly 1 argument, it knows
       // that the argument is going to be consumed in the sense of the MIG
       // consume-on-success convention.
       CALL(1, 0, "IOUserClient", "releaseAsyncReference64"),
       CALL(1, 0, "IOUserClient", "releaseNotificationPort"),
 #undef CALL
   };
 
+  CallDescription OsRefRetain{"os_ref_retain", 1};
+
   void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;
 
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
   // HACK: We're making two attempts to find the bug: checkEndFunction
   // should normally be enough but it fails when the return value is a literal
   // that never gets put into the Environment and ends of function with multiple
   // returns get agglutinated across returns, preventing us from obtaining
   // the return value. The problem is similar to https://reviews.llvm.org/D25326
   // but now we step into it in the top-level function.
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
     checkReturnAux(RS, C);
   }
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const {
     checkReturnAux(RS, C);
   }
 
 };
 } // 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;
 
   // If we optimistically assume that the MIG routine never re-uses the storage
   // that was passed to it as arguments when it invalidates it (but at most when
   // it assigns to parameter variables directly), this procedure correctly
   // determines if the value was loaded from the transitive closure of MIG
   // routine arguments in the heap.
if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
     for (const auto *OMD: MD->overridden_methods())
       if (OMD->hasAttr<MIGServerRoutineAttr>())
         return true;
 
   return false;
 }
 
 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;
 
   auto I = llvm::find_if(Deallocators,
                          [&](const std::pair<CallDescription, unsigned> &Item) {
                            return Call.isCalled(Item.first);
                          });
   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 {
     if (&BR.getBugType() != &BT)
       return "";
     SmallString<64> Str;
     llvm::raw_svector_ostream OS(Str);
     OS << "Value passed through parameter '" << PVD->getName()
        << "\' 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.
 static bool mayBeSuccess(SVal V, CheckerContext &C) {
   ProgramStateRef State = C.getState();
 
   // Can V represent KERN_SUCCESS?
   if (!State->isNull(V).isConstrainedFalse())

clang/test/Analysis/mig.mm

 typedef unsigned vm_address_t;
 typedef unsigned vm_size_t;
 typedef void *ipc_space_t;
 typedef unsigned long io_user_reference_t;
 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))
 
 // IOKit wrappers.
 
 class OSObject;
 typedef kern_return_t IOReturn;
 #define kIOReturnError 1
class MyClient: public IOUserClient {
   }
 
   IOReturn registerNotificationPort(mach_port_t port, UInt32 x, UInt32 y) {
     releaseNotificationPort(port); // expected-note{{Value passed through parameter 'port' is deallocated}}
     return kIOReturnError; // expected-warning{{MIG callback fails with error after deallocating argument value}}
                            // expected-note@-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
+}