RE: [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC)

["Aktemur, Tankut Baris via Gdb-patches" <gdb-patches@sourceware.org>]

On Friday, October 30, 2020 2:14 AM, Pedro Alves wrote:
> On 7/10/20 3:55 AM, Simon Marchi wrote:
> >>> I don't know if it would be worth it, but I'd like if we could assert (abort
> >>> GDB) if an exception does try to exit the destructor.  The `restore` method
> >>> is non-trivial and calls into other non-trivial functions, so it would be
> >>> possible for a change far far away to cause that to happen.
> >>
> >> It will already abort.  Destructors are noexcept by default, so if an exception
> >> escapes a destructor, std::terminate() is called, and that calls abort by default.
> >
> > Oh, didn't know that!  I thought it was just "undefined behavior".
> >
> >>> What do you think of keeping the try/catch, but using `gdb_assert_not_reached`
> >>> in it?
> >>
> >> Not sure.  If we do that, we do get a nicer error message.  However if the user
> >> says "n" to "Quit this debugging session" we still abort.
> >>
> >> /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1441: internal-error:
> scoped_restore_current_thread::~scoped_restore_current_thread(): unexpected exception thrown
> from destructor: hello
> >> A problem internal to GDB has been detected,
> >> further debugging may prove unreliable.
> >> Quit this debugging session? (y or n) n
> >>
> >> This is a bug, please report it.  For instructions, see:
> >> <https://www.gnu.org/software/gdb/bugs/>.
> >>
> >> /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1441: internal-error:
> scoped_restore_current_thread::~scoped_restore_current_thread(): unexpected exception thrown
> from destructor: hello
> >> A problem internal to GDB has been detected,
> >> further debugging may prove unreliable.
> >> Create a core file of GDB? (y or n) n
> >> terminate called after throwing an instance of 'gdb_exception_quit'
> >> Aborted (core dumped)
> >>
> >> Maybe it would be interesting to add a variant of internal_error that did
> >> not throw a quit, so the user could swallow the exception...  Maybe consider
> >> wrapping that as a generic facility to add to all non-trivial RAII destructors
> >> we have?  Like a function that takes a function_view as parameter, so
> >> we would write:
> >>
> >> foo::~foo ()
> >> {
> >>   safe_dtor (__FILE__, __LINE__, [&] ()
> >>     {
> >>       restore ();
> >>     });
> >> }
> >>
> >> Even better, add a SAFE_DTOR macro using similar magic SCOPE_EXIT
> >> macro uses to be able to write:
> >>
> >> foo::~foo ()
> >> {
> >>   SAFE_DTOR { restore (); };
> >> }
> >
> > That's fancier than what I hoped for :).  My goal was just to make sure
> > we catch it if we ever make a change that causes an exception to escape.
> > Although I wouldn't be against what you proposed.
> >
> >> Here's the current version of the patch.
> >
> > That looks fine to me.
> 
> FYI, I've finally merged this to master.
> 
> Thanks,
> Pedro Alves

Hi Pedro,

This patch created the regression below:

  Breakpoint 1, 0x00000000004012bb in eh2 (/gdb-up/gdb/frame.c:641: internal-error: frame_id get_frame_id(frame_info*): Assertion `stashed' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.base/eh_return.exp: hit breakpoint (GDB internal error)

-Baris



Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928