Switch to current thread in finish_step_over
authorYao Qi <yao.qi@linaro.org>
Fri, 17 Jun 2016 09:25:12 +0000 (10:25 +0100)
committerYao Qi <yao.qi@linaro.org>
Fri, 17 Jun 2016 09:38:19 +0000 (10:38 +0100)
This patch adds some sanity check that reinsert breakpoints must be
there when doing step-over on software single step target.  The check
triggers an assert when running forking-threads-plus-breakpoint.exp
on arm-linux target,

 gdb/gdbserver/linux-low.c:4714: A problem internal to GDBserver has been detected.^M
 int finish_step_over(lwp_info*): Assertion `has_reinsert_breakpoints ()' failed.

the error happens when GDBserver has already resumed a thread of
process A for step-over (and wait for it hitting reinsert breakpoint),
but receives detach request for process B from GDB, which is shown in
the backtrace below,

 (gdb) bt
 #2  0x000228aa in finish_step_over (lwp=0x12bbd98) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4703
 #3  0x00025a50 in finish_step_over (lwp=0x12bbd98) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4749
 #4  complete_ongoing_step_over () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4760
 #5  linux_detach (pid=25228) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:1503
 #6  0x00012bae in process_serial_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3974
 #7  handle_serial_event (err=<optimized out>, client_data=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4347
 #8  0x00016d68 in handle_file_event (event_file_desc=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:429
 #9  0x000173ea in process_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:184
 #10 start_event_loop () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:547
 #11 0x0000aa2c in captured_main (argv=<optimized out>, argc=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3719
 #12 main (argc=<optimized out>, argv=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3804

the sanity check tries to find the reinsert breakpoint from process B,
but nothing is found.  It is wrong, we need to search in process A,
since we started step-over of a thread of process A.

 (gdb) p lwp->thread->entry.id
 $3 = {pid = 25120, lwp = 25131, tid = 0}
 (gdb) p current_thread->entry.id
 $4 = {pid = 25228, lwp = 25228, tid = 0}

This patch switched current_thread to the thread we are doing step-over
in finish_step_over.

gdb/gdbserver:

2016-06-17  Yao Qi  <yao.qi@linaro.org>

* linux-low.c (maybe_hw_step): New function.
(linux_resume_one_lwp_throw): Call maybe_hw_step.
(finish_step_over): Switch current_thread to lwp temporarily,
and assert has_reinsert_breakpoints returns true.
(proceed_one_lwp): Call maybe_hw_step.
* mem-break.c (has_reinsert_breakpoints): New function.
* mem-break.h (has_reinsert_breakpoints): Declare.

gdb/gdbserver/ChangeLog
gdb/gdbserver/linux-low.c
gdb/gdbserver/mem-break.c
gdb/gdbserver/mem-break.h

index 079507ae306d941ce8a7101de158b6018c54d449..70f64fff19b80ec92847dd1914f0cab12924611f 100644 (file)
@@ -1,3 +1,13 @@
+2016-06-17  Yao Qi  <yao.qi@linaro.org>
+
+       * linux-low.c (maybe_hw_step): New function.
+       (linux_resume_one_lwp_throw): Call maybe_hw_step.
+       (finish_step_over): Switch current_thread to lwp temporarily,
+       and assert has_reinsert_breakpoints returns true.
+       (proceed_one_lwp): Call maybe_hw_step.
+       * mem-break.c (has_reinsert_breakpoints): New function.
+       * mem-break.h (has_reinsert_breakpoints): Declare.
+
 2016-06-02  Jon Turney  <jon.turney@dronecode.org.uk>
 
        * win32-low.c (win32_create_inferior): Add pointer casts for C++.
index 81134b0c5258eb43e4fd6c966d3d9f6dd19e33ca..77c296ca2a1970ff0c8a6a9573ad90de7e53da81 100644 (file)
@@ -2495,6 +2495,24 @@ linux_low_filter_event (int lwpid, int wstat)
   return child;
 }
 
+/* Return true if THREAD is doing hardware single step.  */
+
+static int
+maybe_hw_step (struct thread_info *thread)
+{
+  if (can_hardware_single_step ())
+    return 1;
+  else
+    {
+      struct process_info *proc = get_thread_process (thread);
+
+      /* GDBserver must insert reinsert breakpoint for software
+        single step.  */
+      gdb_assert (has_reinsert_breakpoints (proc));
+      return 0;
+    }
+}
+
 /* Resume LWPs that are currently stopped without any pending status
    to report, but are resumed from the core's perspective.  */
 
@@ -4215,9 +4233,9 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
                fprintf (stderr, "BAD - reinserting and suspended(%d).\n",
                         lwp->suspended);
            }
-
-         step = 1;
        }
+
+      step = maybe_hw_step (thread);
     }
 
   if (fast_tp_collecting == 1)
@@ -4689,9 +4707,13 @@ finish_step_over (struct lwp_info *lwp)
 {
   if (lwp->bp_reinsert != 0)
     {
+      struct thread_info *saved_thread = current_thread;
+
       if (debug_threads)
        debug_printf ("Finished step over.\n");
 
+      current_thread = get_lwp_thread (lwp);
+
       /* Reinsert any breakpoint at LWP->BP_REINSERT.  Note that there
         may be no breakpoint to reinsert there by now.  */
       reinsert_breakpoints_at (lwp->bp_reinsert);
@@ -4705,9 +4727,13 @@ finish_step_over (struct lwp_info *lwp)
         because we were stepping over a breakpoint, and we hold all
         threads but LWP stopped while doing that.  */
       if (!can_hardware_single_step ())
-       delete_reinsert_breakpoints ();
+       {
+         gdb_assert (has_reinsert_breakpoints (current_process ()));
+         delete_reinsert_breakpoints ();
+       }
 
       step_over_bkpt = null_ptid;
+      current_thread = saved_thread;
       return 1;
     }
   else
@@ -5029,7 +5055,8 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
       if (debug_threads)
        debug_printf ("   stepping LWP %ld, reinsert set\n",
                      lwpid_of (thread));
-      step = 1;
+
+      step = maybe_hw_step (thread);
     }
   else
     step = 0;
index 363d7ca56289aac7eebc7ae7fd16fb5e8c6992bb..3313459b9a21d804dc4d683ddb1f5407c8925785 100644 (file)
@@ -1553,6 +1553,28 @@ reinsert_breakpoints_at (CORE_ADDR pc)
     }
 }
 
+int
+has_reinsert_breakpoints (struct process_info *proc)
+{
+  struct breakpoint *bp, **bp_link;
+
+  bp = proc->breakpoints;
+  bp_link = &proc->breakpoints;
+
+  while (bp)
+    {
+      if (bp->type == reinsert_breakpoint)
+       return 1;
+      else
+       {
+         bp_link = &bp->next;
+         bp = *bp_link;
+       }
+    }
+
+  return 0;
+}
+
 void
 reinsert_all_breakpoints (void)
 {
index 4d9a76c23d467e64ea336562412737c5c53cefc4..b84dc1e2415f5eab6e6ed7cd5e27e2aea96bb72c 100644 (file)
@@ -163,6 +163,10 @@ void delete_reinsert_breakpoints (void);
 
 void reinsert_breakpoints_at (CORE_ADDR where);
 
+/* Process PROC has reinsert breakpoints or not.  */
+
+int has_reinsert_breakpoints (struct process_info *proc);
+
 /* Uninsert breakpoints at WHERE (and change their status to
    uninserted).  This still leaves the breakpoints in the table.  */