This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode


Pedro Alves writes:
...
 > gdb/ChangeLog:
 > 2015-04-17  Pedro Alves  <palves@redhat.com>
 >
 > 	* NEWS: Mention "maint set/show target-non-stop".
 > 	* breakpoint.c (update_global_location_list): Check
 > 	target_is_non_stop_p instead of non_stop.
 > 	* infcmd.c (attach_command_post_wait, attach_command): Likewise.
 > 	* infrun.c (show_can_use_displaced_stepping)
 > 	(can_use_displaced_stepping_p, start_step_over_inferior):
 > 	Likewise.
 > 	(resume): Always resume a single thread if the target is in
 > 	non-stop mode.
 > 	(proceed): Check target_is_non_stop_p instead of non_stop.  If in
 > 	all-mode but the target is always in non-stop mode, start all the
 > 	other threads that are implicitly resumed too.
 > 	(for_each_just_stopped_thread, fetch_inferior_event)
 > 	(adjust_pc_after_break, stop_all_threads): Check
 > 	target_is_non_stop_p instead of non_stop.
 > 	(handle_inferior_event): Likewise.  Handle detach-fork in all-stop
 > 	with the target always in non-stop mode.
 > 	(handle_signal_stop) <random signal>: If we get a signal while
 > 	stepping over a breakpoint, and the target is always in non-stop
 > 	mode, restart all threads.
 > 	(switch_back_to_stepped_thread): Check target_is_non_stop_p
 > 	instead of non_stop.
 > 	(keep_going_stepped_thread): Always resume a single thread if the
 > 	target is in non-stop mode.
 > 	(stop_waiting): If in all-stop mode, and the target is in non-stop
 > 	mode, stop all threads.
 > 	(keep_going_pass): Likewise, when starting a new in-line step-over
 > 	sequence.
 > 	* linux-nat.c (get_pending_status, select_event_lwp)
 > 	(linux_nat_filter_event, linux_nat_wait_1, linux_nat_wait): Check
 > 	target_is_non_stop_p instead of non_stop.
 > 	(linux_nat_always_non_stop_p): New function.
 > 	(linux_nat_stop): Check target_is_non_stop_p instead of non_stop.
 > 	(linux_nat_add_target): Install linux_nat_always_non_stop_p.
 > 	* target-delegates.c: Regenerate.
 > 	* target.c (target_is_non_stop_p): New function.
 > 	(target_non_stop_enabled, target_non_stop_enabled_1): New globals.
 > 	(maint_set_target_non_stop_command)
 > 	(maint_show_target_non_stop_command): New functions.
 > 	(_initilize_target): Install "maint set/show target-non-stop"
 > 	commands.
 > 	* target.h (struct target_ops) <to_always_non_stop_p>: New field.
 > 	(target_non_stop_enabled): New declaration.
 > 	(target_is_non_stop_p): New declaration.
 >
 > gdb/doc/ChangeLog:
 > 2015-04-07  Pedro Alves  <palves@redhat.com>
 >
 > 	* gdb.texinfo (Maintenance Commands): Document "maint set/show
 > 	target-non-stop".
 >
...
 > diff --git a/gdb/infrun.c b/gdb/infrun.c
 > index 8859b9f..f6d3e07 100644
 > --- a/gdb/infrun.c
 > +++ b/gdb/infrun.c
> @@ -1632,7 +1632,7 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 >      fprintf_filtered (file,
 >  		      _("Debugger's willingness to use displaced stepping "
 >  			"to step over breakpoints is %s (currently %s).\n"),
 > -		      value, non_stop ? "on" : "off");
 > +		      value, target_is_non_stop_p () ? "on" : "off");
 >    else
 >      fprintf_filtered (file,
 >  		      _("Debugger's willingness to use displaced stepping "
> @@ -1645,7 +1645,8 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 >  static int
 >  use_displaced_stepping (struct gdbarch *gdbarch)
 >  {
 > -  return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO && non_stop)
 > +  return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
 > +	    && target_is_non_stop_p ())
 >  	   || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
 >  	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
 >  	  && find_record_target () == NULL);
 > @@ -2016,7 +2017,7 @@ start_step_over (void)
 >  	 wouldn't be able to resume anything else until the target
 >  	 stops again.  In non-stop, the resume always resumes only TP,
 >  	 so it's OK to let the thread resume freely.  */
 > -      if (!non_stop && !step_what)
 > +      if (!target_is_non_stop_p () && !step_what)
 >  	continue;
 >
 >        switch_to_thread (tp->ptid);
 > @@ -2035,7 +2036,7 @@ start_step_over (void)
 >  	  return 1;
 >  	}
 >
 > -      if (!non_stop)
 > +      if (!target_is_non_stop_p ())
 >  	{
 >  	  /* On all-stop, shouldn't have resumed unless we needed a
 >  	     step over.  */
 > @@ -2383,7 +2384,10 @@ resume (enum gdb_signal sig)
 >  	      insert_single_step_breakpoint (gdbarch, aspace, pc);
 >  	      insert_breakpoints ();
 >
 > -	      resume_ptid = user_visible_resume_ptid (user_step);
 > +	      if (target_is_non_stop_p ())
 > +		resume_ptid = inferior_ptid;
 > +	      else
 > +		resume_ptid = user_visible_resume_ptid (user_step);

Hi.
For my own education, why is this change needed?

 >  	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
 >  	      discard_cleanups (old_cleanups);
 >  	      tp->resumed = 1;
 > @@ -2498,8 +2502,14 @@ resume (enum gdb_signal sig)
 >    resume_ptid = user_visible_resume_ptid (user_step);
 >
 >    /* Maybe resume a single thread after all.  */
 > -  if ((step || thread_has_single_step_breakpoints_set (tp))
 > -      && tp->control.trap_expected)
 > +  if (target_is_non_stop_p ())
 > +    {
 > +      /* If non-stop mode, threads are always controlled
 > +	 individually.  */
 > +      resume_ptid = inferior_ptid;
 > +    }
 > +  else if ((step || thread_has_single_step_breakpoints_set (tp))
 > +	   && tp->control.trap_expected)
 >      {
 >        /* We're allowing a thread to run past a breakpoint it has
 >  	 hit, by single-stepping the thread with the breakpoint
 > @@ -2935,11 +2945,51 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 >  	 other thread was already doing one.  In either case, don't
 >  	 resume anything else until the step-over is finished.  */
 >      }
 > -  else if (started && !non_stop)
 > +  else if (started && !target_is_non_stop_p ())
 >      {
 >        /* A new displaced stepping sequence was started.  In all-stop,
 >  	 we can't talk to the target anymore until it next stops.  */
 >      }
 > +  else if (!non_stop && target_is_non_stop_p ())

This takes a bit of reading to grok (the difference
b/w non_stop and target_is_non_stop_p).
Can you copy the comment below (marked XYZ) here?

 > +    {
 > +      /* Start all other threads that are implicitly resumed too.  */
 > +      ALL_NON_EXITED_THREADS (tp)
 > +        {
 > +	  /* Ignore threads of processes we're not resuming.  */
 > +	  if (!ptid_match (tp->ptid, resume_ptid))
 > +	    continue;
 > +
 > +	  if (tp->resumed)
 > +	    {
 > +	      if (debug_infrun)
 > +		fprintf_unfiltered (gdb_stdlog,
 > +				    "infrun: proceed: [%s] resumed\n",
 > +				    target_pid_to_str (tp->ptid));
 > +	      gdb_assert (tp->executing || tp->suspend.waitstatus_pending_p);
 > +	      continue;
 > +	    }
 > +
 > +	  if (thread_is_in_step_over_chain (tp))
 > +	    {
 > +	      if (debug_infrun)
 > +		fprintf_unfiltered (gdb_stdlog,
 > +				    "infrun: proceed: [%s] needs step-over\n",
 > +				    target_pid_to_str (tp->ptid));
 > +	      continue;
 > +	    }
 > +
 > +	  if (debug_infrun)
 > +	    fprintf_unfiltered (gdb_stdlog,
 > +				"infrun: proceed: resuming %s\n",
 > +				target_pid_to_str (tp->ptid));
 > +
 > +	  reset_ecs (ecs, tp);
 > +	  switch_to_thread (tp->ptid);
 > +	  keep_going_pass (ecs);
 > +	  if (!ecs->wait_some_more)
 > +	    error ("Command aborted.");
 > +	}
 > +    }
 >    else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
 >      {
 >        /* The thread wasn't started, and isn't queued, run it now.  */
...
> @@ -5037,7 +5089,7 @@ finish_step_over (struct execution_control_state *ecs)
 >  	clear_step_over_info ();
 >      }
 >
 > -  if (!non_stop)
 > +  if (!target_is_non_stop_p ())
 >      return 0;
 >
 >    /* Start a new step-over in another thread if there's one that
> @@ -5614,6 +5666,17 @@ handle_signal_stop (struct execution_control_state *ecs)
 >  	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 >  	  ecs->event_thread->control.trap_expected = 0;
 >
 > +	  if (!non_stop && target_is_non_stop_p ())

Ditto. Copy XYZ here?

 > +	    {
 > +	      keep_going (ecs);
 > +
 > +	      /* We've canceled the step-over temporarily while the
 > +		 signal handler executes.  Let other threads run,
 > +		 according to schedlock.  */
 > +	      restart_threads (ecs->event_thread);
 > +	      return;
 > +	    }
 > +
 >  	  /* If we were nexting/stepping some other thread, switch to
 >  	     it, so that we don't continue it, losing control.  */
 >  	  if (!switch_back_to_stepped_thread (ecs))
...
 > @@ -7153,6 +7220,11 @@ stop_waiting (struct execution_control_state *ecs)
 >
> /* Let callers know we don't want to wait for the inferior anymore. */
 >    ecs->wait_some_more = 0;
 > +
 > +  /* If all-stop, but the target is always in non-stop mode, stop all
 > +     threads now that we're presenting the stop to the user.  */

XYZ^^^
"If all-stop, but the target is always in non-stop mode, ..."

 > +  if (!non_stop && target_is_non_stop_p ())
 > +    stop_all_threads ();
 >  }
 >
 >  /* Like keep_going, but passes the signal to the inferior, even if the
> @@ -7267,7 +7339,7 @@ keep_going_pass (struct execution_control_state *ecs)
 >  	 insert_breakpoints below, because that removes the breakpoint
 >  	 we're about to step over, otherwise other threads could miss
 >  	 it.  */
 > -      if (step_over_info_valid_p () && non_stop)
 > +      if (step_over_info_valid_p () && target_is_non_stop_p ())
 >  	stop_all_threads ();
 >
 >        /* Stop stepping if inserting breakpoints fails.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]