[PATCH] gdb: rename thread_control_state::trap_expected to stepping_over_breakpoint
Simon Marchi
simark@simark.ca
Tue Jun 16 18:03:06 GMT 2020
On 2020-05-28 4:00 p.m., Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> While reading this code, I felt that the field `trap_expected` of
> `struct thread_control_state` did not have a very clear name. It
> basically means that the thread is currently stepping over a breakpoint
> (that's what the comment above the field says anyway). I know this is
> subjective, but I think that naming the field `stepping_over_breakpoint`
> instead would be more descriptive of its function.
>
> My only concern is that `struct thread_info` already contains a
> `stepping_over_breakpoint` field, so it might be a bit confusing, and I
> don't really understand the different between these two fields.
>
> Also, the comment above thread_control_state::stepping_over_breakpoint
> sounds like it's a bit outdated/incomplete. It describes inline step
> overs, where we stop all other threads, but I think this variable is
> also set when the thread is doing a displaced step, and it doesn't
> mention that. I don't feel apt to update the comment right now though.
>
> While at it, change the field to bool.
>
> gdb/ChangeLog:
>
> * gdbthread.h (struct thread_control_state)
> <trap_expected>: Rename to...
> <stepping_over_breakpoint>: ... this. Update all users.
>
> Change-Id: Ifaf5eacf4c503ce2643456b6606482201cda303d
> ---
> gdb/gdbthread.h | 4 +--
> gdb/infrun.c | 81 +++++++++++++++++++++++++------------------------
> 2 files changed, 43 insertions(+), 42 deletions(-)
>
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 717a2ad08c2f..52f5e148d8d0 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -131,7 +131,7 @@ struct thread_control_state
> any inlined frames). */
> struct frame_id step_stack_frame_id {};
>
> - /* Nonzero if we are presently stepping over a breakpoint.
> + /* True if we are presently stepping over a breakpoint.
>
> If we hit a breakpoint or watchpoint, and then continue, we need
> to single step the current thread with breakpoints disabled, to
> @@ -153,7 +153,7 @@ struct thread_control_state
> wait_for_inferior, which calls handle_inferior_event in a loop,
> and until wait_for_inferior exits, this variable is changed only
> by keep_going. */
> - int trap_expected = 0;
> + bool stepping_over_breakpoint = false;
>
> /* Nonzero if the thread is being proceeded for a "finish" command
> or a similar situation when return value should be printed. */
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 95fc3bfe4593..620d03285266 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1685,7 +1685,7 @@ displaced_step_prepare_throw (thread_info *tp)
> gdb_assert (gdbarch_supports_displaced_stepping (gdbarch));
>
> /* Nor if the thread isn't meant to step over a breakpoint. */
> - gdb_assert (tp->control.trap_expected);
> + gdb_assert (tp->control.stepping_over_breakpoint);
>
> /* Disable range stepping while executing in the scratch pad. We
> want a single-step even if executing the displaced instruction in
> @@ -2017,15 +2017,15 @@ start_step_over (void)
> "infrun: step-over queue now empty\n");
> }
>
> - if (tp->control.trap_expected
> + if (tp->control.stepping_over_breakpoint
> || tp->resumed
> || tp->executing)
> {
> internal_error (__FILE__, __LINE__,
> "[%s] has inconsistent state: "
> - "trap_expected=%d, resumed=%d, executing=%d\n",
> + "stepping_over_breakpoint=%d, resumed=%d, executing=%d\n",
> target_pid_to_str (tp->ptid).c_str (),
> - tp->control.trap_expected,
> + tp->control.stepping_over_breakpoint,
> tp->resumed,
> tp->executing);
> }
> @@ -2056,7 +2056,7 @@ start_step_over (void)
> /* If we started a new in-line step-over, we're done. */
> if (step_over_info_valid_p ())
> {
> - gdb_assert (tp->control.trap_expected);
> + gdb_assert (tp->control.stepping_over_breakpoint);
> return 1;
> }
>
> @@ -2064,7 +2064,7 @@ start_step_over (void)
> {
> /* On all-stop, shouldn't have resumed unless we needed a
> step over. */
> - gdb_assert (tp->control.trap_expected
> + gdb_assert (tp->control.stepping_over_breakpoint
> || tp->step_after_step_resume_breakpoint);
>
> /* With remote targets (at least), in all-stop, we can't
> @@ -2362,9 +2362,9 @@ resume_1 (enum gdb_signal sig)
> if (debug_infrun)
> fprintf_unfiltered (gdb_stdlog,
> "infrun: resume (step=%d, signal=%s), "
> - "trap_expected=%d, current thread [%s] at %s\n",
> + "stepping_over_breakpoint=%d, current thread [%s] at %s\n",
> step, gdb_signal_to_symbol_string (sig),
> - tp->control.trap_expected,
> + tp->control.stepping_over_breakpoint,
> target_pid_to_str (inferior_ptid).c_str (),
> paddress (gdbarch, pc));
>
> @@ -2396,7 +2396,7 @@ resume_1 (enum gdb_signal sig)
> "deliver signal first\n");
>
> clear_step_over_info ();
> - tp->control.trap_expected = 0;
> + tp->control.stepping_over_breakpoint = false;
>
> if (tp->control.step_resume_breakpoint == NULL)
> {
> @@ -2448,7 +2448,7 @@ resume_1 (enum gdb_signal sig)
>
> /* If we have a breakpoint to step over, make sure to do a single
> step only. Same if we have software watchpoints. */
> - if (tp->control.trap_expected || bpstat_should_step ())
> + if (tp->control.stepping_over_breakpoint || bpstat_should_step ())
> tp->control.may_range_step = 0;
>
> /* If displaced stepping is enabled, step over breakpoints by executing a
> @@ -2462,7 +2462,7 @@ resume_1 (enum gdb_signal sig)
> We can't use displaced stepping when we are waiting for vfork_done
> event, displaced stepping breaks the vfork child similarly as single
> step software breakpoint. */
> - if (tp->control.trap_expected
> + if (tp->control.stepping_over_breakpoint
> && use_displaced_stepping (tp)
> && !step_over_info_valid_p ()
> && sig == GDB_SIGNAL_0
> @@ -2476,7 +2476,7 @@ resume_1 (enum gdb_signal sig)
> fprintf_unfiltered (gdb_stdlog,
> "Got placed in step-over queue\n");
>
> - tp->control.trap_expected = 0;
> + tp->control.stepping_over_breakpoint = 0;
> return;
> }
> else if (prepared < 0)
> @@ -2553,7 +2553,7 @@ resume_1 (enum gdb_signal sig)
> delete_single_step_breakpoints (tp);
>
> clear_step_over_info ();
> - tp->control.trap_expected = 0;
> + tp->control.stepping_over_breakpoint = 0;
>
> insert_breakpoints ();
> }
> @@ -2564,7 +2564,7 @@ resume_1 (enum gdb_signal sig)
> gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
>
> /* Decide the set of threads to ask the target to resume. */
> - if (tp->control.trap_expected)
> + if (tp->control.stepping_over_breakpoint)
> {
> /* We're allowing a thread to run past a breakpoint it has
> hit, either by single-stepping the thread with the breakpoint
> @@ -2626,7 +2626,7 @@ resume_1 (enum gdb_signal sig)
> }
>
> if (debug_displaced
> - && tp->control.trap_expected
> + && tp->control.stepping_over_breakpoint
> && use_displaced_stepping (tp)
> && !step_over_info_valid_p ())
> {
> @@ -2759,7 +2759,7 @@ clear_proceed_status_thread (struct thread_info *tp)
> delete tp->thread_fsm;
> tp->thread_fsm = NULL;
>
> - tp->control.trap_expected = 0;
> + tp->control.stepping_over_breakpoint = false;
> tp->control.step_range_start = 0;
> tp->control.step_range_end = 0;
> tp->control.may_range_step = 0;
> @@ -4983,7 +4983,7 @@ stop_all_threads (void)
> "step-over queue\n",
> target_pid_to_str (t->ptid).c_str ());
> }
> - t->control.trap_expected = 0;
> + t->control.stepping_over_breakpoint = false;
> thread_step_over_chain_enqueue (t);
> }
> }
> @@ -5014,7 +5014,7 @@ stop_all_threads (void)
> if (displaced_step_fixup (t, sig) < 0)
> {
> /* Add it back to the step-over queue. */
> - t->control.trap_expected = 0;
> + t->control.stepping_over_breakpoint = false;
> thread_step_over_chain_enqueue (t);
> }
>
> @@ -5804,7 +5804,7 @@ finish_step_over (struct execution_control_state *ecs)
> /* If we're stepping over a breakpoint with all threads locked,
> then only the thread that was stepped should be reporting
> back an event. */
> - gdb_assert (ecs->event_thread->control.trap_expected);
> + gdb_assert (ecs->event_thread->control.stepping_over_breakpoint);
>
> clear_step_over_info ();
> }
> @@ -6066,7 +6066,7 @@ handle_signal_stop (struct execution_control_state *ecs)
> delete_just_stopped_threads_single_step_breakpoints ();
>
> if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
> - && ecs->event_thread->control.trap_expected
> + && ecs->event_thread->control.stepping_over_breakpoint
> && ecs->event_thread->stepping_over_watchpoint)
> stopped_by_watchpoint = 0;
> else
> @@ -6145,7 +6145,7 @@ handle_signal_stop (struct execution_control_state *ecs)
> ecs->event_thread->suspend.stop_pc,
> &ecs->ws)
> && !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
> - && ecs->event_thread->control.trap_expected
> + && ecs->event_thread->control.stepping_over_breakpoint
> && pc_at_non_inline_function (aspace,
> ecs->event_thread->prev_pc,
> &ecs->ws)))
> @@ -6163,7 +6163,7 @@ handle_signal_stop (struct execution_control_state *ecs)
> }
>
> if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
> - && ecs->event_thread->control.trap_expected
> + && ecs->event_thread->control.stepping_over_breakpoint
> && gdbarch_single_step_through_delay_p (gdbarch)
> && currently_stepping (ecs->event_thread))
> {
> @@ -6368,7 +6368,7 @@ handle_signal_stop (struct execution_control_state *ecs)
> ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
>
> if (ecs->event_thread->prev_pc == ecs->event_thread->suspend.stop_pc
> - && ecs->event_thread->control.trap_expected
> + && ecs->event_thread->control.stepping_over_breakpoint
> && ecs->event_thread->control.step_resume_breakpoint == NULL)
> {
> /* We were just starting a new sequence, attempting to
> @@ -6388,8 +6388,8 @@ handle_signal_stop (struct execution_control_state *ecs)
>
> insert_hp_step_resume_breakpoint_at_frame (frame);
> ecs->event_thread->step_after_step_resume_breakpoint = 1;
> - /* Reset trap_expected to ensure breakpoints are re-inserted. */
> - ecs->event_thread->control.trap_expected = 0;
> + /* Reset stepping_over_breakpoint to ensure breakpoints are re-inserted. */
> + ecs->event_thread->control.stepping_over_breakpoint = false;
>
> /* If we were nexting/stepping some other thread, switch to
> it, so that we don't continue it, losing control. */
> @@ -6423,8 +6423,8 @@ handle_signal_stop (struct execution_control_state *ecs)
> clear_step_over_info ();
> insert_hp_step_resume_breakpoint_at_frame (frame);
> ecs->event_thread->step_after_step_resume_breakpoint = 1;
> - /* Reset trap_expected to ensure breakpoints are re-inserted. */
> - ecs->event_thread->control.trap_expected = 0;
> + /* Reset stepping_over_breakpoint to ensure breakpoints are re-inserted. */
> + ecs->event_thread->control.stepping_over_breakpoint = false;
> keep_going (ecs);
> return;
> }
> @@ -7340,7 +7340,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
>
> /* Check if the current thread is blocked on an incomplete
> step-over, interrupted by a random signal. */
> - if (ecs->event_thread->control.trap_expected
> + if (ecs->event_thread->control.stepping_over_breakpoint
> && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
> {
> if (debug_infrun)
> @@ -7392,9 +7392,9 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> return 0;
>
> /* Otherwise, we no longer expect a trap in the current thread.
> - Clear the trap_expected flag before switching back -- this is
> + Clear the stepping_over_breakpoint flag before switching back -- this is
> what keep_going does as well, if we call it. */
> - ecs->event_thread->control.trap_expected = 0;
> + ecs->event_thread->control.stepping_over_breakpoint = false;
>
> /* Likewise, clear the signal if it should not be passed. */
> if (!signal_program[ecs->event_thread->suspend.stop_signal])
> @@ -7426,13 +7426,13 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> except the one that needs to move past the breakpoint.
> If a non-event thread has this set, the "incomplete
> step-over" check above should have caught it earlier. */
> - if (tp->control.trap_expected)
> + if (tp->control.stepping_over_breakpoint)
> {
> internal_error (__FILE__, __LINE__,
> "[%s] has inconsistent state: "
> - "trap_expected=%d\n",
> + "stepping_over_breakpoint=%d\n",
> target_pid_to_str (tp->ptid).c_str (),
> - tp->control.trap_expected);
> + tp->control.stepping_over_breakpoint);
> }
>
> /* Did we find the stepping thread? */
> @@ -7555,7 +7555,7 @@ keep_going_stepped_thread (struct thread_info *tp)
> over this exact address in another thread, the breakpoint is
> skipped. */
> clear_step_over_info ();
> - tp->control.trap_expected = 0;
> + tp->control.stepping_over_breakpoint = false;
>
> insert_single_step_breakpoint (get_frame_arch (frame),
> get_frame_address_space (frame),
> @@ -7585,7 +7585,7 @@ currently_stepping (struct thread_info *tp)
> {
> return ((tp->control.step_range_end
> && tp->control.step_resume_breakpoint == NULL)
> - || tp->control.trap_expected
> + || tp->control.stepping_over_breakpoint
> || tp->stepped_breakpoint
> || bpstat_should_step ());
> }
> @@ -7993,13 +7993,13 @@ keep_going_pass_signal (struct execution_control_state *ecs)
> ecs->event_thread->prev_pc
> = regcache_read_pc_protected (get_thread_regcache (ecs->event_thread));
>
> - if (ecs->event_thread->control.trap_expected)
> + if (ecs->event_thread->control.stepping_over_breakpoint)
> {
> struct thread_info *tp = ecs->event_thread;
>
> if (debug_infrun)
> fprintf_unfiltered (gdb_stdlog,
> - "infrun: %s has trap_expected set, "
> + "infrun: %s has stepping_over_breakpoint set, "
> "resuming to collect trap\n",
> target_pid_to_str (tp->ptid).c_str ());
>
> @@ -8101,7 +8101,8 @@ keep_going_pass_signal (struct execution_control_state *ecs)
> return;
> }
>
> - ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
> + ecs->event_thread->control.stepping_over_breakpoint
> + = (remove_bp || remove_wps);
>
> resume (ecs->event_thread->suspend.stop_signal);
> }
> @@ -8116,9 +8117,9 @@ keep_going_pass_signal (struct execution_control_state *ecs)
> static void
> keep_going (struct execution_control_state *ecs)
> {
> - if (ecs->event_thread->control.trap_expected
> + if (ecs->event_thread->control.stepping_over_breakpoint
> && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
> - ecs->event_thread->control.trap_expected = 0;
> + ecs->event_thread->control.stepping_over_breakpoint = false;
>
> if (!signal_program[ecs->event_thread->suspend.stop_signal])
> ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
> --
> 2.26.2
>
Ping.
Simon
More information about the Gdb-patches
mailing list