Re: [ping] [PATCH 1/2] gdbserver: catch fetch registers error

[Luis Machado <lgustavo@codesourcery.com>]

On 01/04/2017 01:20 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Luis Machado [mailto:lgustavo@codesourcery.com]
>> Sent: Tuesday, January 3, 2017 7:29 PM
>> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
>> patches@sourceware.org
>> Cc: Daniel Jacobowitz <drow@false.org>; Pedro Alves (palves@redhat.com)
>> <palves@redhat.com>
>> Subject: Re: [ping] [PATCH 1/2] gdbserver: catch fetch registers error
>
> Hi Luis,
>
> Thanks for your feedback.
>
>>>> When the PTRACE_PEEKUSER ptrace request to read registers fails, gdbserer
>>>> throws
>>>> an error that is caught in captured_main, where it causes a E01 error packet to
>>>> be sent and gdbserer to quit (if --once was specified) or the event loop to be
>>>> re-started (otherwise).
>>>>
>>>> We may get such ptrace errors when trying to fetch registers for an exited or
>>>> running thread.  There are checks in GDB that check those conditions and
>> throw
>>>> meaningful error messages before we could run into the above ptrace error,
>>>> e.g. thread.c:validate_registers_access.
>>>>
>>>> I ran into a new case and, rather than adding another call to
>>>> validate_registers_access in GDB, I propose to catch the error already when
>>>> handling the 'g' packet in gdbserver and reply with an error packet - assuming
>>>> that gdbserver's internal state is still intact.
>>>>
>>>> To not replace a meaningful error message with E01, I'm trying to generate a
>>>> useful error message when the error is detected and the exception is thrown.
>>>>
>>>> It would look like this ...
>>>>
>>>> gdb) PASS: gdb.btrace/enable-running.exp: continue to breakpoint: cont to 44
>>>> cont&
>>>> Continuing.
>>>> (gdb) PASS: gdb.btrace/enable-running.exp: cont&
>>>> record btrace
>>>> warning: Remote failure reply: E.Selected thread is running.
>>>> warning: Remote failure reply: E.Selected thread is running.
>>>>
>>>> ... although in this particular case, I'm going to suppress the warning.
>>>>
>>>> To make this look a bit nicer, we could consider stripping the "E." or the
>>>> entire "Remote failure reply: E." when (re-)throwing the error inside GDB in
>>>> remote.c.
> [...]
>> Is this a guaranteed recoverable scenario? I've seen GDB get confused
>> and mess up its internal state multiple times when it can't fetch
>> something essential like memory or registers.
>>
>> So, even if we handle things gracefully in gdbserver, does GDB handle
>> that gracefully enough to carry on with a debugging session?
>
> At the moment, GDBserver quits or starts over.  This is not recoverable as
> GDB doesn't have a clue that GDBserver just started over.
>
> This patch improves the situation by making GDBserver send an appropriate error
> message to GDB and resume normally.  It suffices in one concrete case (see next
> patch in series).  If in other cases GDB does not handle the error, I'd say we fix GDB
> for those cases.

I missed the second entry in the series. I'll check it.