Re: [PATCH 2/3] Improve vRun error reporting

[Pedro Alves <pedro@palves.net>]

Hi!

This conflicts with Alexandra's just-merged packet_check_result changes.  I see she has some follow up
patches that would help me here.  I will take a look at those.  For the meantime, disregard this patch.

Pedro Alves

On 2024-02-12 20:01, Pedro Alves wrote:
> After the previous commit, if starting the inferior process with "run"
> (vRun packet) fails, GDBserver reports an error using the "E." verbose
> error packet.  On the GDB side, however, GDB doesn't yet do anything
> with verbose error strings when handling vRun errors.  This commit
> fixes that.
> 
> This makes remote debugging output the same as native output, when
> possible, another small step in the "local/remote parity" project.
> 
> E.g., before, against GNU/Linux GDBserver:
> 
>   (gdb) run
>   Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
>   Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed
> 
> After, against GNU/Linux GDBserver (same as native):
> 
>   (gdb) run
>   Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
>   During startup program exited with code 126.
> 
> Change-Id: Ib386f267522603f554b52a885b15229c9639e870
> ---
>  gdb/remote.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index b58dbd4cb66..dca5add260a 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -2400,6 +2400,19 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name,
>      }
>  }
>  
> +/* Strings that starts with "E.", are verbose error messages, like
> +   "E.ERROR_MESSAGE".  If BUF is such an error message, return a
> +   pointer to message after the period.  Otherwise return NULL.  */
> +
> +static const char *
> +verbose_error_message (const char *buf)
> +{
> +  if (buf[0] == 'E' && buf[1] == '.')
> +    return buf + 2;
> +  else
> +    return nullptr;
> +}
> +
>  static enum packet_result
>  packet_check_result (const char *buf)
>  {
> @@ -2415,7 +2428,7 @@ packet_check_result (const char *buf)
>  
>        /* Always treat "E." as an error.  This will be used for
>  	 more verbose error messages, such as E.memtypes.  */
> -      if (buf[0] == 'E' && buf[1] == '.')
> +      if (verbose_error_message (buf) != nullptr)
>  	return PACKET_ERROR;
>  
>        /* The packet may or may not be OK.  Just assume it is.  */
> @@ -10502,7 +10515,13 @@ remote_target::extended_remote_run (const std::string &args)
>      case PACKET_UNKNOWN:
>        return -1;
>      case PACKET_ERROR:
> -      if (remote_exec_file[0] == '\0')
> +      /* If we have a verbose error message, print just that.  This
> +	 makes remote debugging output the same as native output, when
> +	 possible.  */
> +      if (const char *msg = verbose_error_message (rs->buf.data ());
> +	  msg != nullptr)
> +	error (("%s"), msg);
> +      else if (remote_exec_file[0] == '\0')
>  	error (_("Running the default executable on the remote target failed; "
>  		 "try \"set remote exec-file\"?"));
>        else