[Bug c++/107531] New: List of references calls destructors, add warning?

[Bug c++/107531] New: List of references calls destructors, add warning?

[nightstrike at gmail dot com]

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

            Bug ID: 107531
           Summary: List of references calls destructors, add warning?
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nightstrike at gmail dot com
  Target Milestone: ---

The following code results in a destructor being calling twice:

#include <initializer_list>
struct S {
    S() { __builtin_printf("ctor\n"); }
    ~S() { __builtin_printf("dtor\n"); }
};

template<typename ... Args>
void f(Args const & ... args) {
    for (auto const & s: { args... })
        ;
}

int main() {
    S s;
    f(s);
}

$ ./a.out
ctor
dtor
dtor


Changing line 9 to use pointers fixes it:

for (auto const * s: { &args... })

$ ./a.out
ctor
dtor


I think that if my code is exploiting some kind of bad behavior (and it likely
is), a warning at -Wall would be a nice addition (currently, there is none). 
If what I'm doing is allowed, I don't think calling a destructor twice is the
best idea :) :).  I'd be betting money on the first thing, though.  Maybe
adding something from -fsanitize=undefined would be an option?

[Bug c++/107531] List of references calls destructors, add warning?

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
My bet is on a copy constructor being invoked.

[Bug c++/107531] List of references calls destructors, add warning?

[nightstrike at gmail dot com]

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

--- Comment #2 from nightstrike <nightstrike at gmail dot com> ---
It looks like you're right.  The root cause of the problem is that in my
non-reduced case, I didn't have a copy constructor, but I had a non-default
destructor that was releasing resources twice.  So it's clearly my fault, I
just kind of hoped the compiler could be a little more omniscient :)

If a warning is not really possible here, then I guess this could be "closed
invalid".  Or maybe there are already PR's open to request a warning to remind
you that you did something requiring a non-default copy constructor?

[Bug c++/107531] List of references calls destructors, add warning?

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Why should the compiler warn here? There are no resources being freed. Making
trivial copies and then destroying them is not a bug.

If you give the class some resources to manage (e.g. a pointer member) then
-Weffc++ will warn that you messed up:

dest.C:2:8: warning: ‘struct S’ has pointer data members [-Weffc++]
    2 | struct S {
      |        ^
dest.C:2:8: warning:   but does not declare ‘S(const S&)’ [-Weffc++]
dest.C:2:8: warning:   or ‘operator=(const S&)’ [-Weffc++]
dest.C:5:10: note: pointer member ‘S::p’ declared here
    5 |     int* p = nullptr;
      |          ^


It would be nice if you could turn on this warning without the rest of -Weffc++
but that's PR 16166.

[Bug c++/107531] List of references calls destructors, add warning?

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to nightstrike from comment #0)
> Maybe adding something from -fsanitize=undefined would be an option?

There's nothing undefined in your example.

But if you actually allocate and free resources, asan aborts on the
double-free:

=================================================================
==1035874==ERROR: AddressSanitizer: attempting double-free on 0x602000000010 in
thread T0:
    #0 0x7f69bd2bc088 in operator delete(void*, unsigned long)
(/lib64/libasan.so.8+0xbc088)
    #1 0x401391 in S::~S() (/tmp/a.out+0x401391)
    #2 0x401247 in main (/tmp/a.out+0x401247)
    #3 0x7f69bca2950f in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7f69bca295c8 in __libc_start_main_impl ../csu/libc-start.c:389
    #5 0x4010f4 in _start (/tmp/a.out+0x4010f4)

0x602000000010 is located 0 bytes inside of 4-byte region
[0x602000000010,0x602000000014)
freed by thread T0 here:
    #0 0x7f69bd2bc088 in operator delete(void*, unsigned long)
(/lib64/libasan.so.8+0xbc088)
    #1 0x401391 in S::~S() (/tmp/a.out+0x401391)
    #2 0x401577 in void f<S>(S const&) (/tmp/a.out+0x401577)
    #3 0x40123b in main (/tmp/a.out+0x40123b)
    #4 0x7f69bca2950f in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58

previously allocated by thread T0 here:
    #0 0x7f69bd2bb188 in operator new(unsigned long)
(/lib64/libasan.so.8+0xbb188)
    #1 0x4012e3 in S::S() (/tmp/a.out+0x4012e3)
    #2 0x40122f in main (/tmp/a.out+0x40122f)
    #3 0x7f69bca2950f in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: double-free (/lib64/libasan.so.8+0xbc088) in
operator delete(void*, unsigned long)
==1035874==ABORTING

[Bug c++/107531] List of references calls destructors, add warning?

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Unsurprisingly, -fanalyzer doesn't say anything useful about this, because it
doesn't support C++. PR 94355 tracks some of that (and the meta-bug that it
blocks tracks more).