diff mbox

[review,v2] Pass thread_info pointer to various inferior control functions

Message ID 20191220200239.77EC62816F@gnutoolchain-gerrit.osci.io
State New
Headers show

Commit Message

Simon Marchi (Code Review) Dec. 20, 2019, 8:02 p.m. UTC
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/321
......................................................................

Pass thread_info pointer to various inferior control functions

I noticed that some functions in infcmd and infrun call each other and
all call inferior_thread, while they could just get the thread_info
pointer from their caller.  That means less calls to inferior_thread, so
less reliance on global state, since inferior_thread reads
inferior_ptid.

The paths I am unsure about are:

  - fetch_inferior_event calls...
  - step_command_fsm::should_stop calls...
  - prepare_one_step

and

 - process_event_stop_test calls...
 - set_step_info

prepare_one_step used to get the thread from inferior_thread, whereas
fetch_inferior_event passes the thread from the execution_control_state
ecs.  Though that code path is used when a thread completes a step, but
the user had specified a step count (e.g. "step 5") so we decide to do
one more step.  It would be strange (and even a bug) if the thread in
the ecs structure in fetch_inferior_event was not the same thread that
is prepared to stepped by prepare_one_step.  So I believe passing the
ecs thread is fine.

The same logic applies to process_event_stop_test calling
set_step_info.

gdb/ChangeLog:

	* infrun.h: Forward-declare thread_info.
	(set_step_info): Add thread_info parameter, add doc.
	* infrun.c (set_step_info): Add thread_info parameter, move doc
	to header.
	* infrun.c (process_event_stop_test): Pass thread to
	set_step_info call.
	* infcmd.c (set_step_frame): Add thread_info pointer, pass it to
	set_step_info.
	(prepare_one_step): Add thread_info parameter, pass it to
	set_step_frame and prepare_one_step (recursive) call.
	(step_1): Pass thread to prepare_one_step call.
	(step_command_fsm::should_stop): Pass thread to
	prepare_one_step.
	(until_next_fsm): Pass thread to set_step_frame call.
	(finish_command): Pass thread to set_step_info call.

Change-Id: If72b559ce46940bcec46cc8c6bdf529ea9ef956b
---
M gdb/infcmd.c
M gdb/infrun.c
M gdb/infrun.h
3 files changed, 21 insertions(+), 25 deletions(-)
diff mbox

Patch

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index af66eaf..cc6a397 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -896,18 +896,17 @@ 
   continue_1 (all_threads);
 }
 
-/* Record the starting point of a "step" or "next" command.  */
+/* Record in TP the starting point of a "step" or "next" command.  */
 
 static void
-set_step_frame (void)
+set_step_frame (thread_info *tp)
 {
   frame_info *frame = get_current_frame ();
 
   symtab_and_line sal = find_frame_sal (frame);
-  set_step_info (frame, sal);
+  set_step_info (tp, frame, sal);
 
   CORE_ADDR pc = get_frame_pc (frame);
-  thread_info *tp = inferior_thread ();
   tp->control.step_start_function = find_pc_function (pc);
 }
 
@@ -984,7 +983,7 @@ 
   thread->control.stepping_command = 1;
 }
 
-static int prepare_one_step (struct step_command_fsm *sm);
+static int prepare_one_step (thread_info *, struct step_command_fsm *sm);
 
 static void
 step_1 (int skip_subroutines, int single_inst, const char *count_string)
@@ -1022,7 +1021,7 @@ 
      loop.  Let the continuation figure out how many other steps we
      need to do, and handle them one at the time, through
      step_once.  */
-  if (!prepare_one_step (step_sm))
+  if (!prepare_one_step (thr, step_sm))
     proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
   else
     {
@@ -1052,7 +1051,7 @@ 
       /* There are more steps to make, and we did stop due to
 	 ending a stepping range.  Do another step.  */
       if (--count > 0)
-	return prepare_one_step (this);
+	return prepare_one_step (tp, this);
 
       set_finished ();
     }
@@ -1084,19 +1083,13 @@ 
    resumed.  */
 
 static int
-prepare_one_step (struct step_command_fsm *sm)
+prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
 {
   if (sm->count > 0)
     {
       struct frame_info *frame = get_current_frame ();
 
-      /* Don't assume THREAD is a valid thread id.  It is set to -1 if
-	 the longjmp breakpoint was not required.  Use the
-	 INFERIOR_PTID thread instead, which is the same thread when
-	 THREAD is set.  */
-      struct thread_info *tp = inferior_thread ();
-
-      set_step_frame ();
+      set_step_frame (tp);
 
       if (!sm->single_inst)
 	{
@@ -1128,7 +1121,7 @@ 
 		  || !function_name_is_marked_for_skip (fn, sal))
 		{
 		  sm->count--;
-		  return prepare_one_step (sm);
+		  return prepare_one_step (tp, sm);
 		}
 	    }
 
@@ -1466,7 +1459,7 @@ 
   struct until_next_fsm *sm;
 
   clear_proceed_status (0);
-  set_step_frame ();
+  set_step_frame (tp);
 
   frame = get_current_frame ();
 
@@ -1923,7 +1916,7 @@ 
 	 called by that frame.  We don't use the magic "1" value for
 	 step_range_end, because then infrun will think this is nexti,
 	 and not step over the rest of this inlined function call.  */
-      set_step_info (frame, {});
+      set_step_info (tp, frame, {});
       tp->control.step_range_start = get_frame_pc (frame);
       tp->control.step_range_end = tp->control.step_range_start;
       tp->control.step_over_calls = STEP_OVER_ALL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7ddd21d..66cccaf 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3802,12 +3802,12 @@ 
     printf_unfiltered (_("completed.\n"));
 }
 
-/* Record the frame and location we're currently stepping through.  */
-void
-set_step_info (struct frame_info *frame, struct symtab_and_line sal)
-{
-  struct thread_info *tp = inferior_thread ();
+/* See infrun.h.  */
 
+void
+set_step_info (thread_info *tp, struct frame_info *frame,
+	       struct symtab_and_line sal)
+{
   tp->control.step_frame_id = get_frame_id (frame);
   tp->control.step_stack_frame_id = get_stack_frame_id (frame);
 
@@ -6804,7 +6804,7 @@ 
   ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
   ecs->event_thread->control.step_range_end = stop_pc_sal.end;
   ecs->event_thread->control.may_range_step = 1;
-  set_step_info (frame, stop_pc_sal);
+  set_step_info (ecs->event_thread, frame, stop_pc_sal);
 
   if (debug_infrun)
      fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 2b2a3a3..1e9d91f 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -25,6 +25,7 @@ 
 struct frame_info;
 struct address_space;
 struct return_value_info;
+struct thread_info;
 
 /* True if we are debugging run control.  */
 extern unsigned int debug_infrun;
@@ -133,7 +134,9 @@ 
    triggers a non-steppable watchpoint.  */
 extern int stepping_past_nonsteppable_watchpoint (void);
 
-extern void set_step_info (struct frame_info *frame,
+/* Record in TP the frame and location we're currently stepping through.  */
+extern void set_step_info (thread_info *tp,
+			   struct frame_info *frame,
 			   struct symtab_and_line sal);
 
 /* Several print_*_reason helper functions to print why the inferior