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

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

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