[PATCH] [StaticAnalyzer]Handle Destructor call generated by C++ delete expr

[Jordan Rose]

On Sep 13, 2013, at 9:03 , Anna Zaks <ganna at apple.com> wrote:

> Karthik,
> 
> Why do we need the FIXME? It should set ill be possible to report the issue, no? You'd need to associate the warning with the location where the destructor was called.

I asked for this but I think I got it wrong...an arbitrary destructor call won't necessarily have an associated statement, but of course the one for a 'delete' should. I'm not sure we currently store enough information to handle this, though.

Jordan


> +  // FIXME: In case of double delete of class instance. The call to destructor
> +  // on second delete result in use of memory after free but doesn't correspond
> 
> result -> results
> 
> +  // to any stmt. Currently skipping through handle the same.
> The last sentence reads wrong.
> 
> +  if (isReleased(Sym, C) && S) {
>     ReportUseAfterFree(C, S->getSourceRange(), Sym);
>     return true;
>   }
> 
> Cheers,
> Anna.
> 
> On Sep 12, 2013, at 9:58 PM, Karthik Bhat <kv.bhat at samsung.com> wrote:
> 
>> Hi Jordan,
>> Thanks for the review and sorry for keeping this simple patch on hold for such a long time. I'm a bit new to SA Core and was bit confused about the UnknownVal thing. I think i understand now what you wanted to communicate.
>> So if my understanding is correct this is what we want to do-
>> 
>> Memory region can be null here in 2 cases here-
>> 1) When it is actually bound to a null value.
>> 2) In case the SVal is Unknown and we try to get the region corresponding to it.
>> 
>> In the 1st case we can conclude that this is a null region and hence should not run the destructor but in the second case we cannot be sure if it is actually bound to a null region and hence should run destructor in this case.
>> Am i right here?
>> 
> 
> Yes. That is correct.
> 
>> Handled the same in ProcessDeleteDtor. What we are doing here is if we can conclude that the SVal is null don't run the destructor.
>> For all other cases call VisitCXXDestructor so it is handled in the same way as it is currently being done for object destruction.
>> 
>> Also added a fixme in malloc checker and modified test cases to follow llvm coding guidelines.
>> 
>> Thanks!
>> 
>> Hi jordan_rose,
>> 
>> http://llvm-reviews.chandlerc.com/D1594
>> 
>> CHANGE SINCE LAST DIFF
>> http://llvm-reviews.chandlerc.com/D1594?vs=4222&id=4261#toc
>> 
>> Files:
>> test/Analysis/new.cpp
>> include/clang/Analysis/CFG.h
>> lib/StaticAnalyzer/Checkers/MallocChecker.cpp
>> lib/StaticAnalyzer/Core/CallEvent.cpp
>> lib/StaticAnalyzer/Core/ExprEngine.cpp
>> <D1594.5.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>