[PATCH 5/6] gdb: Remove cleanup from linux-fork.c:inferior_call_waitpid

[Pedro Alves]

On 01/09/2019 02:10 PM, Andrew Burgess wrote:
> * Andrew Burgess <andrew.burgess@embecosm.com> [2019-01-09 13:33:23 +0000]:
> 
>> * Pedro Alves <palves@redhat.com> [2019-01-09 12:54:59 +0000]:
>>
>>> On 01/01/2019 10:45 PM, Andrew Burgess wrote:

>>> When writing destructors, we need to keep in mind that if an
>>> exception escapes, gdb is terminated on the spot.
>>>
>>> Things aren't usually correct in the cleanup version either,
>>> since an exception escaping while running cleanups leaves
>>> the cleanup chain with dangling cleanups, but I think that
>>> these conversions are the ideal time to fix it up.
>>>
>>> The solution will usually be to swallow exceptions in the
>>> dtor with try/catch(...) and try to limp along.
>>
>> Is it worth using `internal_warning` rather than silently dropping the
>> exception?
> 
> Here's a patch using internal_warning.  If you don't think that's a
> good idea then I can commit without internal_warning and place this
> comment in the catch instead:

I don't think this would be an internal issue?  For example, something
external to GDB could SIGKILL the inferior process, and then calling lseek
in the inferior within fork_load_infrun_state, could throw, for example.

If we print something, it'd be pedantically better to not talk about
gdb functions.

Maybe something around:

    warning (_("Couldn't restore checkpoint state in %s: %s",
	     target_pid_to_str (fp->ptid), ex.message);

?

> 
>     CATCH (ex, RETURN_MASK_ERROR)

Use RETURN_MASK_ALL, since if a Quit/Ctrl-C escapes, RETURN_MASK_ERROR
won't catch it, and GDB dies.

Pedantically, raw C++ try/catch(...), would catch any kind of exception, even
non-GDB ones, but that would mean losing the ex object, unless you complicate
this some more.  We don't throw around non-GDB exceptions (maybe a
std::logic_error could be possible somewhere, but I'd call that a bug),
so I think
  CATCH (ex, RETURN_MASK_ALL) 
is good enough.

>       {
>         /* It's not clear how we should recover from an exception at
>            this point, so for now ignore the error and push on.  */
>       }
That's fine too.  It's what I done in other similar spots, though I confess
that I had assumed that warning() writes to gdb_stdout, and thus could
paginate and cause another exception (ctrl-c/Quit), but now that I look,
I see that warning() writes to gdb_stderr which doesn't paginate.
There's even a comment about that:

/* Print a warning message.  The first argument STRING is the warning
   message, used as an fprintf format string, the second is the
   va_list of arguments for that string.  A warning is unfiltered (not
   paginated) so that the user does not need to page through each
   screen full of warnings when there are lots of them.  */

Eh.  So yeah, a warning seems good.

Thanks,
Pedro Alves