Just for paranoia's sake, can you also include a test case where there are two use-after-free arguments in the same function call? It's okay if we only warn about one of them (current behavior), but just so that we don't get a crash or something.
Jordan On Apr 10, 2013, at 15:21 , Anton Yartsev <[email protected]> wrote: > Author: ayartsev > Date: Wed Apr 10 17:21:41 2013 > New Revision: 179230 > > URL: http://llvm.org/viewvc/llvm-project?rev=179230&view=rev > Log: > [analyzer] Switched to checkPreCall interface for detecting usage after free. > > Now the check is also applied to arguments for Objective-C method calls and > to 'this' pointer. > > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > cfe/trunk/test/Analysis/NewDelete-checker-test.cpp > cfe/trunk/test/Analysis/malloc.mm > cfe/trunk/test/Analysis/new.cpp > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=179230&r1=179229&r2=179230&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Apr 10 > 17:21:41 2013 > @@ -147,7 +147,7 @@ class MallocChecker : public Checker<che > check::PointerEscape, > check::ConstPointerEscape, > check::PreStmt<ReturnStmt>, > - check::PreStmt<CallExpr>, > + check::PreCall, > check::PostStmt<CallExpr>, > check::PostStmt<CXXNewExpr>, > check::PreStmt<CXXDeleteExpr>, > @@ -181,7 +181,7 @@ public: > > ChecksFilter Filter; > > - void checkPreStmt(const CallExpr *S, CheckerContext &C) const; > + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; > void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; > void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; > void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const; > @@ -1671,26 +1671,39 @@ void MallocChecker::checkDeadSymbols(Sym > C.addTransition(state->set<RegionState>(RS), N); > } > > -void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) > const { > +void MallocChecker::checkPreCall(const CallEvent &Call, > + CheckerContext &C) const { > + > // We will check for double free in the post visit. > - if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) && > - isFreeFunction(C.getCalleeDecl(CE), C.getASTContext())) > - return; > + if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) { > + const FunctionDecl *FD = FC->getDecl(); > + if (!FD) > + return; > > - if (Filter.CNewDeleteChecker && > - isStandardNewDelete(C.getCalleeDecl(CE), C.getASTContext())) > - return; > + if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) && > + isFreeFunction(FD, C.getASTContext())) > + return; > > - // Check use after free, when a freed pointer is passed to a call. > - ProgramStateRef State = C.getState(); > - 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 = C.getSVal(A).getAsSymbol(); > + if (Filter.CNewDeleteChecker && > + isStandardNewDelete(FD, C.getASTContext())) > + return; > + } > + > + // Check if the callee of a method is deleted. > + if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) { > + SymbolRef Sym = CC->getCXXThisVal().getAsSymbol(); > + if (!Sym || checkUseAfterFree(Sym, C, CC->getCXXThisExpr())) > + return; > + } > + > + // Check arguments for being used after free. > + for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { > + SVal ArgSVal = Call.getArgSVal(I); > + if (ArgSVal.getAs<Loc>()) { > + SymbolRef Sym = ArgSVal.getAsSymbol(); > if (!Sym) > continue; > - if (checkUseAfterFree(Sym, C, A)) > + if (checkUseAfterFree(Sym, C, Call.getArgExpr(I))) > return; > } > } > > Modified: cfe/trunk/test/Analysis/NewDelete-checker-test.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp?rev=179230&r1=179229&r2=179230&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/NewDelete-checker-test.cpp (original) > +++ cfe/trunk/test/Analysis/NewDelete-checker-test.cpp Wed Apr 10 17:21:41 > 2013 > @@ -54,7 +54,6 @@ void testGlobalNoThrowPlacementExprNewBe > // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} > #endif > > - > //----- Standard pointer placement operators > void testGlobalPointerPlacementNew() { > int i; > @@ -91,14 +90,54 @@ void testNewInvalidationPlacement(PtrWra > // other checks > //--------------- > > -void f(int *); > +class SomeClass { > +public: > + void f(int *p); > +}; > + > +void f(int *p1, int *p2 = 0, int *p3 = 0); > +void g(SomeClass &c, ...); > > -void testUseAfterDelete() { > +void testUseFirstArgAfterDelete() { > int *p = new int; > delete p; > f(p); // expected-warning{{Use of memory after it is freed}} > } > > +void testUseMiddleArgAfterDelete(int *p) { > + delete p; > + f(0, p); // expected-warning{{Use of memory after it is freed}} > +} > + > +void testUseLastArgAfterDelete(int *p) { > + delete p; > + f(0, 0, p); // expected-warning{{Use of memory after it is freed}} > +} > + > +void testUseRefArgAfterDelete(SomeClass &c) { > + delete &c; > + g(c); // expected-warning{{Use of memory after it is freed}} > +} > + > +void testVariadicArgAfterDelete() { > + SomeClass c; > + int *p = new int; > + delete p; > + g(c, 0, p); // expected-warning{{Use of memory after it is freed}} > +} > + > +void testUseMethodArgAfterDelete(int *p) { > + SomeClass *c = new SomeClass; > + delete p; > + c->f(p); // expected-warning{{Use of memory after it is freed}} > +} > + > +void testUseThisAfterDelete() { > + SomeClass *c = new SomeClass; > + delete c; > + c->f(0); // expected-warning{{Use of memory after it is freed}} > +} > + > void testDeleteAlloca() { > int *p = (int *)__builtin_alloca(sizeof(int)); > delete p; // expected-warning{{Memory allocated by alloca() should not be > deallocated}} > > Modified: cfe/trunk/test/Analysis/malloc.mm > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.mm?rev=179230&r1=179229&r2=179230&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/malloc.mm (original) > +++ cfe/trunk/test/Analysis/malloc.mm Wed Apr 10 17:21:41 2013 > @@ -81,7 +81,17 @@ void testRelinquished2() { > void *data = malloc(42); > NSData *nsdata; > free(data); > - [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Attempt > to free released memory}} > + [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Use of > memory after it is freed}} > +} > + > +@interface My > ++ (void)param:(void *)p; > +@end > + > +void testUseAfterFree() { > + int *p = (int *)malloc(sizeof(int)); > + free(p); > + [My param:p]; // expected-warning{{Use of memory after it is freed}} > } > > void testNoCopy() { > > Modified: cfe/trunk/test/Analysis/new.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=179230&r1=179229&r2=179230&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/new.cpp (original) > +++ cfe/trunk/test/Analysis/new.cpp Wed Apr 10 17:21:41 2013 > @@ -8,6 +8,12 @@ extern "C" void *malloc(size_t); > extern "C" void free(void *); > > int someGlobal; > + > +class SomeClass { > +public: > + void f(int *p); > +}; > + > void testImplicitlyDeclaredGlobalNew() { > if (someGlobal != 0) > return; > @@ -101,6 +107,12 @@ void testCacheOut(PtrWrapper w) { > new (&w.x) (int*)(0); // we cache out here; don't crash > } > > +void testUseAfter(int *p) { > + SomeClass *c = new SomeClass; > + free(p); > + c->f(p); // expected-warning{{Use of memory after it is freed}} > + delete c; > +} > > //-------------------------------------------------------------------- > // Check for intersection with other checkers from MallocChecker.cpp > @@ -152,6 +164,12 @@ void testCustomPlacementNewAfterFree() { > p = new(0, p) int; // expected-warning{{Use of memory after it is freed}} > } > > +void testUsingThisAfterDelete() { > + SomeClass *c = new SomeClass; > + delete c; > + c->f(0); // no-warning > +} > + > //-------------------------------- > // Incorrectly-modelled behavior > //-------------------------------- > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
