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

["vultkayn at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

--- Comment #6 from Benjamin Priour <vultkayn at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #5)
> (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
> 

Wouldn't considering "clobber" as a trigger for double-delete also lead to many
false positives ?
I'm not yet 100% familiar with and when this "clobber" appears.

[...snip...]

> > 
> > 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 believe in the case of a non-empty destructor, "double-delete" would be more
on point than "use-after-delete", as ultimately the issue is the second call to
delete. "double-delete" immediately warns about the actual issue, whereas if
the first "delete" is not as obvious as in the above test case, a
"use-after-delete" might confuse the user.

"use-after-delete" could mislead the user to believe there is something wrong
with their destructor, although only their double delete is.