Bug 66139 - destructor not called for members of partially constructed anonymous struct/array
Summary: destructor not called for members of partially constructed anonymous struct/a...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
: 80664 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-05-13 20:14 UTC by Frank Heckenbach
Modified: 2017-12-12 16:16 UTC (History)
27 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.8.5, 6.3.0, 7.1.0, 8.0
Last reconfirmed: 2017-04-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Heckenbach 2015-05-13 20:14:31 UTC
The attached test program tries to construct various structures
consisting of two members of some type a, where construction of the
first one succeeds and the second one throws, and checks whether the
destructor of the first one was called before the exception is
caught.

Since the first member was constructed successfully, I'd expect its
destructor to be called. Granted, I don't know the exact standard
wording, but my general idea is that once a constructor call
succeeds, an object starts to exist, thus it must at some point
cease to exist, and then its destructor must be called. clang 3.5.0
and visual c++ (according to http://webcompiler.cloudapp.net/,
without the GCC extension "(a[2]) { ... }") do call the destructor
every time.

The g++ output is:

destructor called
destructor not called
destructor called
destructor not called
destructor not called
destructor not called

So with a named struct/array variable, the destructor gets called,
with anonymous ones it doesn't.

Even with a named initializer_list it doesn't, but I suppose that's
because the compiler first constructs an anonymous array (and then
would construct the initializer_list if it got there).

#include <iostream>

int c, d;

struct a
{
  a (int i) { if (i) throw i; c++; }
  ~a () { d++; }
};

void check (void (*f) ())
{
  try
  {
    c = d = 0;
    f ();
  }
  catch (int)
  {
    std::cerr << (c != 1 ? "constructor not completed exactly once\n" :
                  d == 0 ? "destructor not called\n" :
                  d == 1 ? "destructor called\n" :
                           "destructor called more than once\n");
    return;
  }
  std::cerr << "exception not thrown\n";
}

int main ()
{
  struct s { a x, y; };
  check ([] { s t { 0, 1 }; });
  check ([] { s { 0, 1 }; });
  check ([] { a t[2] { 0, 1 }; });
  check ([] { (a[2]) { 0, 1 }; });
  check ([] { std::initializer_list <a> t { 0, 1 }; });
  check ([] { std::initializer_list <a> { 0, 1 }; });
}
Comment 1 Mikhail Kremnyov 2017-03-10 12:35:35 UTC
This issue is also present in GCC 5.x, 6.x and the trunk.

I wonder if it's possible to increase its priority since it makes it impossible to guarantee code exception safety.
Comment 2 Tomasz Kamiński 2017-04-18 20:36:05 UTC
Example of some real life safe-code (now raw allocations, STL container used), that is leaking:
#include <string>
#include <vector>
#include <iostream>

using namespace std;


struct Test {
    string a;
    string b;
};

Test make_test() {
    vector<string> strings {"123456789abcdef0"};
    return Test {
        strings.at(0),
        strings.at(2) // throws; the destructor for a copy of strings.at(0) is not called and memory leakds
    };
}

int main() {
    try {
        Test test = make_test();
    } catch (exception &e) {
        cerr << e.what() << endl;
    }
}

Live code proving leaks: https://wandbox.org/permlink/GjjxFw9jAE9bfvyS
Comment 3 Carlo Wood 2017-05-05 16:06:57 UTC
Still present in 7.x and 8 (HEAD).
clang does not show this behavior (no leak).

Possibly related:
https://wandbox.org/permlink/yJM4fr9Z9D8GN6on
which also shows different behavior from clang
(found on https://akrzemi1.wordpress.com/2017/04/27/a-serious-bug-in-gcc/)

Can anyone confirm that the keyword 'wrong-code' also
applies to that latter example?
Comment 4 Richard Biener 2017-05-08 11:25:43 UTC
*** Bug 80664 has been marked as a duplicate of this bug. ***
Comment 5 Andrzej Krzemienski 2017-05-08 12:00:05 UTC
What does this mean that the status of this bug report is "NEW"? It is 2 years old. In GCC Bugzilla one can assign status "CONFIRMED" to bug reports. Why is this one not confirmed? Was nobody able to confirm that this bug exists in GCC?

It really looks serious, as it undermines C++'s exception safety rules and guarantees.
Comment 6 Markus Trippelsdorf 2017-05-08 12:13:45 UTC
(In reply to Andrzej Krzemienski from comment #5)
> What does this mean that the status of this bug report is "NEW"? It is 2
> years old. In GCC Bugzilla one can assign status "CONFIRMED" to bug reports.
> Why is this one not confirmed? Was nobody able to confirm that this bug
> exists in GCC?
> 
> It really looks serious, as it undermines C++'s exception safety rules and
> guarantees.

Calm down. NEW means confirmed, otherwise it would be UNCONFIRMED.
Writing trollish blog posts won't get the bug fixed any sooner.
Comment 7 Xi Ruoyao 2017-05-08 12:15:26 UTC
std::basic_string<...> is too large.  Replace it with a dummy
default constructable and copyable class Foo.  Then get GIMPLE:

          _1 = &<retval>->a;
          _2 = std::vector<Foo>::at (&strings, 0);
          Foo::Foo (_1, _2);
          _3 = &<retval>->b;
          _4 = std::vector<Foo>::at (&strings, 2);
          Foo::Foo (_3, _4);
          return <retval>;

No exception handling code here.
Comment 8 Markus Trippelsdorf 2017-05-10 12:29:03 UTC
*** Bug 80683 has been marked as a duplicate of this bug. ***
Comment 9 Jaak Ristioja 2017-05-10 13:04:05 UTC
According to a Stack Overflow answer [1] this bug occurs when the constructor is the first thing executed in the try-block. For example:


#include <iostream>
struct A { A(int e) { throw e; } };

struct B {
    A a{42}; // Same with = 42; syntax
};

int main() {
    try {
        // Remove this printout line to trigger the bug:
        std::cout << "Welcome" << std::endl; 
        B b;
    } catch (int e) {
        return e;
    }
}


  [1]: http://stackoverflow.com/a/43892501/3919155
Comment 10 Ondřej Majerech 2017-05-10 13:50:45 UTC
That SO answer appears to be plain out wrong. Running your snippet on GCC 6.3.1 and 8.0.0 20170507, the program calls terminate for me, even with the cout << "Welcome" line included.
Comment 11 Xi Ruoyao 2017-05-10 14:41:00 UTC
This happens for all TARGET_EXPRs with the third operand (cleanup expression),
as an INIT_EXPR's rhs.

The cleanup sequence are pushed in gimplify_target_expr, which doesn't
handle TARGET_EXPRs as the INIT_EXPRs' rhs.  So they just go missing.
Comment 12 Xi Ruoyao 2017-05-10 14:53:22 UTC
(In reply to Jaak Ristioja from comment #9)
>   [1]: http://stackoverflow.com/a/43892501/3919155

I don't think this is the same bug.
This bug seems happening because GCC created "constexpr B::B(void)", but actually
it throws, so can not be constexpr.
Comment 13 Ondřej Majerech 2017-05-10 15:19:13 UTC
(In reply to Xi Ruoyao from comment #12)
> (In reply to Jaak Ristioja from comment #9)
> >   [1]: http://stackoverflow.com/a/43892501/3919155
> 
> I don't think this is the same bug.
> This bug seems happening because GCC created "constexpr B::B(void)", but
> actually
> it throws, so can not be constexpr.

I would also think that to be a different bug, but when I reported that (bug 80683), it got marked as a duplicate of this one. Maybe the duplicate classification of 80683 should be re-reviewed?