[Bug c++/100495] New: constexpr virtual destructor incorrectly reports memory leak

[Bug c++/100495] New: constexpr virtual destructor incorrectly reports memory leak

[ldalessandro at gmail dot com]

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

            Bug ID: 100495
           Summary: constexpr virtual destructor incorrectly reports
                    memory leak
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ldalessandro at gmail dot com
  Target Milestone: ---

When using a constexpr virtual destructor gcc fails to match delete with
corresponding new.


```
struct Foo {
    constexpr virtual ~Foo() {}
};

constexpr bool foo() {
    Foo *ptr = new Foo();
    delete ptr;
    return true;
}

static_assert(foo()); //error: 'foo()' is not a constant expression because
allocated storage has not been deallocated
```


https://godbolt.org/z/W6haTEvzs

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[ldalessandro at gmail dot com]

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

--- Comment #1 from Luke Dalessandro <ldalessandro at gmail dot com> ---
A short-term workaround for this appears to be explicit allocator usage (works
back at least to 10.2).

```
#include <memory>

struct Foo {
    constexpr virtual ~Foo() {}
};

constexpr bool foo() {
    std::allocator<Foo> alloc = {};

    Foo *ptr = alloc.allocate(1);
    std::construct_at(ptr);
    std::destroy_at(ptr);
    alloc.deallocate(ptr, 1);
    return true;
}

static_assert(foo());
```

https://godbolt.org/z/ohKE6h3Px

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[ldalessandro at gmail dot com]

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

--- Comment #2 from Luke Dalessandro <ldalessandro at gmail dot com> ---
It's also possible to workaround this with array allocation.

```
struct Foo {
    constexpr virtual ~Foo() {}
};

constexpr bool foo() {
    Foo *ptr = new Foo[1]{};
    delete [] ptr;
    return true;
}

static_assert(foo());
```

Obviously both of the workarounds require that we be able to cast from a `Foo*`
to our derived pointer, at which point we might as well skip the virtual
destructor and add an abstract `virtual void destroy() = 0` member where we can
just call `delete this` anyway.

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-05-10

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
constexpr.c (cxx_eval_call_expression) has:
  if (DECL_CLONED_FUNCTION_P (fun))
    fun = DECL_CLONED_FUNCTION (fun);
and similarly constexpr.c (maybe_save_constexpr_fundef) has:
  if (processing_template_decl
      || !DECL_DECLARED_CONSTEXPR_P (fun)
      || cp_function_chain->invalid_constexpr
      || DECL_CLONED_FUNCTION_P (fun))
    return;
which means instead of calling during constexpr evaluation the deleting
destructor (_ZN3FooD0Ev) we call the destructor it is cloned from (_ZN3FooD4Ev)
and that means we don't constexpr evaluate the actual delete operator call.
I guess it would be possible to remember
DECL_DELETING_DESTRUCTOR_P (fun) from before the fun = DECL_CLONED_FUNCTION
(fun); and constexpr evaluate a delete operator call, but I'm afraid I have no
idea how this works for constructors or any other cloned function either, how
do we figure out what arguments to bind to e.g. in_chrg argument etc.?

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |friedkeenan at protonmail dot com

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 102167 has been marked as a duplicate of this bug. ***

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, for constructors this is likely a non-issue, because in_chrg etc. only
appears on constructors of classes with virtual bases and such constructors are
not constexpr.

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51415
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51415&action=edit
gcc12-pr100495.patch

The easiest fix seems to be to just remember the deleting dtor bodies and
constexpr evaluating them, they are quite special anyway, as they are
created by build_delete_destructor_body and essentially contain just two calls,
so there is no user code in those and thus practically they aren't really
clones of those user functions - they just call some other dtor fndecl (with
the user code in it) and delete.

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:81f9718139cb1cc164ada411ada8cca9f32b8be8

commit r12-3387-g81f9718139cb1cc164ada411ada8cca9f32b8be8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 7 19:33:28 2021 +0200

    c++: Fix up constexpr evaluation of deleting dtors [PR100495]

    We do not save bodies of constexpr clones and instead evaluate the bodies
    of the constexpr functions they were cloned from.
    I believe that is just fine for constructors because complete vs. base
    ctors differ only in classes that have virtual bases and such constructors
    aren't constexpr, similarly complete/base destructors.
    But as the testcase below shows, for deleting destructors it is not fine,
    deleting dtors while marked as clones in fact are just artificial functions
    with synthetized body which calls the user destructor and deallocation.

    So, either we'd need to evaluate the destructor and afterwards synthetize
    and evaluate the deallocation, or we can just save and use the deleting
    dtors bodies.  The latter seems much easier to me.

    2021-09-07  Jakub Jelinek  <jakub@redhat.com>

            PR c++/100495
            * constexpr.c (maybe_save_constexpr_fundef): Save body even for
            constexpr deleting dtors.
            (cxx_eval_call_expression): Don't use DECL_CLONED_FUNCTION for
            deleting dtors.

            * g++.dg/cpp2a/constexpr-new21.C: New test.

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9f300873f6bf8456ebdbd40d0211aefe57f95cb5

commit r11-8968-g9f300873f6bf8456ebdbd40d0211aefe57f95cb5
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 7 19:33:28 2021 +0200

    c++: Fix up constexpr evaluation of deleting dtors [PR100495]

    We do not save bodies of constexpr clones and instead evaluate the bodies
    of the constexpr functions they were cloned from.
    I believe that is just fine for constructors because complete vs. base
    ctors differ only in classes that have virtual bases and such constructors
    aren't constexpr, similarly complete/base destructors.
    But as the testcase below shows, for deleting destructors it is not fine,
    deleting dtors while marked as clones in fact are just artificial functions
    with synthetized body which calls the user destructor and deallocation.

    So, either we'd need to evaluate the destructor and afterwards synthetize
    and evaluate the deallocation, or we can just save and use the deleting
    dtors bodies.  The latter seems much easier to me.

    2021-09-07  Jakub Jelinek  <jakub@redhat.com>

            PR c++/100495
            * constexpr.c (maybe_save_constexpr_fundef): Save body even for
            constexpr deleting dtors.
            (cxx_eval_call_expression): Don't use DECL_CLONED_FUNCTION for
            deleting dtors.

            * g++.dg/cpp2a/constexpr-new21.C: New test.

    (cherry picked from commit 81f9718139cb1cc164ada411ada8cca9f32b8be8)

[Bug c++/100495] constexpr virtual destructor incorrectly reports memory leak

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.3+/12+ so far, probably should be backported for 10.4 too.