Re: [PATCH v3 05/17] Embed the pending step-over chain in thread_info objects

[Doug Evans <dje@google.com>]

Pedro Alves writes:
 > In order to teach non-stop mode to do in-line step-overs (pause all
 > threads, remove breakpoint, single-step, reinsert breakpoint, restart
 > threads), we'll need to be able to queue in-line step over requests,
 > much like we queue displaced stepping (out-of-line) requests.
 > Actually, the queue should be the same -- threads wait for their turn
 > to step past something (breakpoint, watchpoint), doesn't matter what
 > technique we end up using when the step over actually starts.
 > 
 > I found that the queue management ends up simpler and more efficient
 > if embedded in the thread objects themselves.  This commit converts
 > the existing displaced stepping queue to that.  Later patches will
 > make the in-line step-overs code paths use it too.
 > 
 > gdb/ChangeLog:
 > 2015-04-17  Pedro Alves  <palves@redhat.com>
 > 
 > 	* gdbthread.h (struct thread_info) <step_over_prev,
 > 	step_over_next>: New fields.
 > 	(thread_step_over_chain_enqueue, thread_step_over_chain_remove)
 > 	(thread_step_over_chain_next, thread_is_in_step_over_chain): New
 > 	declarations.
 > 	* infrun.c (struct displaced_step_request): Delete.
 > 	(struct displaced_step_inferior_state) <step_request_queue>:
 > 	Delete field.
 > 	(displaced_step_in_progress): New function.
 > 	(displaced_step_prepare): Assert that trap_expected is set.  Use
 > 	thread_step_over_chain_enqueue.  Split starting a new displaced
 > 	step to ...
 > 	(start_step_over): ... this new function.
 > 	(resume): Assert the thread isn't waiting for a step over already.
 > 	(proceed): Assert the thread isn't waiting for a step over
 > 	already.
 > 	(infrun_thread_stop_requested): Adjust to remove threads from the
 > 	embedded step-over chain.
 > 	(handle_inferior_event) <fork/vfork>: Call start_step_over after
 > 	displaced_step_fixup.
 > 	(handle_signal_stop): Call start_step_over after
 > 	displaced_step_fixup.
 > 	* infrun.h (step_over_queue_head): New declaration.
 > 	* thread.c (step_over_chain_enqueue, step_over_chain_remove)
 > 	(thread_step_over_chain_next, thread_is_in_step_over_chain)
 > 	(thread_step_over_chain_enqueue)
 > 	(thread_step_over_chain_remove): New functions.
 > 	(delete_thread_1): Remove thread from the step-over chain.
 > 
 > v3:
 > 
 > 	More comments.  The step-over chain is now a global instead of
 > 	being per-inferior.  Previous versions had actually broken
 > 	multiple-processes displaced stepping at the same time.  Added new
 > 	thread_is_in_step_over_chain predicate, and new
 > 	thread_step_over_chain_next helper function.  Should make reading
 > 	the code a bit easier (following patches adjusted too).
 >...
 > 
 > diff --git a/gdb/infrun.c b/gdb/infrun.c
 > index 534ecef..f325a53 100644
 > --- a/gdb/infrun.c
 > +++ b/gdb/infrun.c
 >...
 > @@ -1855,24 +1842,44 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 >    do_cleanups (old_cleanups);
 >  
 >    displaced->step_ptid = null_ptid;
 > +}
 >  
 > -  /* Are there any pending displaced stepping requests?  If so, run
 > -     one now.  Leave the state object around, since we're likely to
 > -     need it again soon.  */
 > -  while (displaced->step_request_queue)
 > +/* Are there any pending step-over requests?  If so, run one now.  */

Hi.

Nit: IIUC "run one now" should read "run all we can now".

 > +
 > +static void
 > +start_step_over (void)
 > +{
 >...