Skip to content

Commit

Permalink
[analyzer] MIGChecker: Add support for os_ref_retain().
Browse files Browse the repository at this point in the history
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

llvm-svn: 360737
  • Loading branch information
haoNoQ authored and MrSidims committed May 17, 2019
1 parent 31bcbfc commit e02d861
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 5 deletions.
33 changes: 28 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
Expand Up @@ -84,6 +84,8 @@ class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>,
#undef CALL
};

CallDescription OsRefRetain{"os_ref_retain", 1};

void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;

public:
Expand All @@ -105,10 +107,17 @@ class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>,
};
} // 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;

Expand Down Expand Up @@ -168,6 +177,19 @@ static bool isInMIGCall(CheckerContext &C) {
}

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;

Expand All @@ -178,10 +200,11 @@ void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const {
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 {
Expand All @@ -193,7 +216,7 @@ void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const {
<< "\' 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.
Expand Down
20 changes: 20 additions & 0 deletions clang/test/Analysis/mig.mm
Expand Up @@ -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))

Expand Down Expand Up @@ -237,3 +250,10 @@ IOReturn registerNotificationPort(mach_port_t port, UInt32 x, UInt32 y) {
// 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
}

0 comments on commit e02d861

Please sign in to comment.