Bug 57510 - subobjects not destroyed when exception thrown during list-initialization
Summary: subobjects not destroyed when exception thrown during list-initialization
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.2
: P2 normal
Target Milestone: 5.0
Assignee: Jason Merrill
URL:
Keywords: wrong-code
: 57930 87637 (view as bug list)
Depends on: 41449
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-03 12:22 UTC by Matt A
Modified: 2020-03-10 20:10 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 10.0
Known to fail:
Last reconfirmed: 2013-06-03 00:00:00


Attachments
Code demonstrating the problem (222 bytes, text/x-csrc)
2013-06-03 12:22 UTC, Matt A
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt A 2013-06-03 12:22:53 UTC
Created attachment 30247 [details]
Code demonstrating the problem

It looks as though initializer_list can suffer from memory leaks - if during construction of the initializer_list members an exception is thrown then the previously allocated members are not correctly destroyed. Unfortunately I do not have access to a newer g++ version than 4.7.2, however I could not see any mention of this issue in the closed bugs.

Code (also attached):
--
#include <memory>
#include <random>

struct X
{
    X () : m_n (std::unique_ptr<int> (new int)) { if (random () & 1) throw 1; }
    std::unique_ptr<int> m_n;
};

void foo (std::initializer_list<X>)
{
}

int main ()
{
  for (int i = 0; i < 10; ++i)
  {
    try
    {
      foo ({ X{}, X{} });
    }
    catch (...) {}
  }
}
--

$ g++ -v
Using built-in specs.
COLLECT_GCC=/software/thirdparty/gcc/4.7.2-0.el6_64/bin/g++
COLLECT_LTO_WRAPPER=/software/thirdparty/gcc/4.7.2-0.el6_64/libexec/gcc/x86_64-unknown-linux-gnu/4.7.2/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../configure --prefix=/software/thirdparty/gcc/4.7.2-0.el6_64 --with-system-zlib --enable-shared --enable-threads=posix --enable-laguages=all
Thread model: posix
gcc version 4.7.2 (GCC)

$ g++ -std=c++11 memory_leak.cpp

$ valgrind ./a.out
==13775== Memcheck, a memory error detector
==13775== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==13775== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==13775== Command: ./a.out
==13775==
==13775==
==13775== HEAP SUMMARY:
==13775==     in use at exit: 12 bytes in 3 blocks
==13775==   total heap usage: 23 allocs, 20 frees, 1,244 bytes allocated
==13775==
==13775== LEAK SUMMARY:
==13775==    definitely lost: 12 bytes in 3 blocks
==13775==    indirectly lost: 0 bytes in 0 blocks
==13775==      possibly lost: 0 bytes in 0 blocks
==13775==    still reachable: 0 bytes in 0 blocks
==13775==         suppressed: 0 bytes in 0 blocks
==13775== Rerun with --leak-check=full to see details of leaked memory
==13775==
==13775== For counts of detected and suppressed errors, rerun with: -v
==13775== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)
Comment 1 Jonathan Wakely 2013-06-03 13:09:38 UTC
Testcase without <memory and <random> and not requiring valgrind:

#include <initializer_list>

struct counter
{
  static int n;

  counter() { ++n; }
  counter(const counter&) { ++n; }
  ~counter() { --n; }
};

int counter::n = 0;

struct X
{
    X () { if (counter::n > 1) throw 1; }

    counter c;
};

int main ()
{
  try
  {
    auto x = { X{}, X{} };
  }
  catch (...)
  {
    if ( counter::n != 0 )
      throw;
  }
}
Comment 2 Paolo Carlini 2013-06-03 14:27:59 UTC
Thus I would say wrong code and P2.
Comment 3 Jonathan Wakely 2013-07-22 10:40:03 UTC
*** Bug 57930 has been marked as a duplicate of this bug. ***
Comment 4 Tavian Barnes 2014-12-08 21:03:28 UTC
I have a testing tool that automatically inserts operator new failures, to help test exception safety and check for leaks.  This bug causes all kinds of spurious failures that I have to work around, since anything like this

    vector<string> vec = {"some", "strings"};

leaks memory if the second string constructor fails.  Usually it can be worked around like this

    string arr[] = {"some", "strings"};

but it's still quite annoying.

If someone can point me in the right direction or give an outline of how to fix this bug, I'm happy to try and write up a patch myself.  Thanks!
Comment 5 Jason Merrill 2014-12-12 03:49:27 UTC
Author: jason
Date: Fri Dec 12 03:48:55 2014
New Revision: 218653

URL: https://gcc.gnu.org/viewcvs?rev=218653&root=gcc&view=rev
Log:
	PR c++/57510
	* typeck2.c (split_nonconstant_init_1): Handle arrays here.
	(store_init_value): Not here.
	(split_nonconstant_init): Look through TARGET_EXPR.  No longer static.
	* cp-tree.h: Declare split_nonconstant_init.
	* call.c (set_up_extended_ref_temp): Use split_nonconstant_init.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/initlist90.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/typeck2.c
Comment 6 Jason Merrill 2014-12-12 03:57:54 UTC
Fixed for GCC 5.
Comment 7 Jason Merrill 2015-01-23 16:30:37 UTC
Author: jason
Date: Fri Jan 23 16:30:00 2015
New Revision: 220047

URL: https://gcc.gnu.org/viewcvs?rev=220047&root=gcc&view=rev
Log:
	PR c++/64314
	PR c++/57510
	* typeck2.c (split_nonconstant_init_1): Remove a sub-CONSTRUCTOR
	that has been completely split out.

Added:
    trunk/gcc/testsuite/g++.dg/init/array38.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/typeck2.c
Comment 8 Jonathan Wakely 2018-08-17 09:56:05 UTC
Comment 1 was fixed, but comment 0 was not, this bug is still present on trunk. The example in comment 0 built with ASan shows:


=================================================================
==6556==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x7ffa9489a340 in operator new(unsigned long) /home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cc:90
    #1 0x400ef6 in X::X() /tmp/leak2.cc:6
    #2 0x400db1 in main /tmp/leak2.cc:20
    #3 0x7ffa93b33f29 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 2 allocation(s).



Possibly a dup of PR 66139
Comment 9 Jonathan Wakely 2018-08-17 09:59:59 UTC
The example from PR 57930 isn't fixed either:

extern "C" int printf(const char *,...);
struct B {
	B(int,int) { printf("CB %p\n",this); }
	B(const B&) { printf("const CB %pn\n",this); }
	B(B&&) { printf("const CB %pn\n",this); }
	~B() { printf("B %p\n",this);  }
};
struct C {
	int c;
	int c2;
};
struct A {
	 struct B b;
	 struct C c;
};
A test() {
	//const A a1 = { { 1, 2 }, { 3, (throw 9, 4) } } ; // destructor for B called
	//const A &a2 = { { 1, 2 }, { 3, (throw 9, 4) } } ; // destructor for B not called
	return { { 1, 2 }, { 3, (throw 9, 4) } } ; // destructor for B not called
};
int main() {
	try {
		test();
	} catch (...) {
	}
}

This prints:

CB 0x7ffd6eac7620


But should be:

CB 0x7ffd6eac7620
B 0x7ffd6eac7620
Comment 10 Jonathan Wakely 2018-10-18 09:13:07 UTC
Another example provided by Hubert in PR 87637:

In the following program, the initialization of the A subobject of the B temporary associated with the brace-initializing cast expression is complete when an exception is thrown during the further initialization of the B temporary.

When compiled with GCC, stack unwinding for the exception fails to invoke the destructor of the A subobject.

### SOURCE (<stdin>):
extern "C" int printf(const char *, ...);

struct A {
  A() { printf("%s\n", __PRETTY_FUNCTION__); }
  A(const A &) = delete;
  ~A() { printf("%s\n", __PRETTY_FUNCTION__); }
};

struct B { A a; int q; };

int foo() { throw 0; }

int main(void) {
  try {
    (void) B{{}, foo()};
  }
  catch (...) { }
}


### COMPILER INVOCATION:
g++ -x c++ -std=c++11 -o prog -


### RUN INVOCATION:
./prog


### ACTUAL RUN OUTPUT:
A::A()


### EXPECTED RUN OUTPUT:
A::A()
A::~A()


### COMPILER VERSION INFO (g++ -v):
Using built-in specs.
COLLECT_GCC=/opt/wandbox/gcc-head/bin/g++
COLLECT_LTO_WRAPPER=/opt/wandbox/gcc-head/libexec/gcc/x86_64-pc-linux-gnu/9.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../source/configure --prefix=/opt/wandbox/gcc-head --enable-languages=c,c++ --disable-multilib --without-ppl --without-cloog-ppl --enable-checking=release --disable-nls --enable-lto LDFLAGS=-Wl,-rpath,/opt/wandbox/gcc-head/lib,-rpath,/opt/wandbox/gcc-head/lib64,-rpath,/opt/wandbox/gcc-head/lib32
Thread model: posix
gcc version 9.0.0 20181016 (experimental) (GCC)
Comment 11 Jonathan Wakely 2018-10-18 09:13:08 UTC
*** Bug 87637 has been marked as a duplicate of this bug. ***
Comment 12 Jason Merrill 2019-12-19 14:07:19 UTC
Author: jason
Date: Thu Dec 19 14:06:45 2019
New Revision: 279576

URL: https://gcc.gnu.org/viewcvs?rev=279576&root=gcc&view=rev
Log:
	PR c++/66139 - EH cleanups for partially constructed aggregates.

There were several overlapping PRs about failure to clean up fully
constructed subobjects when an exception is thrown during aggregate
initialization of a temporary.  I fixed this for non-temporaries in the
context of 57510, but that fix didn't handle temporaries.  So this patch
does split_nonconstant_init at gimplification time, which is much smaller
than alternatives I tried.

	PR c++/57510
	* cp-gimplify.c (cp_gimplify_init_expr): Use split_nonconstant_init.
	* typeck2.c (split_nonconstant_init): Handle non-variable dest.
	(split_nonconstant_init_1): Clear TREE_SIDE_EFFECTS.
	* tree.c (is_local_temp): New.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/initlist116.C
    trunk/gcc/testsuite/g++.dg/cpp0x/initlist117.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-gimplify.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck2.c
    trunk/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
Comment 13 Jason Merrill 2019-12-19 14:13:58 UTC
Fixed for GCC 10 so far.
Comment 14 Jason Merrill 2020-03-10 20:10:29 UTC
Let's close this one and handle the remaining cases under PRs 52320 and 66139.