Bug 33799 - Return value's destructor not executed when a local variable's destructor throws
Summary: Return value's destructor not executed when a local variable's destructor throws
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.2.1
: P3 major
Target Milestone: 12.0
Assignee: Jason Merrill
: 101961 (view as bug list)
Depends on:
Reported: 2007-10-17 11:31 UTC by Matti Rintala
Modified: 2023-07-06 01:56 UTC (History)
9 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work: 10.0
Known to fail:
Last reconfirmed: 2007-10-17 17:07:45


Note You need to log in before you can comment on or make changes to this bug.
Description Matti Rintala 2007-10-17 11:31:06 UTC
In C++, if a local variable's destructor throws after the return value object has been created, the return value object is never destroyed!
G++ uses the allowed C++ return value optimisation and creates a return value object directly without copying it. This is probably one source of the bug.

This bug manifests in all versions of g++ (3.2.3 - 4.2.2) I happened to have on my machine, it also shows on both i686 and x86_64 architectures. It seems that throwing destructors are rare enough so that this bug has gone unnoticed for really long... :)

Test case:
-------- etest.cc --------------
#include <iostream>
#include <ostream>
using namespace std;

class X
  X(bool throws) : throws_(throws)
    cerr << "X::X() (" << throws_ << ")" << endl;

  X(const X& x) : throws_(x.throws_)
    cerr << "X::X(const X&) (" << throws_ << ")" << endl;

    cerr << "X::~X() (" << throws_ << ")" << endl;
    if (throws_) { throw 1; }
  bool throws_;

X f()
  X x(true);
  return X(false);

int main()
  catch (...)
    cerr << "Catch!" << endl;
---------- end of etest.cc -----------
pulu:/home/bitti/tmp/t> g++ etest.cc
pulu:/home/bitti/tmp/t> ./a.out
X::X() (1)
X::X() (0)
X::~X() (1)
Comment 1 Richard Biener 2007-10-17 12:03:20 UTC
As a data point, EDG based icpc behaves the same.
Comment 2 Matti Rintala 2007-10-17 12:21:04 UTC
I also tried on other compilers. Sun's compiler (CC: Sun C++ 5.9 SunOS_sparc Patch 124863-01 2007/07/25) shows the same bug as Gcc. Microsoft Visual Studio 2005 works ok and destroys both objects.
Comment 3 Jason Merrill 2007-10-17 17:07:45 UTC
Similar to Bug 15764.
Comment 4 Matti Rintala 2008-09-19 12:12:52 UTC
(In reply to comment #3)
> Similar to Bug 15764.

I ran again into this bug in gcc 4.3.2. Any idea when there's time to fix it?

Matti Rintala

Comment 5 Kenton Varda 2013-11-28 04:12:06 UTC
It's now 2013 and this issue is giving me memory leaks in real code.  Will it ever be fixed?
Comment 6 Jonathan Wakely 2013-11-28 11:18:07 UTC
It's now 2013 so the sensible thing to do is not return by value if your destructor can throw.

FWIW Clang also behaves the same as G++ and Intel, and of course calls std::terminate() in C++11 mode.
Comment 7 Kenton Varda 2013-11-28 12:14:59 UTC
> It's now 2013 so the sensible thing to do is not return by value
> if your destructor can throw.

That actually sounds like a pretty difficult rule to follow, unless you either ban throwing destructors altogether or ban returning by value altogether.

The don't-throw-from-destructors rule is, of course, popular, but not universally agreed upon.  I don't think this bug is the right place to debate it (maybe try http://goo.gl/haB5nm), but I would hope that GCC wouldn't refuse to fix a bug simply because they disagree with the coding style that triggers that bug.

> FWIW Clang also behaves the same as G++ and Intel,

Yes, I noticed, and Clang has also had a bug filed against them which has languished for years.  Nevertheless, the behavior is wrong.

> and of course calls std::terminate() in C++11 mode.

Unless the destructor has noexcept(false).
Comment 8 dakron 2019-11-17 19:23:24 UTC
It seems that this is still the case for all g++ version (up until 9.2). Is there any plan to address that? The behavior seems wrong as there is not strict requirement (is it?) about non-throwing destructors.
Comment 9 CVS Commits 2020-01-13 17:51:32 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r10-5926-g7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jan 10 17:14:12 2020 -0500

    PR c++/33799 - destroy return value if local cleanup throws.
    This is a pretty rare situation since the C++11 change to make all
    destructors default to noexcept, but it is still possible to define throwing
    destructors, and if a destructor for a local variable throws during the
    return, we've already constructed the return value, so now we need to
    destroy it.  I handled this somewhat like the new-expression cleanup; as in
    that case, this cleanup can't properly nest with the cleanups for local
    variables, so I introduce a cleanup region around the whole function and a
    flag variable to indicate whether the return value actually needs to be
    Setting the flag requires giving a COMPOUND_EXPR as the operand of a
    RETURN_EXPR, so I adjust gimplify_return_expr to handle that.
    This doesn't currently work with deduced return type because we don't know
    the type when we're deciding whether to introduce the cleanup region.
    	* gimplify.c (gimplify_return_expr): Handle COMPOUND_EXPR.
    	* cp-tree.h (current_retval_sentinel): New macro.
    	* decl.c (start_preparsed_function): Set up cleanup for retval.
    	* typeck.c (check_return_expr): Set current_retval_sentinel.
Comment 10 CVS Commits 2020-01-15 20:15:29 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r10-5989-g299ddc612136421f1d9865ea4f2f84f7e3791824
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Jan 15 14:13:13 2020 -0500

    Revert "PR c++/33799 - destroy return value if local cleanup throws."
    This change was blocking the coroutines merge, so I'm backing it out for now
    to adjust my approach.
    This reverts commit 7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90.
Comment 11 CVS Commits 2020-01-19 18:58:13 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r10-6077-gbcfc2227c556f2801a657ce3007374732baa8333
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Jan 19 09:14:54 2020 -0500

    PR c++/33799 - destroy return value, take 2.
    This patch differs from the reverted patch for 33799 in that it adds the
    CLEANUP_STMT for the return value at the end of the function, and only if
    we've seen a cleanup that might throw, so it should not affect most C++11
    	* cp-tree.h (current_retval_sentinel): New macro.
    	(struct language_function): Add throwing_cleanup bitfield.
    	* decl.c (cxx_maybe_build_cleanup): Set it.
    	* except.c (maybe_set_retval_sentinel)
    	(maybe_splice_retval_cleanup): New functions.
    	* parser.c (cp_parser_compound_statement): Call
    	* typeck.c (check_return_expr): Call maybe_set_retval_sentinel.
Comment 12 Jason Merrill 2020-01-20 04:18:24 UTC
Fixed for GCC 10.
Comment 13 CVS Commits 2020-01-23 18:11:54 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r10-6183-g20afdcd36982752ba012960b862e9be7154b1274
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jan 23 12:32:02 2020 -0500

    c++: Fix ICE with defaulted destructor and template.
    In a template we don't instantiate a deferred noexcept-spec, and we don't
    need it because we aren't going to do anything with the value of
    throwing_cleanup in a template anyway.
    	PR c++/93345 - ICE with defaulted dtor and template.
    	PR c++/33799
    	* decl.c (cxx_maybe_build_cleanup): Don't try to set
    	throwing_cleanup in a template.
Comment 14 mm-nospam 2020-04-20 03:30:37 UTC
The bug still occurs for the same test case with the f() function modified as such:

    X f()
      try {
         X x(true);
         return X(false);
      } catch(...) { }

      return X(false);

The first X(false) is constructed and never destructed.

This example also occurs in [except.ctor]/2 of the C++17 standard, note that since C++17 X's destructor should be marked as noexcept(false) in the test case.
Comment 15 Jonathan Wakely 2020-04-20 09:24:16 UTC
Reopening. Modified version of g++.dg/eh/return1.C showing the remaining bug, in the new i() function:

// PR c++/33799
// { dg-do run }

extern "C" void abort();

int c, d;

#if __cplusplus >= 201103L
#define THROWS noexcept(false)
#define THROWS

struct X
  X(bool throws) : throws_(throws) { ++c; }
  X(const X& x) : throws_(x.throws_) { ++c; }
    if (throws_) { throw 1; }
  bool throws_;

X f()
  X x(true);
  return X(false);

X g()
  return X(true),X(false);

void h()
#if __cplusplus >= 201103L
  []{ return X(true),X(false); }();

X i()
  try {
    X x(true);
    return X(false);
  } catch(...) {}
  return X(false);

int main()
  try { f(); }
  catch (...) {}

  try { g(); }
  catch (...) {}

  try { h(); }
  catch (...) {}

  try { i(); }
  catch (...) {}

  if (c != d)
Comment 16 Jakub Jelinek 2020-05-07 11:56:27 UTC
GCC 10.1 has been released.
Comment 17 Richard Biener 2020-07-23 06:52:03 UTC
GCC 10.2 is released, adjusting target milestone.
Comment 18 Richard Biener 2021-04-08 12:02:26 UTC
GCC 10.3 is being released, retargeting bugs to GCC 10.4.
Comment 19 Jason Merrill 2022-01-06 14:31:36 UTC
*** Bug 101961 has been marked as a duplicate of this bug. ***
Comment 20 CVS Commits 2022-01-07 00:26:07 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r12-6333-gb10e031458d541f794dfaa08ba606487603a4bb6
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Jan 5 17:01:12 2022 -0500

    c++: destroy retval on throwing cleanup in try [PR33799]
    My earlier attempt to fix this bug didn't handle the case where both the
    return and the throwing cleanup are within a try-block that catches and
    discards the exception.  Fixed by adding the retval cleanup to any
    try-blocks as well as the function body.  PR102191 pointed out that we also
    weren't handling templates properly, so I moved the call out of the parser.
            PR c++/33799
            PR c++/102191
            * except.c (maybe_splice_retval_cleanup): Check
            * semantics.c (do_poplevel): Call it here.
            * parser.c (cp_parser_compound_statement): Not here.
            * g++.dg/eh/return1.C: Add temporary in try block case.
            * g++.dg/cpp2a/constexpr-dtor11.C: New test.
Comment 21 Jason Merrill 2022-02-01 22:51:06 UTC
Fixed for GCC 12.
Comment 22 CVS Commits 2023-06-07 01:33:31 UTC
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:


commit r14-1591-g08cea4e56a094ff9cc7c65fdc9ce8c4d7aff64be
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jun 6 15:31:23 2023 -0400

    c++: fix throwing cleanup with label
    While looking at PR92407 I noticed that the expectations of
    maybe_splice_retval_cleanup weren't being met; an sk_cleanup level was
    confusing its attempt to recognize the outer block of the function.  And
    even if I fixed the detection, it failed to actually wrap the body of the
    function because the STATEMENT_LIST it got only had the label, not anything
    after it.  So I moved the call after poplevel does pop_stmt_list on all the
    sk_cleanup levels.
            PR c++/33799
            * except.cc (maybe_splice_retval_cleanup): Change
            recognition of function body and try scopes.
            * semantics.cc (do_poplevel): Call it after poplevel.
            (at_try_scope): New.
            * cp-tree.h (maybe_splice_retval_cleanup): Adjust.
            * g++.dg/eh/return1.C: Add label cases.