Move threads out of jumppad without single step

  Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> I've tested this in all stop/non stop and it works properly.
>
> Basically what happens is that if stabilize_threads is not called in
> the context of linux_resume and that gdbserver needs to report an
> event, it won't since last_resume_kind can be resume_stop.
>
> In the current case gdbserver is in cmd_qtdp, the last command was
> continue (vCont;c) in all stop mode so last_resume_kind is
> resume_stop.
>
> So when going in linux_wait, the event is filtered out by :
>  event_thread = (struct thread_info *)
> 	find_inferior (&all_threads, status_pending_p_callback, &filter_ptid);
>
> Since status_pending_p_callback returns false.
>
> Note that this fix may not the best one... but it may be some progress...
>
> Any ideas are welcome, otherwise I will add it to my patch set and
> there can be more discussion at review.

Hi Antoine,
I don't have an idea to your problem and your fix, but I don't
understand why don't we see this problem before.  I may miss some
details of your problem, so please include these details in your
patches.

What I want to say here is that we still need more tests and works to
get software single step properly/fully engaged with the rest part of
GDBserver before we introduce (fast) tracepoint for ARM into GDBserver.
The software single step in GDBserver isn't fully exercised yet, for
example, GDB still doesn't emit vCont;s and vCont;S to ask GDBserver to
do single step on arm-linux.  If we force GDB to emit vCont;s on
arm-linux, as the patch below does, there are still some problems,

 1. PTRACE_SINGLESTEP is used in ptrace call in GDBserver, which is
 obviously wrong,

 2. PC increment is still incorrect if the single step is requested by
 GDB.  We've had a fix https://sourceware.org/ml/gdb-patches/2015-11/msg00470.html
 to fix the PC increment when if single step is requested by GDBserver
 itself.

I did some fixes but there are still some work to do software single
step in GDBserver requested by GDB.  I don't want to block your fast
tracepoint work, just raise these issues, and let you know my thoughts.
  

On 02/04/2016 11:58 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> I've tested this in all stop/non stop and it works properly.
>>
>> Basically what happens is that if stabilize_threads is not called in
>> the context of linux_resume and that gdbserver needs to report an
>> event, it won't since last_resume_kind can be resume_stop.
>>
>> In the current case gdbserver is in cmd_qtdp, the last command was
>> continue (vCont;c) in all stop mode so last_resume_kind is
>> resume_stop.
>>
>> So when going in linux_wait, the event is filtered out by :
>>   event_thread = (struct thread_info *)
>> 	find_inferior (&all_threads, status_pending_p_callback, &filter_ptid);
>>
>> Since status_pending_p_callback returns false.
>>
>> Note that this fix may not the best one... but it may be some progress...
>>
>> Any ideas are welcome, otherwise I will add it to my patch set and
>> there can be more discussion at review.
>
> Hi Antoine,
> I don't have an idea to your problem and your fix, but I don't
> understand why don't we see this problem before.  I may miss some
> details of your problem, so please include these details in your
> patches.
>

Indeed, in fact I just tried on x86 on master and I got the same problem 
ending with:

<<<< exiting linux_wait_1
../../../gdb/gdbserver/linux-low.c:1922: A problem internal to GDBserver 
has been detected.
unsuspend LWP 24815, suspended=-1

So maybe we should have seen this before ?

Or my test setup is messing up the normal flow.

At least it makes it easier to reproduce, you can run the same thing on 
x86 with master and get the same situation (See my previous mail on the 
exact steps).

If I understand what you mean by details as a proper problem 
description. It's hard to provide all the details in a coherent way for 
me at this time since I'm not sure I understand the problem correctly, 
mainly due to my lack of experience with the control flow and all the 
states that compose this flow.

That's why I asked here, see if you or Pedro could shed some light based 
on a reproducible program and logs. If it's non-trivial for you guys 
also I'll take more time to dig.

> What I want to say here is that we still need more tests and works to
> get software single step properly/fully engaged with the rest part of
> GDBserver before we introduce (fast) tracepoint for ARM into GDBserver.

That's fine with me.

> The software single step in GDBserver isn't fully exercised yet, for
> example, GDB still doesn't emit vCont;s and vCont;S to ask GDBserver to
> do single step on arm-linux.  If we force GDB to emit vCont;s on
> arm-linux, as the patch below does, there are still some problems,
>
>   1. PTRACE_SINGLESTEP is used in ptrace call in GDBserver, which is
>   obviously wrong,
>
>   2. PC increment is still incorrect if the single step is requested by
>   GDB.  We've had a fix https://sourceware.org/ml/gdb-patches/2015-11/msg00470.html
>   to fix the PC increment when if single step is requested by GDBserver
>   itself.
>

I had not tested that at all thanks for working on it.

If there's anything I can do to help please let me know.

> I did some fixes but there are still some work to do software single
> step in GDBserver requested by GDB.  I don't want to block your fast
> tracepoint work, just raise these issues, and let you know my thoughts.
>

Thanks for letting me know that's always appreciated.

On my end my priority is to get the normal tracepoints in first, still a 
few patches waiting for review there.

In the meantime even if the basic fast tracepoint patch set is ready we 
still need to work on the JIT condition evaluation and relocating PC 
relative instructions which should take some time.

Regards,
Antoine