Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -84,6 +84,8 @@ #undef CALL }; + CallDescription OsRefRetain{"os_ref_retain", 1}; + void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const; public: @@ -105,10 +107,17 @@ }; } // 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 @@ } 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(PVD)); + } + return; + } + if (!isInMIGCall(C)) return; @@ -178,10 +200,11 @@ 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(PVD)) return; const NoteTag *T = C.getNoteTag([this, PVD](BugReport &BR) -> std::string { @@ -193,7 +216,7 @@ << "\' is deallocated"; return OS.str(); }); - C.addTransition(C.getState()->set(true), T); + C.addTransition(State->set(true), T); } // Returns true if V can potentially represent a "successful" kern_return_t. Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -19,11 +19,24 @@ 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 @@ // 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 +}