public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
@ 2015-05-18 21:27 Doug Evans
  2015-05-19 19:19 ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Doug Evans @ 2015-05-18 21:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

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.  */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-05-18 21:27 [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode Doug Evans
@ 2015-05-19 19:19 ` Pedro Alves
  0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2015-05-19 19:19 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On 05/18/2015 10:27 PM, Doug Evans wrote:

>   >  	  /* 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?

Please see the comment tweaks below.

> 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?

Done (in all places).

Let me know what you think.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 82fa526..2f8a708 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2384,6 +2384,11 @@ resume (enum gdb_signal sig)
 	      insert_single_step_breakpoint (gdbarch, aspace, pc);
 	      insert_breakpoints ();
 
+	      /* In non-stop, we always control threads individually.
+		 Note that the target may always work in non-stop mode
+		 even with "set non-stop off", in which case
+		 user_visible_resume_ptid could return a wildcard
+		 ptid.  */
 	      if (target_is_non_stop_p ())
 		resume_ptid = inferior_ptid;
 	      else
@@ -2946,7 +2951,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     }
   else if (!non_stop && target_is_non_stop_p ())
     {
-      /* Start all other threads that are implicitly resumed too.  */
+      /* In all-stop, but the target is always in non-stop mode.
+	 Start all other threads that are implicitly resumed too.  */
       ALL_NON_EXITED_THREADS (tp)
         {
 	  /* Ignore threads of processes we're not resuming.  */
@@ -5670,13 +5676,15 @@ handle_signal_stop (struct execution_control_state *ecs)
 
 	  if (target_is_non_stop_p ())
 	    {
+	      /* Either "set non-stop" is "on", or the target is
+		 always in non-stop mode.  In this case, we have a bit
+		 more work to do.  Resume the current thread, and if
+		 we had paused all threads, restart them while the
+		 signal handler runs.  */
 	      keep_going (ecs);
 
-	      /* The step-over has been canceled temporarily while the
-		 signal handler executes.  */
 	      if (was_in_line)
 		{
-		  /* We had paused all threads, restart them.  */
 		  restart_threads (ecs->event_thread);
 		}
 	      else if (debug_infrun)
-- 
1.9.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-05-19 18:08         ` Pedro Alves
@ 2015-05-21  9:17           ` Yao Qi
  0 siblings, 0 replies; 8+ messages in thread
From: Yao Qi @ 2015-05-21  9:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Pedro Alves <palves@redhat.com> writes:

> So the only difference between stepping over a breakpoint in-line or
> out-of-line here, is that with the former, threads had been previously
> paused, so we re-resume them while the signal handler runs (to avoid
> debugger-induced inferior deadlock).

Looks in-line stepping and out-of-line stepping behaves similarly under
this context.

>
> Let me know if that answered your question.

Yes, that is what I want to know, thanks.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-04-24  7:39       ` Yao Qi
@ 2015-05-19 18:08         ` Pedro Alves
  2015-05-21  9:17           ` Yao Qi
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2015-05-19 18:08 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

I had not realized I missed answering this.

On 04/24/2015 08:39 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>> This path is about the case that a signal is got while in in-line
>>> stepping, isn't?  If so, non_stop should be an invariant false.  We
>>> don't need to check it.
>>
>> Hmm, not sure what you mean:
>>
> 
> Let me ask it in another way, when we get here, it means a signal
> arrived, the code some lines above is:
> 
>           if (debug_infrun)
>             fprintf_unfiltered (gdb_stdlog,
>                                 "infrun: signal arrived while stepping over "
>                                 "breakpoint\n");
> 
> GDB just did the breakpoint step over, via in-line stepping or
> out-of-line stepping, right? as your patch below shows.
> 
>>  - We need to do this with displaced stepping too, because we can't
>>    deliver signals while doing a displaced step.  See comments at the
>>    top of displaced_step_prepare and in do_target_resume.
> 
> The first sentence is contradictory, or you mean we *can* do either
> out-of-line stepping or in-line stepping, but we can't deliver a signal
> while doing a displaced stepping...

By "need to do this" I meant that we need to cancel the step over in
progress, and insert the step-resume at the signal handlers return.

> 
>>
>>  - We can certainly get a signal while doing an in-line step-over.
>>    The simplest would be, trying to step-over a breakpoint here:
>>
>>       *(volatile int *)0 = 1;
>>
>>    which usually results SIGSEGV.
> 
> ... while we can deliver a signal in in-line stepping.  Is it correct?
> 

Not really.

We can't deliver a signal while a displaced step is in
progress, because we wouldn't be able to tell whether the thread stepped
to the handler or to main line code, so we wouldn't know whether to apply
the displaced step fixup.  Also, if the thread stops for some reason while
in the handler, we'd end up with the scratch pad still in the frame chain,
so later when the thread is re-resumed, it'd return to the scratch pad, but
the scratch pad would have already have been overwritten.

And we can't deliver a signal while stepping over a breakpoint in-line
either, because that requires removing the breakpoint, and the signal handler
can recurse and call the same code that had the breakpoint we were
stepping over -- which would mean we'd miss trapping on that breakpoint.
(this is tested by signest.exp).

So the only difference between stepping over a breakpoint in-line or
out-of-line here, is that with the former, threads had been previously
paused, so we re-resume them while the signal handler runs (to avoid
debugger-induced inferior deadlock).

Let me know if that answered your question.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-04-22 20:16     ` Pedro Alves
@ 2015-04-24  7:39       ` Yao Qi
  2015-05-19 18:08         ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Yao Qi @ 2015-04-24  7:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Pedro Alves <palves@redhat.com> writes:

>> This path is about the case that a signal is got while in in-line
>> stepping, isn't?  If so, non_stop should be an invariant false.  We
>> don't need to check it.
>
> Hmm, not sure what you mean:
>

Let me ask it in another way, when we get here, it means a signal
arrived, the code some lines above is:

          if (debug_infrun)
            fprintf_unfiltered (gdb_stdlog,
                                "infrun: signal arrived while stepping over "
                                "breakpoint\n");

GDB just did the breakpoint step over, via in-line stepping or
out-of-line stepping, right? as your patch below shows.

>  - We need to do this with displaced stepping too, because we can't
>    deliver signals while doing a displaced step.  See comments at the
>    top of displaced_step_prepare and in do_target_resume.

The first sentence is contradictory, or you mean we *can* do either
out-of-line stepping or in-line stepping, but we can't deliver a signal
while doing a displaced stepping...

>
>  - We can certainly get a signal while doing an in-line step-over.
>    The simplest would be, trying to step-over a breakpoint here:
>
>       *(volatile int *)0 = 1;
>
>    which usually results SIGSEGV.

... while we can deliver a signal in in-line stepping.  Is it correct?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-04-21 11:09   ` Yao Qi
@ 2015-04-22 20:16     ` Pedro Alves
  2015-04-24  7:39       ` Yao Qi
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2015-04-22 20:16 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 04/21/2015 12:08 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> 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
>         ^^^^^^^^
> 
> all-stop mode.

Thanks, fixed.

> 
>> 	(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.
> 
>> @@ -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 ())
>> +	    {
> 
> This path is about the case that a signal is got while in in-line
> stepping, isn't?  If so, non_stop should be an invariant false.  We
> don't need to check it.

Hmm, not sure what you mean:

 - We need to do this with displaced stepping too, because we can't
   deliver signals while doing a displaced step.  See comments at the
   top of displaced_step_prepare and in do_target_resume.

 - We can certainly get a signal while doing an in-line step-over.
   The simplest would be, trying to step-over a breakpoint here:

      *(volatile int *)0 = 1;

   which usually results SIGSEGV.

 - We can step past breakpoints in-line in non-stop mode too.

However, I agree there's something wrong here.  If we get here
with "set non-stop on", and we were doing an in-line step-over,
then we also need to restart threads, not just
in all-stop + "target always-non-stop".  I've applied this change
on top now, and it tested OK on x86-64, native with
both displaced on/off, and gdbserver.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e236510..3e60313 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5732,6 +5732,8 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  && ecs->event_thread->control.trap_expected
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
 	{
+	  int was_in_line;
+
 	  /* We were just starting a new sequence, attempting to
 	     single-step off of a breakpoint and expecting a SIGTRAP.
 	     Instead this signal arrives.  This signal will take us out
@@ -5747,20 +5749,29 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "infrun: signal arrived while stepping over "
                                 "breakpoint\n");

+	  was_in_line = step_over_info_valid_p ();
 	  clear_step_over_info ();
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
 	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;

-	  if (!non_stop && target_is_non_stop_p ())
+	  if (target_is_non_stop_p ())
 	    {
 	      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);
+	      /* The step-over has been canceled temporarily while the
+		 signal handler executes.  */
+	      if (was_in_line)
+		{
+		  /* We had paused all threads, restart them.  */
+		  restart_threads (ecs->event_thread);
+		}
+	      else if (debug_infrun)
+		{
+		  fprintf_unfiltered (gdb_stdlog,
+				      "infrun: no need to restart threads\n");
+		}
 	      return;
 	    }

-- 
1.9.3


I've now folded that into the patch that teaches non-stop about
in-line step overs.


>> +	      keep_going (ecs);
>> +
>> +	      /* We've canceled the step-over temporarily while the
>> +		 signal handler executes.  Let other threads run,
>> +		 according to schedlock.  */
> 
> IMO, we don't need to run other threads according to schedlock.  GDB
> resumes some threads here and operations should be invisible to user,
> schedlock, as a user visible option, shouldn't affect what threads
> should be resumed.  On the other hand, restart_threads is to undo the
> changes done by stop_all_threads, so no user preference (schedlock)
> should be considered here.

Yes, you're right, thanks for noticing this.  The code in restart_threads does
not in fact look at schedlock (through e.g., user_visible_resume_ptid) at
all, it's the comment that is stale from a previous attempt, from
before https://sourceware.org/ml/gdb-patches/2015-03/msg00293.html.
It was issues like that led to that schedlock patch, even.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-04-17 11:38 ` [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
@ 2015-04-21 11:09   ` Yao Qi
  2015-04-22 20:16     ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Yao Qi @ 2015-04-21 11:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> 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
        ^^^^^^^^

all-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.

> @@ -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 ())
> +	    {

This path is about the case that a signal is got while in in-line
stepping, isn't?  If so, non_stop should be an invariant false.  We
don't need to check it.

> +	      keep_going (ecs);
> +
> +	      /* We've canceled the step-over temporarily while the
> +		 signal handler executes.  Let other threads run,
> +		 according to schedlock.  */

IMO, we don't need to run other threads according to schedlock.  GDB
resumes some threads here and operations should be invisible to user,
schedlock, as a user visible option, shouldn't affect what threads
should be resumed.  On the other hand, restart_threads is to undo the
changes done by stop_all_threads, so no user preference (schedlock)
should be considered here.

> +	      restart_threads (ecs->event_thread);
> +	      return;
> +	    }
> +

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode
  2015-04-17 10:47 [PATCH v3 00/23] All-stop on top of non-stop Pedro Alves
@ 2015-04-17 11:38 ` Pedro Alves
  2015-04-21 11:09   ` Yao Qi
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2015-04-17 11:38 UTC (permalink / raw)
  To: gdb-patches

This finally implements user-visible all-stop mode running with the
target_ops backend always in non-stop mode.  This is a stepping stone
towards finer-grained control of threads, being able to do interesting
things like thread groups, associating groups with breakpoints, etc.
From the user's perspective, all-stop mode is really just a special
case of being able to stop and resume specific sets of threads, so it
makes sense to do this step first.

With this, even in all-stop, the target is no longer in charge of
stopping all threads before reporting an event to the core -- the core
takes care of it when it sees fit.  For example, when "next"- or
"step"-ing, we can avoid stopping and resuming all threads at each
internal single-step, and instead only stop all threads when we're
about to present the stop to the user.

The implementation is almost straight forward, as the heavy lifting
has been done already in previous patches.  Basically, we replace
checks for "set non-stop on/off" (the non_stop global), with calls to
a new target_is_non_stop_p function.  In a few places, if "set
non-stop off", we stop all threads explicitly, and in a few other
places we resume all threads explicitly, making use of existing
methods that were added for teaching non-stop to step over breakpoints
without displaced stepping.

This adds a new "maint set target-non-stop on/off/auto" knob that
allows both disabling the feature if we find problems, and
force-enable it for development (useful when teaching a target about
this.  The default is "auto", which means the feature is enabled if a
new target method says it should be enabled.  The patch implements the
method in linux-nat.c, just for illustration, because it still returns
false.  We'll need a few follow up fixes before turning it on by
default.  This is a separate target method from indicating regular
non-stop support, because e.g., while e.g., native linux-nat.c is
close to regression free with all-stop-non-stop (with following
patches will fixing the remaining regressions), remote.c+gdbserver
will still need more fixing, even though it supports "set non-stop
on".

Tested on x86_64 Fedora 20, native, with and without "set displaced
off", and with and without "maint set target-non-stop on"; and also
against gdbserver.

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".

v3:

  Adjustments as per previous patches:

  - Change 'resumed' instead of 'executing' in 'proceed' to avoid
    resuming threads with pending statuses.

  - Adjust to use thread_is_in_step_over_chain.

v2:

 - Documentation adjusted per Eli's review.  NEWS entry added.
 - `proceed' change simplified, thanks to fixes in v2 of previous
   patches in the series.
---
 gdb/NEWS               |   6 +++
 gdb/breakpoint.c       |   2 +-
 gdb/doc/gdb.texinfo    |  24 +++++++++++
 gdb/infcmd.c           |   4 +-
 gdb/infrun.c           | 110 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/linux-nat.c        |  25 +++++++----
 gdb/target-delegates.c |  31 ++++++++++++++
 gdb/target.c           |  62 ++++++++++++++++++++++++++++
 gdb/target.h           |  13 ++++++
 9 files changed, 247 insertions(+), 30 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 62cbdcb..7b412e6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,12 @@ maint print symbol-cache-statistics
 maint flush-symbol-cache
   Flush the contents of the symbol cache.
 
+maint set target-non-stop (on|off|auto)
+maint show target-non-stop
+  Control whether GDB targets always operate in non-stop mode even if
+  "set non-stop" is "off".  The default is "auto", meaning non-stop
+  mode is enabled if supported by the target.
+
 record btrace bts
 record bts
   Start branch trace recording using Branch Trace Store (BTS) format.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e23d223..96d2a14 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12345,7 +12345,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
 
       if (!found_object)
 	{
-	  if (removed && non_stop
+	  if (removed && target_is_non_stop_p ()
 	      && need_moribund_for_location_type (old_loc))
 	    {
 	      /* This location was removed from the target.  In
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0410702..97f7f78 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34192,6 +34192,30 @@ asynchronous mode (@pxref{Background Execution}).  Normally the
 default is asynchronous, if it is available; but this can be changed
 to more easily debug problems occurring only in synchronous mode.
 
+@kindex maint set target-non-stop @var{mode} [on|off|auto]
+@kindex maint show target-non-stop
+@item maint set target-non-stop
+@itemx maint show target-non-stop
+
+This controls whether @value{GDBN} targets always operate in non-stop
+mode even if @code{set non-stop} is @code{off} (@pxref{Non-Stop
+Mode}).  The default is @code{auto}, meaning non-stop mode is enabled
+if supported by the target.
+
+@table @code
+@item maint set target-non-stop auto
+This is the default mode.  @value{GDBN} controls the target in
+non-stop mode if the target supports it.
+
+@item maint set target-non-stop on
+@value{GDBN} controls the target in non-stop mode even if the target
+does not indicate support.
+
+@item maint set target-non-stop off
+@value{GDBN} does not control the target in non-stop mode even if the
+target supports it.
+@end table
+
 @kindex maint set per-command
 @kindex maint show per-command
 @item maint set per-command
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 7e2484b..3505b1b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2517,7 +2517,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
 	 selected thread is stopped, others may still be executing.
 	 Be sure to explicitly stop all threads of the process.  This
 	 should have no effect on already stopped threads.  */
-      if (non_stop)
+      if (target_is_non_stop_p ())
 	target_stop (pid_to_ptid (inferior->pid));
 
       /* Tell the user/frontend where we're stopped.  */
@@ -2622,7 +2622,7 @@ attach_command (char *args, int from_tty)
   init_wait_for_inferior ();
   clear_proceed_status (0);
 
-  if (non_stop)
+  if (target_is_non_stop_p ())
     {
       /* If we find that the current thread isn't stopped, explicitly
 	 do so now, because we're going to install breakpoints and
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);
 	      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 ())
+    {
+      /* 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.  */
@@ -3151,7 +3201,7 @@ for_each_just_stopped_thread (for_each_just_stopped_thread_callback_func func)
   if (!target_has_execution || ptid_equal (inferior_ptid, null_ptid))
     return;
 
-  if (non_stop)
+  if (target_is_non_stop_p ())
     {
       /* If in non-stop mode, only the current thread stopped.  */
       func (inferior_thread ());
@@ -3632,7 +3682,7 @@ fetch_inferior_event (void *client_data)
   /* If an error happens while handling the event, propagate GDB's
      knowledge of the executing state to the frontend/user running
      state.  */
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     ts_old_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
   else
     ts_old_chain = make_cleanup (finish_thread_state_cleanup, &ecs->ptid);
@@ -3871,7 +3921,8 @@ adjust_pc_after_break (struct thread_info *thread,
      to get the "stopped by SW BP and needs adjustment" info out of
      the target/kernel (and thus never reach here; see above).  */
   if (software_breakpoint_inserted_here_p (aspace, breakpoint_pc)
-      || (non_stop && moribund_breakpoint_here_p (aspace, breakpoint_pc)))
+      || (target_is_non_stop_p ()
+	  && moribund_breakpoint_here_p (aspace, breakpoint_pc)))
     {
       struct cleanup *old_cleanups = make_cleanup (null_cleanup, NULL);
 
@@ -4152,7 +4203,7 @@ stop_all_threads (void)
   ptid_t entry_ptid;
   struct cleanup *old_chain;
 
-  gdb_assert (non_stop);
+  gdb_assert (target_is_non_stop_p ());
 
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads\n");
@@ -4466,7 +4517,7 @@ handle_inferior_event (struct execution_control_state *ecs)
   {
     ptid_t mark_ptid;
 
-    if (!non_stop)
+    if (!target_is_non_stop_p ())
       mark_ptid = minus_one_ptid;
     else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
 	     && ecs->ws.kind == TARGET_WAITKIND_EXITED)
@@ -4769,7 +4820,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  child = ecs->ws.value.related_pid;
 
 	  /* In non-stop mode, also resume the other branch.  */
-	  if (non_stop && !detach_fork)
+	  if (!detach_fork && (non_stop
+			       || (sched_multi && target_is_non_stop_p ())))
 	    {
 	      if (follow_child)
 		switch_to_thread (parent);
@@ -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 ())
+	    {
+	      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))
@@ -6498,7 +6561,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 static int
 switch_back_to_stepped_thread (struct execution_control_state *ecs)
 {
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       struct thread_info *tp;
       struct thread_info *stepping_thread;
@@ -6589,7 +6652,8 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
       ALL_NON_EXITED_THREADS (tp)
         {
-	  /* Ignore threads of processes we're not resuming.  */
+	  /* Ignore threads of processes the caller is not
+	     resuming.  */
 	  if (!sched_multi
 	      && ptid_get_pid (tp->ptid) != ptid_get_pid (ecs->ptid))
 	    continue;
@@ -6735,7 +6799,10 @@ keep_going_stepped_thread (struct thread_info *tp)
 				     stop_pc);
 
       tp->resumed = 1;
-      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+      if (target_is_non_stop_p ())
+	resume_ptid = inferior_ptid;
+      else
+	resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
       do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
     }
   else
@@ -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.  */
+  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.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b04aa68..b449485 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1375,13 +1375,13 @@ get_pending_status (struct lwp_info *lp, int *status)
     signo = GDB_SIGNAL_0; /* a pending ptrace event, not a real signal.  */
   else if (lp->status)
     signo = gdb_signal_from_host (WSTOPSIG (lp->status));
-  else if (non_stop && !is_executing (lp->ptid))
+  else if (target_is_non_stop_p () && !is_executing (lp->ptid))
     {
       struct thread_info *tp = find_thread_ptid (lp->ptid);
 
       signo = tp->suspend.stop_signal;
     }
-  else if (!non_stop)
+  else if (!target_is_non_stop_p ())
     {
       struct target_waitstatus last;
       ptid_t last_ptid;
@@ -2931,7 +2931,7 @@ select_event_lwp (ptid_t filter, struct lwp_info **orig_lp, int *status)
      having stepped the thread, wouldn't understand what the trap was
      for, and therefore would report it to the user as a random
      signal.  */
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       event_lp = iterate_over_lwps (filter,
 				    select_singlestep_lwp_callback, NULL);
@@ -3278,7 +3278,7 @@ linux_nat_filter_event (int lwpid, int status)
     {
       enum gdb_signal signo = gdb_signal_from_host (WSTOPSIG (status));
 
-      if (!non_stop)
+      if (!target_is_non_stop_p ())
 	{
 	  /* Only do the below in all-stop, as we currently use SIGSTOP
 	     to implement target_stop (see linux_nat_stop) in
@@ -3544,7 +3544,7 @@ linux_nat_wait_1 (struct target_ops *ops,
   status = lp->status;
   lp->status = 0;
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       /* Now stop all other LWP's ...  */
       iterate_over_lwps (minus_one_ptid, stop_callback, NULL);
@@ -3586,7 +3586,7 @@ linux_nat_wait_1 (struct target_ops *ops,
      clears it.  */
   last_resume_kind = lp->last_resume_kind;
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       /* In all-stop, from the core's perspective, all LWPs are now
 	 stopped until a new resume action is sent over.  */
@@ -3719,7 +3719,7 @@ linux_nat_wait (struct target_ops *ops,
      specific_process, for example, see linux_nat_wait_1), and
      meanwhile the event became uninteresting.  Don't bother resuming
      LWPs we're not going to wait for if they'd stop immediately.  */
-  if (non_stop)
+  if (target_is_non_stop_p ())
     iterate_over_lwps (minus_one_ptid, resume_stopped_resumed_lwps, &ptid);
 
   event_ptid = linux_nat_wait_1 (ops, ptid, ourstatus, target_options);
@@ -4560,6 +4560,14 @@ linux_nat_supports_non_stop (struct target_ops *self)
   return 1;
 }
 
+/* to_always_non_stop_p implementation.  */
+
+static int
+linux_nat_always_non_stop_p (struct target_ops *self)
+{
+  return 0;
+}
+
 /* True if we want to support multi-process.  To be removed when GDB
    supports multi-exec.  */
 
@@ -4779,7 +4787,7 @@ linux_nat_stop_lwp (struct lwp_info *lwp, void *data)
 static void
 linux_nat_stop (struct target_ops *self, ptid_t ptid)
 {
-  if (non_stop)
+  if (target_is_non_stop_p ())
     iterate_over_lwps (ptid, linux_nat_stop_lwp, NULL);
   else
     linux_ops->to_stop (linux_ops, ptid);
@@ -4878,6 +4886,7 @@ linux_nat_add_target (struct target_ops *t)
   t->to_can_async_p = linux_nat_can_async_p;
   t->to_is_async_p = linux_nat_is_async_p;
   t->to_supports_non_stop = linux_nat_supports_non_stop;
+  t->to_always_non_stop_p = linux_nat_always_non_stop_p;
   t->to_async = linux_nat_async;
   t->to_terminal_inferior = linux_nat_terminal_inferior;
   t->to_terminal_ours = linux_nat_terminal_ours;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 36eacbf..8a92acf 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1744,6 +1744,33 @@ debug_supports_non_stop (struct target_ops *self)
 }
 
 static int
+delegate_always_non_stop_p (struct target_ops *self)
+{
+  self = self->beneath;
+  return self->to_always_non_stop_p (self);
+}
+
+static int
+tdefault_always_non_stop_p (struct target_ops *self)
+{
+  return 0;
+}
+
+static int
+debug_always_non_stop_p (struct target_ops *self)
+{
+  int result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_always_non_stop_p (...)\n", debug_target.to_shortname);
+  result = debug_target.to_always_non_stop_p (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_always_non_stop_p (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_int (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
+static int
 delegate_find_memory_regions (struct target_ops *self, find_memory_region_ftype arg1, void *arg2)
 {
   self = self->beneath;
@@ -4005,6 +4032,8 @@ install_delegators (struct target_ops *ops)
     ops->to_async = delegate_async;
   if (ops->to_supports_non_stop == NULL)
     ops->to_supports_non_stop = delegate_supports_non_stop;
+  if (ops->to_always_non_stop_p == NULL)
+    ops->to_always_non_stop_p = delegate_always_non_stop_p;
   if (ops->to_find_memory_regions == NULL)
     ops->to_find_memory_regions = delegate_find_memory_regions;
   if (ops->to_make_corefile_notes == NULL)
@@ -4232,6 +4261,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_is_async_p = tdefault_is_async_p;
   ops->to_async = tdefault_async;
   ops->to_supports_non_stop = tdefault_supports_non_stop;
+  ops->to_always_non_stop_p = tdefault_always_non_stop_p;
   ops->to_find_memory_regions = dummy_find_memory_regions;
   ops->to_make_corefile_notes = dummy_make_corefile_notes;
   ops->to_get_bookmark = tdefault_get_bookmark;
@@ -4380,6 +4410,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_is_async_p = debug_is_async_p;
   ops->to_async = debug_async;
   ops->to_supports_non_stop = debug_supports_non_stop;
+  ops->to_always_non_stop_p = debug_always_non_stop_p;
   ops->to_find_memory_regions = debug_find_memory_regions;
   ops->to_make_corefile_notes = debug_make_corefile_notes;
   ops->to_get_bookmark = debug_get_bookmark;
diff --git a/gdb/target.c b/gdb/target.c
index e992a35..ab6c977 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3752,6 +3752,58 @@ maint_show_target_async_command (struct ui_file *file, int from_tty,
 		      "asynchronous mode is %s.\n"), value);
 }
 
+/* See target.h.  */
+
+int
+target_is_non_stop_p (void)
+{
+  return (non_stop
+	  || target_non_stop_enabled == AUTO_BOOLEAN_TRUE
+	  || (target_non_stop_enabled == AUTO_BOOLEAN_AUTO
+	      && current_target.to_always_non_stop_p (&current_target)));
+}
+
+/* Controls if targets can report that they always run in non-stop
+   mode.  This is just for maintainers to use when debugging gdb.  */
+enum auto_boolean target_non_stop_enabled = AUTO_BOOLEAN_AUTO;
+
+/* The set command writes to this variable.  If the inferior is
+   executing, target_non_stop_enabled is *not* updated.  */
+static enum auto_boolean target_non_stop_enabled_1 = AUTO_BOOLEAN_AUTO;
+
+/* Implementation of "maint set target-non-stop".  */
+
+static void
+maint_set_target_non_stop_command (char *args, int from_tty,
+				   struct cmd_list_element *c)
+{
+  if (have_live_inferiors ())
+    {
+      target_non_stop_enabled_1 = target_non_stop_enabled;
+      error (_("Cannot change this setting while the inferior is running."));
+    }
+
+  target_non_stop_enabled = target_non_stop_enabled_1;
+}
+
+/* Implementation of "maint show target-non-stop".  */
+
+static void
+maint_show_target_non_stop_command (struct ui_file *file, int from_tty,
+				    struct cmd_list_element *c,
+				    const char *value)
+{
+  if (target_non_stop_enabled == AUTO_BOOLEAN_AUTO)
+    fprintf_filtered (file,
+		      _("Whether the target is always in non-stop mode "
+			"is %s (currently %s).\n"), value,
+		      target_is_non_stop_p () ? "on" : "off");
+  else
+    fprintf_filtered (file,
+		      _("Whether the target is always in non-stop mode "
+			"is %s.\n"), value);
+}
+
 /* Temporary copies of permission settings.  */
 
 static int may_write_registers_1 = 1;
@@ -3854,6 +3906,16 @@ Tells gdb whether to control the inferior in asynchronous mode."),
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
 
+  add_setshow_auto_boolean_cmd ("target-non-stop", no_class,
+				&target_non_stop_enabled_1, _("\
+Set whether gdb always controls the inferior in non-stop mode."), _("\
+Show whether gdb always controls the inferior in non-stop mode."), _("\
+Tells gdb whether to control the inferior in non-stop mode."),
+			   maint_set_target_non_stop_command,
+			   maint_show_target_non_stop_command,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
+
   add_setshow_boolean_cmd ("may-write-registers", class_support,
 			   &may_write_registers_1, _("\
 Set permission to write into registers."), _("\
diff --git a/gdb/target.h b/gdb/target.h
index 3f1a8b0..17dee43 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -640,6 +640,10 @@ struct target_ops
        comment on 'to_can_run'.  */
     int (*to_supports_non_stop) (struct target_ops *)
       TARGET_DEFAULT_RETURN (0);
+    /* Return true if the target operates in non-stop mode even when
+       "set non-stop off".  */
+    int (*to_always_non_stop_p) (struct target_ops *)
+      TARGET_DEFAULT_RETURN (0);
     /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (struct target_ops *,
 				   find_memory_region_ftype func, void *data)
@@ -1706,6 +1710,15 @@ extern int target_async_permitted;
 /* Enables/disabled async target events.  */
 extern void target_async (int enable);
 
+/* Whether support for controlling the target backends always in
+   non-stop mode is enabled.  */
+extern enum auto_boolean target_non_stop_enabled;
+
+/* Is the target in non-stop mode?  Some targets control the inferior
+   in non-stop mode even with "set non-stop off".  Always true if "set
+   non-stop" is on.  */
+extern int target_is_non_stop_p (void);
+
 #define target_execution_direction() \
   (current_target.to_execution_direction (&current_target))
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-05-21  9:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18 21:27 [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode Doug Evans
2015-05-19 19:19 ` Pedro Alves
  -- strict thread matches above, loose matches on Subject: below --
2015-04-17 10:47 [PATCH v3 00/23] All-stop on top of non-stop Pedro Alves
2015-04-17 11:38 ` [PATCH v3 10/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
2015-04-21 11:09   ` Yao Qi
2015-04-22 20:16     ` Pedro Alves
2015-04-24  7:39       ` Yao Qi
2015-05-19 18:08         ` Pedro Alves
2015-05-21  9:17           ` Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).