Re: [PATCH v3 09/17] Teach non-stop to do in-line step-overs (stop all, step, restart)

Pedro Alves
2015-04-22T20:03:15Z

On 04/21/2015 04:01 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> - that adjust_pc_after_break doesn't end up confused. */
>> - gdb_assert (sig != GDB_SIGNAL_0);
>> + that adjust_pc_after_break doesn't end up confused.
>> +
>> + - In non-stop if we insert a breakpoint (e.g., a step-resume)
>> + in one thread after another thread that was stepping had been
>> + momentarily paused for a step-over. When we re-resume the
>> + stepping thread, it may be resumed from that address with a
>> + breakpoint that hasn't trapped yet. Seen with
>> + gdb.threads/non-stop-fair-events.exp, on targets that don't
>> + do displaced stepping. */
>
> Does this comment mean the sequence like this?
>
> thread B is paused,
> a breakpoint is inserted at address PC for thread C,
> thread B may be resumed from address PC

Yep. In that case, we should _not_ step over the breakpoint,
as thread B has not reported that breakpoint event. Detecting the
case and reporting the breakpoint immediately without actually
resuming isn't correct, as in that case we'd lose the signal.
It's just simpler to let the breakpoint trap.

>
>>
>> +/* Select a thread at random, out of those which are resumed and have
>> + had events. */
>> +
>> +static struct thread_info *
>> +random_pending_event_thread (ptid_t waiton_ptid)
>> +{
>
> If GDB core knows select events at random, then we don't have to do
> something similar in linux-nat.c:select_event_lwp, right?
>

I think so, yes. Though I'd rather leave it be for a while,
so that the "maint set target-non-stop off" fallback option
is fully functional as backup plan. In the long term, if we manage
to merge linux-nat.c and gdbserver/linux-low.c, the resulting
code will still need it as long as we'd like to support gdbserver
connected to a gdb that doesn't force non-stop mode though, and,
probably as long as gdbserver's own step-over-breakpoints/tracepoints
implementation is baked in the backend too. :-/

Thanks,
Pedro Alves