From: Anna Zaks Date: Wed, 8 Feb 2012 23:16:56 +0000 (+0000) Subject: [analyzer] MallocChecker: implement pessimistic version of the checker, X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=91c2a1192cdd4e7b2b4ac7838c5aceef200ea251;p=clang [analyzer] MallocChecker: implement pessimistic version of the checker, which allows values to escape through unknown calls. Assumes all calls but the malloc family are unknown. Also, catch a use-after-free when a pointer is passed to a function after a call to free (previously, you had to explicitly dereference the pointer value). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150112 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index b14b400202..c110e0f6f8 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -131,6 +131,10 @@ private: void ReallocMem(CheckerContext &C, const CallExpr *CE) const; static void CallocMem(CheckerContext &C, const CallExpr *CE); + bool checkEscape(SymbolRef Sym, const Stmt *S, CheckerContext &C) const; + bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, + const Stmt *S = 0) const; + static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const; @@ -186,6 +190,7 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { return; } + if (Filter.CMallocOptimistic) // Check all the attributes, if there are any. // There can be multiple of these attributes. if (FD->hasAttrs()) { @@ -206,6 +211,22 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { } } } + + if (Filter.CMallocPessimistic) { + ProgramStateRef State = C.getState(); + // The pointer might escape through a function call. + for (CallExpr::const_arg_iterator I = CE->arg_begin(), + E = CE->arg_end(); I != E; ++I) { + const Expr *A = *I; + if (A->getType().getTypePtr()->isAnyPointerType()) { + SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol(); + if (!Sym) + return; + checkEscape(Sym, A, C); + checkUseAfterFree(Sym, C, A); + } + } + } } void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) { @@ -642,32 +663,36 @@ void MallocChecker::checkEndPath(CheckerContext &Ctx) const { } } -void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { - const Expr *retExpr = S->getRetValue(); - if (!retExpr) - return; - +bool MallocChecker::checkEscape(SymbolRef Sym, const Stmt *S, + CheckerContext &C) const { ProgramStateRef state = C.getState(); - - SymbolRef Sym = state->getSVal(retExpr, C.getLocationContext()).getAsSymbol(); - if (!Sym) - return; - const RefState *RS = state->get(Sym); if (!RS) - return; + return false; - // FIXME: check other cases. - if (RS->isAllocated()) + if (RS->isAllocated()) { state = state->set(Sym, RefState::getEscaped(S)); + C.addTransition(state); + return true; + } + return false; +} - C.addTransition(state); +void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { + const Expr *E = S->getRetValue(); + if (!E) + return; + SymbolRef Sym = C.getState()->getSVal(E, C.getLocationContext()).getAsSymbol(); + if (!Sym) + return; + + checkEscape(Sym, S, C); } ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, SVal Cond, bool Assumption) const { - // If a symblic region is assumed to NULL, set its state to AllocateFailed. + // If a symbolic region is assumed to NULL, set its state to AllocateFailed. // FIXME: should also check symbols assumed to non-null. RegionStateTy RS = state->get(); @@ -681,24 +706,32 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, return state; } +bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, + const Stmt *S) const { + assert(Sym); + const RefState *RS = C.getState()->get(Sym); + if (RS && RS->isReleased()) { + if (ExplodedNode *N = C.addTransition()) { + if (!BT_UseFree) + BT_UseFree.reset(new BuiltinBug("Use dynamically allocated memory " + "after it is freed.")); + + BugReport *R = new BugReport(*BT_UseFree, BT_UseFree->getDescription(),N); + if (S) + R->addRange(S->getSourceRange()); + C.EmitReport(R); + return true; + } + } + return false; +} + // Check if the location is a freed symbolic region. void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const { SymbolRef Sym = l.getLocSymbolInBase(); - if (Sym) { - const RefState *RS = C.getState()->get(Sym); - if (RS && RS->isReleased()) { - if (ExplodedNode *N = C.addTransition()) { - if (!BT_UseFree) - BT_UseFree.reset(new BuiltinBug("Use dynamically allocated memory " - "after it is freed.")); - - BugReport *R = new BugReport(*BT_UseFree, BT_UseFree->getDescription(), - N); - C.EmitReport(R); - } - } - } + if (Sym) + checkUseAfterFree(Sym, C); } void MallocChecker::checkBind(SVal location, SVal val, diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index d9087ab830..f19510b639 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -4,23 +4,9 @@ void *malloc(size_t); void free(void *); void *realloc(void *ptr, size_t size); void *calloc(size_t nmemb, size_t size); -void __attribute((ownership_returns(malloc))) *my_malloc(size_t); -void __attribute((ownership_takes(malloc, 1))) my_free(void *); -void __attribute((ownership_returns(malloc, 1))) *my_malloc2(size_t); -void __attribute((ownership_holds(malloc, 1))) my_hold(void *); - -// Duplicate attributes are silly, but not an error. -// Duplicate attribute has no extra effect. -// If two are of different kinds, that is an error and reported as such. -void __attribute((ownership_holds(malloc, 1))) -__attribute((ownership_holds(malloc, 1))) -__attribute((ownership_holds(malloc, 3))) my_hold2(void *, void *, void *); -void *my_malloc3(size_t); -void *myglobalpointer; -struct stuff { - void *somefield; -}; -struct stuff myglobalstuff; + +void myfoo(int *p); +void myfooint(int p); void f1() { int *p = malloc(12); @@ -44,107 +30,6 @@ void f2_realloc_1() { int *q = realloc(p,0); // no-warning } -// ownership attributes tests -void naf1() { - int *p = my_malloc3(12); - return; // no-warning -} - -void n2af1() { - int *p = my_malloc2(12); - return; // expected-warning{{Allocated memory never released. Potential memory leak.}} -} - -void af1() { - int *p = my_malloc(12); - return; // expected-warning{{Allocated memory never released. Potential memory leak.}} -} - -void af1_b() { - int *p = my_malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}} -} - -void af1_c() { - myglobalpointer = my_malloc(12); // no-warning -} - -void af1_d() { - struct stuff mystuff; - mystuff.somefield = my_malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}} -} - -// Test that we can pass out allocated memory via pointer-to-pointer. -void af1_e(void **pp) { - *pp = my_malloc(42); // no-warning -} - -void af1_f(struct stuff *somestuff) { - somestuff->somefield = my_malloc(12); // no-warning -} - -// Allocating memory for a field via multiple indirections to our arguments is OK. -void af1_g(struct stuff **pps) { - *pps = my_malloc(sizeof(struct stuff)); // no-warning - (*pps)->somefield = my_malloc(42); // no-warning -} - -void af2() { - int *p = my_malloc(12); - my_free(p); - free(p); // expected-warning{{Try to free a memory block that has been released}} -} - -void af2b() { - int *p = my_malloc(12); - free(p); - my_free(p); // expected-warning{{Try to free a memory block that has been released}} -} - -void af2c() { - int *p = my_malloc(12); - free(p); - my_hold(p); // expected-warning{{Try to free a memory block that has been released}} -} - -void af2d() { - int *p = my_malloc(12); - free(p); - my_hold2(0, 0, p); // expected-warning{{Try to free a memory block that has been released}} -} - -// No leak if malloc returns null. -void af2e() { - int *p = my_malloc(12); - if (!p) - return; // no-warning - free(p); // no-warning -} - -// This case would inflict a double-free elsewhere. -// However, this case is considered an analyzer bug since it causes false-positives. -void af3() { - int *p = my_malloc(12); - my_hold(p); - free(p); // no-warning -} - -// This case would inflict a double-free elsewhere. -// However, this case is considered an analyzer bug since it causes false-positives. -int * af4() { - int *p = my_malloc(12); - my_free(p); - return p; // no-warning -} - -// This case is (possibly) ok, be conservative -int * af5() { - int *p = my_malloc(12); - my_hold(p); - return p; // no-warning -} - - - // This case tests that storing malloc'ed memory to a static variable which is // then returned is not leaked. In the absence of known contracts for functions // or inter-procedural analysis, this is a conservative answer. @@ -261,3 +146,97 @@ char callocZeroesBad () { } return result; // expected-warning{{never released}} } + +void nullFree() { + int *p = 0; + free(p); // no warning - a nop +} + +void paramFree(int *p) { + myfoo(p); + free(p); // no warning + myfoo(p); // TODO: This should be a warning. +} + +int* mallocEscapeRet() { + int *p = malloc(12); + return p; // no warning +} + +void mallocEscapeFoo() { + int *p = malloc(12); + myfoo(p); + return; // no warning +} + +void mallocEscapeFree() { + int *p = malloc(12); + myfoo(p); + free(p); +} + +void mallocEscapeFreeFree() { + int *p = malloc(12); + myfoo(p); + free(p); + free(p); // expected-warning{{Try to free a memory block that has been released}} +} + +void mallocEscapeFreeUse() { + int *p = malloc(12); + myfoo(p); + free(p); + myfoo(p); // expected-warning{{Use dynamically allocated memory after it is freed.}} +} + +int *myalloc(); +void myalloc2(int **p); + +void mallocEscapeFreeCustomAlloc() { + int *p = malloc(12); + myfoo(p); + free(p); + p = myalloc(); + free(p); // no warning +} + +void mallocEscapeFreeCustomAlloc2() { + int *p = malloc(12); + myfoo(p); + free(p); + myalloc2(&p); + free(p); // no warning +} + +void mallocBindFreeUse() { + int *x = malloc(12); + int *y = x; + free(y); + myfoo(x); // expected-warning{{Use dynamically allocated memory after it is freed.}} +} + +void mallocEscapeMalloc() { + int *p = malloc(12); + myfoo(p); + p = malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}} +} + +void mallocMalloc() { + int *p = malloc(12); + p = malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak}} +} + +void mallocFreeMalloc() { + int *p = malloc(12); + free(p); + p = malloc(12); + free(p); +} + +void MallocFreeUse_params() { + int *p = malloc(12); + free(p); + myfoo(p); //expected-warning{{Use dynamically allocated memory after it is freed}} + myfooint(*p); //expected-warning{{Use dynamically allocated memory after it is freed}} +} +