[1/2,gdbserver] assert on step if !can_hardware_single_step

  From: Yao Qi <yao.qi@linaro.org>

GDB sends vCont;s by mistake to GDBserver on arm target which doesn't
have single step at all.  However, it is hard to find the problem from
the debugging log.  With this patch applied, the problem is easy to
identify, like:

(gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: switch to thread 6 to step it
step&^M
(gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: set 6 thread stepping
thread /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:3686: A problem internal to GDBserver has been detected.^M
linux_resume_one_lwp_throw: Assertion `step == 0' failed.

gdb/gdbserver:

2015-04-02  Yao Qi  <yao.qi@linaro.org>

	* linux-low.c (linux_resume_one_lwp_throw): Assert on step.
---
 gdb/gdbserver/linux-low.c | 3 +++
 1 file changed, 3 insertions(+)
  

On 04/07/2015 04:52 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
> 
> GDB sends vCont;s by mistake to GDBserver on arm target which doesn't
> have single step at all.  However, it is hard to find the problem from
> the debugging log.  With this patch applied, the problem is easy to
> identify, like:
> 
> (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: switch to thread 6 to step it
> step&^M
> (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: set 6 thread stepping
> thread /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:3686: A problem internal to GDBserver has been detected.^M
> linux_resume_one_lwp_throw: Assertion `step == 0' failed.
> 
> gdb/gdbserver:
> 
> 2015-04-02  Yao Qi  <yao.qi@linaro.org>
> 
> 	* linux-low.c (linux_resume_one_lwp_throw): Assert on step.
> ---
>  gdb/gdbserver/linux-low.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index e4c5420..bc6ab1ae 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -3682,6 +3682,9 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
>    if (the_low_target.prepare_to_resume != NULL)
>      the_low_target.prepare_to_resume (lwp);
>  
> +  if (!can_hardware_single_step ())
> +    gdb_assert (step == 0);


Yeah, I have something like that on my x86 software single-step
branch, on the native side, and also in infrun.c.  See:

 https://github.com/palves/gdb/commits/palves/x86_software_single_step
 https://github.com/palves/gdb/commit/52940835548c124a80bd6f381f1a463eda9bab4c

( I just realized/recalled the top commit fixes the exact same as
your patch #2 :-) )

So I think your patch is a good idea.  :-)  But as you're suggesting
it for inclusion, I have to raise the bar a little.  I think gdbserver
crashing/exiting due to bad input from gdb isn't ideal.  This isn't
gdbserver's fault after all.  I think this should be an error instead.
Or perhaps even better, this could stay as an assert in the backend,
if server.c errors out earlier, even, while parsing the
vCont;s / s packets.

(
One nit, as step is a boolean, I think:

   gdb_assert (!step);

would read more naturally.
)

Thanks,
Pedro Alves