Re: Move threads out of jumppad without single step

[Yao Qi <qiyaoltc at gmail dot com>]

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> I've tested this in all stop/non stop and it works properly.
>
> Basically what happens is that if stabilize_threads is not called in
> the context of linux_resume and that gdbserver needs to report an
> event, it won't since last_resume_kind can be resume_stop.
>
> In the current case gdbserver is in cmd_qtdp, the last command was
> continue (vCont;c) in all stop mode so last_resume_kind is
> resume_stop.
>
> So when going in linux_wait, the event is filtered out by :
>  event_thread = (struct thread_info *)
> 	find_inferior (&all_threads, status_pending_p_callback, &filter_ptid);
>
> Since status_pending_p_callback returns false.
>
> Note that this fix may not the best one... but it may be some progress...
>
> Any ideas are welcome, otherwise I will add it to my patch set and
> there can be more discussion at review.

Hi Antoine,
I don't have an idea to your problem and your fix, but I don't
understand why don't we see this problem before.  I may miss some
details of your problem, so please include these details in your
patches.

What I want to say here is that we still need more tests and works to
get software single step properly/fully engaged with the rest part of
GDBserver before we introduce (fast) tracepoint for ARM into GDBserver.
The software single step in GDBserver isn't fully exercised yet, for
example, GDB still doesn't emit vCont;s and vCont;S to ask GDBserver to
do single step on arm-linux.  If we force GDB to emit vCont;s on
arm-linux, as the patch below does, there are still some problems,

 1. PTRACE_SINGLESTEP is used in ptrace call in GDBserver, which is
 obviously wrong,

 2. PC increment is still incorrect if the single step is requested by
 GDB.  We've had a fix https://sourceware.org/ml/gdb-patches/2015-11/msg00470.html
 to fix the PC increment when if single step is requested by GDBserver
 itself.

I did some fixes but there are still some work to do software single
step in GDBserver requested by GDB.  I don't want to block your fast
tracepoint work, just raise these issues, and let you know my thoughts.

-- 
Yao (éå)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 3421f3b..dead2b9 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -935,11 +935,6 @@ arm_linux_software_single_step (struct frame_info *frame)
   VEC (CORE_ADDR) *next_pcs = NULL;
   struct cleanup *old_chain = make_cleanup (VEC_cleanup (CORE_ADDR), &next_pcs);
 
-  /* If the target does have hardware single step, GDB doesn't have
-     to bother software single step.  */
-  if (target_can_do_single_step () == 1)
-    return 0;
-
   arm_get_next_pcs_ctor (&next_pcs_ctx,
 			 &arm_linux_get_next_pcs_ops,
 			 gdbarch_byte_order (gdbarch),
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index ef715e7..0d51dec 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2958,12 +2958,15 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
 	{
 	  strcpy (own_buf, "vCont;c;C;t");
 
-	  if (target_supports_hardware_single_step () || !vCont_supported)
+	  if (target_supports_hardware_single_step ()
+	      || target_supports_software_single_step ()
+	      || !vCont_supported)
 	    {
-	      /* If target supports hardware single step, add actions s
-		 and S to the list of supported actions.  On the other
-		 hand, if GDB doesn't request the supported vCont actions
-		 in qSupported packet, add s and S to the list too.  */
+	      /* If target supports single step either by hardware or by
+		 software, add actions s and S to the list of supported
+		 actions.  On the other hand, if GDB doesn't request the
+		 supported vCont actions in qSupported packet, add s and
+		 S to the list too.  */
 	      own_buf = own_buf + strlen (own_buf);
 	      strcpy (own_buf, ";s;S");
 	    }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 15210c9..3e2d76e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2246,6 +2246,11 @@ maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
   int hw_step = 1;
 
+  /* If the target does have hardware single step, GDB doesn't have
+     to bother software single step.  */
+  if (target_can_do_single_step () == 1)
+    return 1;
+
   if (execution_direction == EXEC_FORWARD
       && gdbarch_software_single_step_p (gdbarch)
       && gdbarch_software_single_step (gdbarch, get_current_frame ()))