[Bug c++/106965] New: g++ optimization removes assigning 0 to deleted pointer- causes double free.

[Bug c++/106965] New: g++ optimization removes assigning 0 to deleted pointer- causes double free.

[olddra3rd at mozmail dot com]

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

            Bug ID: 106965
           Summary: g++ optimization removes assigning 0 to deleted
                    pointer- causes double free.
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: olddra3rd at mozmail dot com
  Target Milestone: ---

Created attachment 53590
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53590&action=edit
source file of the function implementations.

Dear GNU, how are you?
Using gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1) on a Ubuntu 22.04,
received a double free after using a destructor twice (one explicitly and one
at the end of function) with compiling optimizations..

Comp
Bug occurs when when the call happens on a different compilation unit:

int main()
{
 X x1;
 x1.~X();

 return 0;
}

From looking at the assembly - it seems assigning 0 to the pointer is removed
from the code.

Compilation flags:
g++ -O
Seems to work fine with clang++.

Have a great day!

[Bug c++/106965] g++ optimization removes assigning 0 to deleted pointer- causes double free.

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think it's undefined to invoke a DTOR twice which is what you do here.  After
the DTOR the m_ptr member becomes undefined so re-evaluating that in the second
invocation (when there's no object of type X anymore) is undefined.

[Bug c++/106965] g++ optimization removes assigning 0 to deleted pointer- causes double free.

[olddra3rd at mozmail dot com]

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

--- Comment #2 from Boaz <olddra3rd at mozmail dot com> ---
(In reply to Richard Biener from comment #1)
> I think it's undefined to invoke a DTOR twice which is what you do here. 
> After the DTOR the m_ptr member becomes undefined so re-evaluating that in
> the second invocation (when there's no object of type X anymore) is
> undefined.

Damn, you're right. Was told it's legal, but upon further check seems I was
wrong.
Cheers and thanks.

[Bug c++/106965] g++ optimization removes assigning 0 to deleted pointer- causes double free.

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> I think it's undefined to invoke a DTOR twice which is what you do here. 
> After the DTOR the m_ptr member becomes undefined so re-evaluating that in
> the second invocation (when there's no object of type X anymore) is
> undefined.

Right, and because you can't ever use the m_ptr member after the destructor,
there's no point writing the 0 to it. That's a dead store, so the compiler is
allowed to eliminate it. A correct program can never observe whether that store
happened or not.

[Bug c++/106965] g++ optimization removes assigning 0 to deleted pointer- causes double free.

[olddra3rd at mozmail dot com]

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

--- Comment #4 from Boaz <olddra3rd at mozmail dot com> ---
(In reply to Jonathan Wakely from comment #3)
> (In reply to Richard Biener from comment #1)
> > I think it's undefined to invoke a DTOR twice which is what you do here. 
> > After the DTOR the m_ptr member becomes undefined so re-evaluating that in
> > the second invocation (when there's no object of type X anymore) is
> > undefined.
> 
> Right, and because you can't ever use the m_ptr member after the destructor,
> there's no point writing the 0 to it. That's a dead store, so the compiler
> is allowed to eliminate it. A correct program can never observe whether that
> store happened or not.

My logic in assigning 0 was preventing delete on a dangling pointer in case of
a double call to a destructor (for example, if the object was dynamically
allocated) which is a good practice as far as I know.

But my mistake was indeed calling a destructor explicitly on an automatic
storage, quoting from ISO2020:

"If a variable with automatic storage duration has initialization or a
destructor with side effects, an implemen-
tation shall not destroy it before the end of its block nor eliminate it as an
optimization, even if it appears to
be unused, except that a class object or its copy/move may be eliminated as
specified in 11.10.6."

So... my bad.

[Bug c++/106965] g++ optimization removes assigning 0 to deleted pointer- causes double free.

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Boaz from comment #4)
> which is a good practice as far as I know.

Not really, because it's dead code and typically optimized away anyway.

It's better to use static analysis tools, or dynamic analysis like ASan.