[review] Pass thread_info pointer to various inferior control functions
Commit Message
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(-)
Comments
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/321
......................................................................
Patch Set 1:
Ping.
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/321
......................................................................
Patch Set 2:
I rebased this on master, there was a small conflict.
Simon Marchi has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/321 )
Change subject: Pass thread_info pointer to various inferior control functions
......................................................................
Abandoned
Moved to the mailing list: https://sourceware.org/ml/gdb-patches/2020-02/msg00546.html
@@ -901,18 +901,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);
}
@@ -989,7 +988,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)
@@ -1027,7 +1026,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
{
@@ -1057,7 +1056,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 ();
}
@@ -1089,19 +1088,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)
{
@@ -1119,7 +1112,7 @@
step_into_inline_frame (tp);
sm->count--;
- return prepare_one_step (sm);
+ return prepare_one_step (tp, sm);
}
pc = get_frame_pc (frame);
@@ -1456,7 +1449,7 @@
struct until_next_fsm *sm;
clear_proceed_status (0);
- set_step_frame ();
+ set_step_frame (tp);
frame = get_current_frame ();
@@ -1913,7 +1906,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;
@@ -3809,12 +3809,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);
@@ -6763,7 +6763,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");
@@ -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