[Bug analyzer/109365] Double delete yields -Wanalyzer-use-after-free instead of -Wanalyzer-double-free

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109365

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dmalcolm at gcc dot gnu.org

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Benjamin Priour from comment #4)
> (In reply to Benjamin Priour from comment #3)

Here's a link to the reproducer:
  https://godbolt.org/z/Wa3fqjrTK
with the fields renamed to avoid reusing the name "a".


> [...snip...]
> > 
> > 
> >   <bb 3> :
> >   *a.0_11 ={v} {CLOBBER};
> >   operator delete (a.0_11, 8);
> >
> [...snip...] 
> >
> > Entry statement of bb3 is the one actually detected as
> > -Wanalyzer-double-free.
> 
> Given the above IPA, we cannot just ignore the assignment statement, as it
> could really be an injurious statement, not just a pre-deallocation
> statement at it is now.

Ths assignment statement:
  *a.0_11 ={v} {CLOBBER};
is a "clobber", which is a special-case of assignment, generated by the
frontends when something is going out of scope, or becoming invalid.

We could potentially just special-case such ass

> 
> Consider the code:
>   A* a;
>   ...
>   delete a;
>   a->x = 7; // (1)
>   operator delete (a); (2)  
> 
> On my box, (1) and (2) generated the IPA
> <bb 4> :
>   a_10->a = 7;
>   operator delete (a_10);
> 
> Thus, I'd first only consider types where a destructor is provided (by the
> user or generated).
> Indeed, when a destructor is supplied for a type, <bb 3> becomes something
> akin to :
> 
> struct A
> {
>   ...
>   ~A() {}
> }
> 
> ...
> 
> <bb 3> :
>   A::~A (a.0_12);
>   operator delete (a.0_12, 8);
>  
> 
> The warnings stay the same, though it is now more reliable to check for a
> destructor call, instead any random single assignment. 

There's a sense in which it does make sense to complain about use-after-delete
in the destructor (when the destructor is non-empty): the memory is being
accessed after deletion.  Maybe this case would make more sense to the user? 
(albeit being rather verbose)

> I'm considering
> adding a new state to sm-malloc, RS_DESTROYED, that would also help flag use
> after standalone destruction (without a succeeding deallocation).