This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[ping v2] [PATCH 1/2] gdbserver: catch fetch registers error
- From: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- To: Daniel Jacobowitz <drow at false dot org>, "Pedro Alves (palves at redhat dot com)" <palves at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Thu, 19 Jan 2017 08:59:05 +0000
- Subject: [ping v2] [PATCH 1/2] gdbserver: catch fetch registers error
- Authentication-results: sourceware.org; auth=none
ping
> > -----Original Message-----
> > From: Metzger, Markus T
> > Sent: Tuesday, December 6, 2016 4:55 PM
> > To: gdb-patches@sourceware.org
> > Cc: Daniel Jacobowitz <drow@false.org>
> > Subject: [PATCH 1/2] gdbserver: catch fetch registers error
> >
> > 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.
> >
> > CC: Daniel Jacobowitz <drow@false.org>
> >
> > 2016-12-06 Markus Metzger <markus.t.metzger@intel.com>
> >
> > gdbserver/
> > * server.c (process_serial_event): Add TRY/CATCH.
> > * linux-low.c (fetch_register): Improve error message.
> > ---
> > gdb/gdbserver/linux-low.c | 19 ++++++++++++++++++-
> > gdb/gdbserver/server.c | 18 ++++++++++++++++--
> > 2 files changed, 34 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> > index e3e372c..a942b87 100644
> > --- a/gdb/gdbserver/linux-low.c
> > +++ b/gdb/gdbserver/linux-low.c
> > @@ -5692,7 +5692,24 @@ fetch_register (const struct usrregs_info *usrregs,
> > (PTRACE_TYPE_ARG3) (uintptr_t) regaddr, (PTRACE_TYPE_ARG4)
> > 0);
> > regaddr += sizeof (PTRACE_XFER_TYPE);
> > if (errno != 0)
> > - error ("reading register %d: %s", regno, strerror (errno));
> > + {
> > + /* ESRCH could mean that the thread is not traced, exited, or is not
> > + stopped. */
> > + if (errno == ESRCH)
> > + {
> > + struct lwp_info *lwp = get_thread_lwp (current_thread);
> > +
> > + if (!lwp_is_stopped (lwp))
> > + error (_("Selected thread is running."));
> > +
> > + if (lwp_is_marked_dead (lwp))
> > + error (_("Selected thread has terminated."));
> > + }
> > +
> > + /* Report a generic error if we could not determine the exact
> > + reason. */
> > + error (_("Could not read register %d: %s."), regno, strerror (errno));
> > + }
> > }
> >
> > if (the_low_target.supply_ptrace_register)
> > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> > index ef8dd03..3064b4f 100644
> > --- a/gdb/gdbserver/server.c
> > +++ b/gdb/gdbserver/server.c
> > @@ -4132,8 +4132,22 @@ process_serial_event (void)
> > write_enn (own_buf);
> > else
> > {
> > - regcache = get_thread_regcache (current_thread, 1);
> > - registers_to_string (regcache, own_buf);
> > + TRY
> > + {
> > + regcache = get_thread_regcache (current_thread, 1);
> > + registers_to_string (regcache, own_buf);
> > + }
> > + CATCH (exception, RETURN_MASK_ALL)
> > + {
> > + const char *message;
> > +
> > + message = exception.message;
> > + if (message == NULL)
> > + message = _("Reading registers failed.");
> > +
> > + sprintf (own_buf, "E.%s", message);
> > + }
> > + END_CATCH
> > }
> > }
> > break;
> > --
> > 1.8.3.1
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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928