r179230 - [analyzer] Switched to checkPreCall interface for detecting usage after free.

Jordan Rose jordan_rose at apple.com
Wed Apr 10 15:28:20 PDT 2013


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 <anton.yartsev at gmail.com> 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 at -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}}
> +}
> +
> + at interface My
> ++ (void)param:(void *)p;
> + at 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130410/7a473acd/attachment.html>


More information about the cfe-commits mailing list