public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v2 07/23] Embed the pending step-over chain in thread_info objects
Date: Tue, 07 Apr 2015 12:59:00 -0000	[thread overview]
Message-ID: <1428410990-28560-8-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1428410990-28560-1-git-send-email-palves@redhat.com>

In order to teach non-stop mode to do in-line step-overs (pause all
threads, remove breakpoint, single-step, reinsert breakpoint, restart
threads), we'll need to be able to queue in-line step over requests,
much like we queue displaced stepping (out-of-line) requests.
Actually, the queue should be the same -- threads wait for their turn
to step past something (breakpoint, watchpoint), doesn't matter what
technique we end up using when the step over actually starts.

I found that the queue management ends up simpler and more efficient
if embedded in the thread objects themselves.  This commit converts
the existing displaced stepping queue to that.  Later patches will
make the in-line step-overs code paths use it too.

gdb/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (struct thread_info) <step_over_prev,
	step_over_next>: New fields.
	(thread_step_over_chain_enqueue, thread_step_over_chain_remove)
	(inferior_step_over_chain_remove_all, step_over_chain_dequeue):
	New declarations.
	* inferior.h (struct inferior) <step_over_queue_head>: New field.
	* infrun.c (struct displaced_step_request): Delete.
	(struct displaced_step_inferior_state) <step_request_queue>:
	Delete field.
	(displaced_step_in_progress): New function.
	(displaced_step_prepare): Assert that trap_expected is set.  Use
	thread_step_over_chain_enqueue.  Split starting a new displaced
	stepping to ...
	(start_step_over_inferior): ... this new function.
	(start_step_over): New function.
	(infrun_thread_ptid_changed): Delete references to the old
	displaced step request queue.
	(proceed): Assert the thread isn't waiting for a step over
	already.
	(infrun_thread_stop_requested): Adjust to remove threads from the
	embedded step-over chain.
	(handle_inferior_event) <fork/vfork>: Call start_step_over after
	displaced_step_fixup.
	(handle_signal_stop): Call start_step_over after
	displaced_step_fixup.
	* thread.c (step_over_chain_enqueue, step_over_chain_remove)
	(step_over_chain_dequeue, thread_step_over_chain_enqueue)
	(thread_step_over_chain_remove)
	(inferior_step_over_chain_remove_all): New function.
	(delete_thread_1): Remove thread from the step-over chain.
---
 gdb/gdbthread.h |  20 +++++++++
 gdb/inferior.h  |   4 ++
 gdb/infrun.c    | 128 +++++++++++++++++++++++++++++---------------------------
 gdb/thread.c    |  93 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 184 insertions(+), 61 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index bb15717..e654432 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -285,6 +285,10 @@ struct thread_info
   /* Values that are stored as temporaries on stack while evaluating
      expressions.  */
   value_vec *stack_temporaries;
+
+  /* Step-over chain.  */
+  struct thread_info *step_over_prev;
+  struct thread_info *step_over_next;
 };
 
 /* Create an empty thread list, or empty the existing one.  */
@@ -498,6 +502,22 @@ extern struct value *get_last_thread_stack_temporary (ptid_t);
 
 extern int value_in_thread_stack_temporaries (struct value *, ptid_t);
 
+/* Add TP to the end of its inferior's pending step-over chain.  */
+
+extern void thread_step_over_chain_enqueue (struct thread_info *tp);
+
+/* Remove TP from its inferior's pending step-over chain.  */
+
+extern void thread_step_over_chain_remove (struct thread_info *tp);
+
+/* Remove all threads from inferior INF's pending step-over chain.  */
+
+extern void inferior_step_over_chain_remove_all (struct inferior *inf);
+
+/* Remove the head of LIST_P, a pending step-over chain.  */
+
+extern void step_over_chain_dequeue (struct thread_info **list_p);
+
 extern struct thread_info *thread_list;
 
 #endif /* GDBTHREAD_H */
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 2530777..3cec101 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -410,6 +410,10 @@ struct inferior
      this gdbarch.  */
   struct gdbarch *gdbarch;
 
+  /* The queue of this inferior's threads that need to do a step-over
+     operation to get past e.g., a breakpoint.  */
+  struct thread_info *step_over_queue_head;
+
   /* Per inferior data-pointers required by other GDB modules.  */
   REGISTRY_FIELDS;
 };
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c544362..937a29d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1406,12 +1406,6 @@ step_over_info_valid_p (void)
    displaced step operation on it.  See displaced_step_prepare and
    displaced_step_fixup for details.  */
 
-struct displaced_step_request
-{
-  ptid_t ptid;
-  struct displaced_step_request *next;
-};
-
 /* Per-inferior displaced stepping state.  */
 struct displaced_step_inferior_state
 {
@@ -1421,10 +1415,6 @@ struct displaced_step_inferior_state
   /* The process this displaced step state refers to.  */
   int pid;
 
-  /* A queue of pending displaced stepping requests.  One entry per
-     thread that needs to do a displaced step.  */
-  struct displaced_step_request *step_request_queue;
-
   /* If this is not null_ptid, this is the thread carrying out a
      displaced single-step in process PID.  This thread's state will
      require fixing up once it has completed its step.  */
@@ -1674,6 +1664,9 @@ displaced_step_prepare (ptid_t ptid)
      support displaced stepping.  */
   gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
 
+  /* Nor if the thread isn't meant to step over a breakpoint.  */
+  gdb_assert (tp->control.trap_expected);
+
   /* Disable range stepping while executing in the scratch pad.  We
      want a single-step even if executing the displaced instruction in
      the scratch buffer lands within the stepping range (e.g., a
@@ -1689,28 +1682,13 @@ displaced_step_prepare (ptid_t ptid)
     {
       /* Already waiting for a displaced step to finish.  Defer this
 	 request and place in queue.  */
-      struct displaced_step_request *req, *new_req;
 
       if (debug_displaced)
 	fprintf_unfiltered (gdb_stdlog,
-			    "displaced: defering step of %s\n",
+			    "displaced: deferring step of %s\n",
 			    target_pid_to_str (ptid));
 
-      new_req = xmalloc (sizeof (*new_req));
-      new_req->ptid = ptid;
-      new_req->next = NULL;
-
-      if (displaced->step_request_queue)
-	{
-	  for (req = displaced->step_request_queue;
-	       req && req->next;
-	       req = req->next)
-	    ;
-	  req->next = new_req;
-	}
-      else
-	displaced->step_request_queue = new_req;
-
+      thread_step_over_chain_enqueue (tp);
       return 0;
     }
   else
@@ -1856,24 +1834,34 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
   do_cleanups (old_cleanups);
 
   displaced->step_ptid = null_ptid;
+}
+
+/* Are there any pending step-over requests for INF?  */
 
-  /* Are there any pending displaced stepping requests?  If so, run
-     one now.  Leave the state object around, since we're likely to
-     need it again soon.  */
-  while (displaced->step_request_queue)
+static void
+start_step_over_inferior (struct inferior *inf)
+{
+  /* Don't start a new step-over if we already have a displaced step
+     operation ongoing.  */
+  if (displaced_step_in_progress (inf->pid))
+    return;
+
+  while (inf->step_over_queue_head != NULL)
     {
-      struct displaced_step_request *head;
       ptid_t ptid;
+      struct displaced_step_inferior_state *displaced;
       struct regcache *regcache;
       struct gdbarch *gdbarch;
       CORE_ADDR actual_pc;
       struct address_space *aspace;
+      struct thread_info *tp;
+
+      tp = inf->step_over_queue_head;
+      displaced = get_displaced_stepping_state (inf->pid);
 
-      head = displaced->step_request_queue;
-      ptid = head->ptid;
-      displaced->step_request_queue = head->next;
-      xfree (head);
+      step_over_chain_dequeue (&inf->step_over_queue_head);
 
+      ptid = tp->ptid;
       context_switch (ptid);
 
       regcache = get_thread_regcache (ptid);
@@ -1935,6 +1923,25 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 	     thread waiting for its turn.  */
 	}
     }
+
+  if (inf->step_over_queue_head == NULL)
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: step-over queue of process %d now empty\n",
+			    inf->pid);
+    }
+}
+
+/* Are there any pending step-over requests for the inferior of
+   EVENT_PTID?  */
+
+static void
+start_step_over (ptid_t event_ptid)
+{
+  struct inferior *inf = find_inferior_ptid (event_ptid);
+
+  start_step_over_inferior (inf);
 }
 
 /* Update global variables holding ptids to hold NEW_PTID if they were
@@ -1954,10 +1961,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
     {
       if (ptid_equal (displaced->step_ptid, old_ptid))
 	displaced->step_ptid = new_ptid;
-
-      for (it = displaced->step_request_queue; it; it = it->next)
-	if (ptid_equal (it->ptid, old_ptid))
-	  it->ptid = new_ptid;
     }
 }
 
@@ -2664,6 +2667,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   /* Fill in with reasonable starting values.  */
   init_thread_stepping_state (tp);
 
+  gdb_assert (tp->step_over_next == NULL);
+  gdb_assert (tp->step_over_prev == NULL);
+
   if (addr == (CORE_ADDR) -1)
     {
       if (pc == stop_pc
@@ -2971,35 +2977,31 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
 static void
 infrun_thread_stop_requested (ptid_t ptid)
 {
-  struct displaced_step_inferior_state *displaced;
-
   /* PTID was requested to stop.  Remove it from the displaced
      stepping queue, so we don't try to resume it automatically.  */
 
-  for (displaced = displaced_step_inferior_states;
-       displaced;
-       displaced = displaced->next)
+  if (ptid_equal (minus_one_ptid, ptid))
     {
-      struct displaced_step_request *it, **prev_next_p;
+      struct inferior *inf;
 
-      it = displaced->step_request_queue;
-      prev_next_p = &displaced->step_request_queue;
-      while (it)
+      ALL_INFERIORS (inf)
 	{
-	  if (ptid_match (it->ptid, ptid))
-	    {
-	      *prev_next_p = it->next;
-	      it->next = NULL;
-	      xfree (it);
-	    }
-	  else
-	    {
-	      prev_next_p = &it->next;
-	    }
-
-	  it = *prev_next_p;
+	  inferior_step_over_chain_remove_all (inf);
 	}
     }
+  else if (ptid_is_pid (ptid))
+    {
+      struct inferior *inf = find_inferior_ptid (ptid);
+
+      inferior_step_over_chain_remove_all (inf);
+    }
+  else
+    {
+      struct thread_info *tp = find_thread_ptid (ptid);
+
+      if (tp->step_over_next != NULL)
+	thread_step_over_chain_remove (tp);
+    }
 
   iterate_over_threads (infrun_thread_stop_requested_callback, &ptid);
 }
@@ -4050,6 +4052,9 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	       that this operation also cleans up the child process for vfork,
 	       because their pages are shared.  */
 	    displaced_step_fixup (ecs->ptid, GDB_SIGNAL_TRAP);
+	    /* Start a new step-over in another thread if there's one
+	       that needs it.  */
+	    start_step_over (ecs->ptid);
 
 	    if (ecs->ws.kind == TARGET_WAITKIND_FORKED)
 	      {
@@ -4282,6 +4287,7 @@ handle_signal_stop (struct execution_control_state *ecs)
      the PC, so do it here, before we set stop_pc.)  */
   displaced_step_fixup (ecs->ptid,
 			ecs->event_thread->suspend.stop_signal);
+  start_step_over (ecs->ptid);
 
   /* If we either finished a single-step or hit a breakpoint, but
      the user wanted this thread to be stopped, pretend we got a
diff --git a/gdb/thread.c b/gdb/thread.c
index db631c9..d5c9896 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -307,6 +307,95 @@ add_thread (ptid_t ptid)
   return add_thread_with_info (ptid, NULL);
 }
 
+/* Add TP to the end of the step-over chain LIST_P.  */
+
+static void
+step_over_chain_enqueue (struct thread_info **list_p, struct thread_info *tp)
+{
+  gdb_assert (tp->step_over_next == NULL);
+  gdb_assert (tp->step_over_prev == NULL);
+
+  if (*list_p == NULL)
+    {
+      *list_p = tp;
+      tp->step_over_prev = tp->step_over_next = tp;
+    }
+  else
+    {
+      struct thread_info *head = *list_p;
+      struct thread_info *tail = head->step_over_prev;
+
+      tp->step_over_prev = tail;
+      tp->step_over_next = head;
+      head->step_over_prev = tp;
+      tail->step_over_next = tp;
+    }
+}
+
+/* Remove TP from step-over chain LIST_P.  */
+
+static void
+step_over_chain_remove (struct thread_info **list_p, struct thread_info *tp)
+{
+  gdb_assert (tp->step_over_next != NULL);
+  gdb_assert (tp->step_over_prev != NULL);
+
+  if (*list_p == tp)
+    {
+      if (tp == tp->step_over_next)
+	*list_p = NULL;
+      else
+	*list_p = tp->step_over_next;
+    }
+
+  tp->step_over_prev->step_over_next = tp->step_over_next;
+  tp->step_over_next->step_over_prev = tp->step_over_prev;
+  tp->step_over_prev = tp->step_over_next = NULL;
+}
+
+/* See gdbthread.h.  */
+
+void
+step_over_chain_dequeue (struct thread_info **list_p)
+{
+  step_over_chain_remove (list_p, *list_p);
+}
+
+/* See gdbthread.h.  */
+
+void
+thread_step_over_chain_enqueue (struct thread_info *tp)
+{
+  struct inferior *inf;
+
+  inf = find_inferior_ptid (tp->ptid);
+  gdb_assert (inf != NULL);
+
+  step_over_chain_enqueue (&inf->step_over_queue_head, tp);
+}
+
+/* See gdbthread.h.  */
+
+void
+thread_step_over_chain_remove (struct thread_info *tp)
+{
+  struct inferior *inf;
+
+  inf = find_inferior_ptid (tp->ptid);
+  gdb_assert (inf != NULL);
+
+  step_over_chain_remove (&inf->step_over_queue_head, tp);
+}
+
+/* See gdbthread.h.  */
+
+void
+inferior_step_over_chain_remove_all (struct inferior *inf)
+{
+  while (inf->step_over_queue_head != NULL)
+    step_over_chain_dequeue (&inf->step_over_queue_head);
+}
+
 /* Delete thread PTID.  If SILENT, don't notify the observer of this
    exit.  */
 static void
@@ -323,6 +412,10 @@ delete_thread_1 (ptid_t ptid, int silent)
   if (!tp)
     return;
 
+  /* Dead threads don't need to step-over.  Remove from queue.  */
+  if (tp->step_over_next != NULL)
+    thread_step_over_chain_remove (tp);
+
   /* If this is the current thread, or there's code out there that
      relies on it existing (refcount > 0) we can't delete yet.  Mark
      it as exited, and notify it.  */
-- 
1.9.3

  parent reply	other threads:[~2015-04-07 12:59 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-07 12:49 [PATCH v2 00/23] All-stop on top of non-stop Pedro Alves
2015-04-07 12:50 ` [PATCH v2 15/23] Implement all-stop on top of a target running non-stop mode Pedro Alves
2015-04-07 13:36   ` Eli Zaretskii
2015-04-08  9:34   ` Yao Qi
2015-04-08  9:53     ` Pedro Alves
2015-04-08 11:08       ` Pedro Alves
2015-04-08 19:35         ` Pedro Alves
2015-04-08 19:41           ` Pedro Alves
2015-04-07 12:50 ` [PATCH v2 16/23] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop Pedro Alves
2015-04-07 12:50 ` [PATCH v2 02/23] Fix and test "checkpoint" in non-stop mode Pedro Alves
2015-04-07 12:50 ` [PATCH v2 21/23] PPC64: Fix gdb.arch/ppc64-atomic-inst.exp with displaced stepping Pedro Alves
2015-04-07 12:50 ` [PATCH v2 20/23] PPC64: symbol-file + exec-file results in broken " Pedro Alves
2015-04-07 12:50 ` [PATCH v2 12/23] Misc switch_back_to_stepped_thread cleanups Pedro Alves
2015-04-07 12:50 ` [PATCH v2 04/23] Change adjust_pc_after_break's prototype Pedro Alves
2015-04-07 12:50 ` [PATCH v2 11/23] Use keep_going in proceed and start_step_over too Pedro Alves
2015-04-07 12:50 ` [PATCH v2 22/23] S/390: displaced stepping and PC-relative RIL-b/RIL-c instructions Pedro Alves
2015-04-07 12:50 ` [PATCH v2 10/23] PPC64: Fix step-over-trips-on-watchpoint.exp with displaced stepping on Pedro Alves
2015-04-07 12:50 ` [PATCH v2 09/23] Make gdb.threads/step-over-trips-on-watchpoint.exp effective on !x86 Pedro Alves
2015-04-07 12:50 ` [PATCH v2 05/23] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG Pedro Alves
2015-04-07 12:50 ` [PATCH v2 23/23] native Linux: enable always non-stop by default Pedro Alves
2015-04-07 12:50 ` [PATCH v2 19/23] Disable displaced stepping if trying it fails Pedro Alves
2015-04-07 12:50 ` [PATCH v2 01/23] Fix gdb.base/sigstep.exp with displaced stepping on software single-step targets Pedro Alves
2015-04-10  9:56   ` Pedro Alves
2015-04-07 12:50 ` [PATCH v2 03/23] PR13858 - Can't do displaced stepping with no symbols Pedro Alves
2015-04-09 12:46   ` Pedro Alves
2015-04-07 12:55 ` [PATCH v2 17/23] Fix interrupt-noterm.exp on targets always in non-stop Pedro Alves
2015-04-07 12:57 ` [PATCH v2 08/23] Test step-over-{lands-on-breakpoint|trips-on-watchpoint}.exp with displaced stepping Pedro Alves
2015-04-10 14:54   ` Pedro Alves
2015-04-07 12:59 ` Pedro Alves [this message]
2015-04-07 12:59 ` [PATCH v2 13/23] Factor out code to re-resume stepped thread Pedro Alves
2015-04-07 12:59 ` [PATCH v2 14/23] Teach non-stop to do in-line step-overs (stop all, step, restart) Pedro Alves
2015-04-07 13:30 ` [PATCH v2 18/23] Fix step-over-{trips-on-watchpoint|lands-on-breakpoint}.exp race Pedro Alves
2015-04-07 13:30 ` [PATCH v2 06/23] Make thread_still_needs_step_over consider stepping_over_watchpoint too Pedro Alves
2015-04-08  9:28   ` Yao Qi
2015-04-13 10:47     ` Pedro Alves
2015-04-08  9:45 ` [PATCH v2 00/23] All-stop on top of non-stop Yao Qi
2015-04-08 10:17   ` Pedro Alves
2015-04-08 10:30     ` Pedro Alves
2015-04-10  8:41     ` Yao Qi
2015-04-10  8:50       ` Pedro Alves
2015-04-10  8:22 ` Yao Qi
2015-04-10  8:34   ` Pedro Alves
2015-04-10  9:26     ` Yao Qi
2015-04-13 15:28       ` Pedro Alves
2015-04-13 16:16         ` Yao Qi
2015-04-13 16:23           ` Pedro Alves
2015-04-13 16:23           ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1428410990-28560-8-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).