diff mbox

[11/18] Implement all-stop on top of a target running non-stop mode

Message ID 55609CB8.3050104@redhat.com
State New
Headers show

Commit Message

Pedro Alves May 23, 2015, 3:28 p.m. UTC
On 05/22/2015 07:35 PM, Doug Evans wrote:

> I found another instance in keep_going_stepped_thread:
> 
>        if (target_is_non_stop_p ())
>          resume_ptid = inferior_ptid;
>        else
>          resume_ptid = user_visible_resume_ptid  
> (tp->control.stepping_command);
> 
> Can you add a comment here too?
> Thanks.
> [An alternative would be to add a utility function that
> contained this code and then put the comment there,
> but I'll leave it to you to decide if you want
> such a function.]

That's a good idea.  How about this?  I'll fold it in
if it looks good.

---
 gdb/infrun.c | 53 ++++++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

Comments

Doug Evans May 28, 2015, 6:56 p.m. UTC | #1
On Sat, May 23, 2015 at 8:28 AM, Pedro Alves <palves@redhat.com> wrote:
> On 05/22/2015 07:35 PM, Doug Evans wrote:
>
>> I found another instance in keep_going_stepped_thread:
>>
>>        if (target_is_non_stop_p ())
>>          resume_ptid = inferior_ptid;
>>        else
>>          resume_ptid = user_visible_resume_ptid
>> (tp->control.stepping_command);
>>
>> Can you add a comment here too?
>> Thanks.
>> [An alternative would be to add a utility function that
>> contained this code and then put the comment there,
>> but I'll leave it to you to decide if you want
>> such a function.]
>
> That's a good idea.  How about this?  I'll fold it in
> if it looks good.
>
> ---
>  gdb/infrun.c | 53 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d9fc4e5..fe48c91 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2173,6 +2173,25 @@ user_visible_resume_ptid (int step)
>    return resume_ptid;
>  }
>
> +/* Return a ptid representing the set of threads that we will resume,
> +   in the perspective of the target, assuming run control handling
> +   does not require leaving some threads stopped (e.g., stepping past
> +   breakpoint).  USER_STEP indicates whether we're about to start the
> +   target for a stepping command.  */
> +
> +static ptid_t
> +internal_resume_ptid (int user_step)
> +{
> +  /* 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 ())
> +    return inferior_ptid;
> +  else
> +    return user_visible_resume_ptid (user_step);
> +}
> +
>  /* Wrapper for target_resume, that handles infrun-specific
>     bookkeeping.  */
>
> @@ -2384,15 +2403,7 @@ 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
> -               resume_ptid = user_visible_resume_ptid (user_step);
> +             resume_ptid = internal_resume_ptid (user_step);
>               do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
>               discard_cleanups (old_cleanups);
>               tp->resumed = 1;
> @@ -2501,20 +2512,9 @@ resume (enum gdb_signal sig)
>       use singlestep breakpoint.  */
>    gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
>
> -  /* Decide the set of threads to ask the target to resume.  Start
> -     by assuming everything will be resumed, than narrow the set
> -     by applying increasingly restricting conditions.  */
> -  resume_ptid = user_visible_resume_ptid (user_step);
> -
> -  /* Maybe resume a single thread after all.  */
> -  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)
> +  /* Decide the set of threads to ask the target to resume.  */
> +  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
> @@ -2523,6 +2523,8 @@ resume (enum gdb_signal sig)
>          breakpoint if allowed to run.  */
>        resume_ptid = inferior_ptid;
>      }
> +  else
> +    resume_ptid = internal_resume_ptid (user_step);
>
>    if (execution_direction != EXEC_REVERSE
>        && step && breakpoint_inserted_here_p (aspace, pc))
> @@ -6833,10 +6835,7 @@ keep_going_stepped_thread (struct thread_info *tp)
>                                      stop_pc);
>
>        tp->resumed = 1;
> -      if (target_is_non_stop_p ())
> -       resume_ptid = inferior_ptid;
> -      else
> -       resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
> +      resume_ptid = internal_resume_ptid (tp->control.stepping_command);
>        do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
>      }
>    else

"works for me"

Thanks.
diff mbox

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index d9fc4e5..fe48c91 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2173,6 +2173,25 @@  user_visible_resume_ptid (int step)
   return resume_ptid;
 }
 
+/* Return a ptid representing the set of threads that we will resume,
+   in the perspective of the target, assuming run control handling
+   does not require leaving some threads stopped (e.g., stepping past
+   breakpoint).  USER_STEP indicates whether we're about to start the
+   target for a stepping command.  */
+
+static ptid_t
+internal_resume_ptid (int user_step)
+{
+  /* 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 ())
+    return inferior_ptid;
+  else
+    return user_visible_resume_ptid (user_step);
+}
+
 /* Wrapper for target_resume, that handles infrun-specific
    bookkeeping.  */
 
@@ -2384,15 +2403,7 @@  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
-		resume_ptid = user_visible_resume_ptid (user_step);
+	      resume_ptid = internal_resume_ptid (user_step);
 	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
 	      discard_cleanups (old_cleanups);
 	      tp->resumed = 1;
@@ -2501,20 +2512,9 @@  resume (enum gdb_signal sig)
      use singlestep breakpoint.  */
   gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
 
-  /* Decide the set of threads to ask the target to resume.  Start
-     by assuming everything will be resumed, than narrow the set
-     by applying increasingly restricting conditions.  */
-  resume_ptid = user_visible_resume_ptid (user_step);
-
-  /* Maybe resume a single thread after all.  */
-  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)
+  /* Decide the set of threads to ask the target to resume.  */
+  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
@@ -2523,6 +2523,8 @@  resume (enum gdb_signal sig)
 	 breakpoint if allowed to run.  */
       resume_ptid = inferior_ptid;
     }
+  else
+    resume_ptid = internal_resume_ptid (user_step);
 
   if (execution_direction != EXEC_REVERSE
       && step && breakpoint_inserted_here_p (aspace, pc))
@@ -6833,10 +6835,7 @@  keep_going_stepped_thread (struct thread_info *tp)
 				     stop_pc);
 
       tp->resumed = 1;
-      if (target_is_non_stop_p ())
-	resume_ptid = inferior_ptid;
-      else
-	resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+      resume_ptid = internal_resume_ptid (tp->control.stepping_command);
       do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
     }
   else