From 64ce06e4cd025f3486465a0070baad47248ec69e Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 24 Mar 2015 17:50:31 +0000 Subject: [PATCH] Remove 'step' parameters from 'proceed' and 'resume' The "step" parameters of 'proceed' and 'resume' aren't really useful as indication of whether run control wants to single-step the target, as that information must already be retrievable from currently_stepping. In fact, if currently_stepping disagrees with whether we single-stepped the target, then things break. Thus instead of having the same information in two places, this patch removes those parameters. Setting 'step_start_function' is the only user of proceed's 'step' argument, other than passing the 'step' argument down to 'resume' and debug log output. Move that instead to set_step_frame, where we already set other related fields. clear_proceed_status keeps its "step" parameter for now because it needs to know which set of threads should have their state cleared, and is called before the "stepping_command" flag is set. Tested on x86_64 Fedora 20, native and gdbserver. gdb/ChangeLog: 2015-03-24 Pedro Alves * breakpoint.c (until_break_command): Adjust call to proceed. * gdbthread.h (struct thread_control_state) : New field. * infcall.c (run_inferior_call): Adjust call to proceed. * infcmd.c (run_command_1, proceed_thread_callback, continue_1): Adjust calls to proceed. (set_step_frame): Set the current thread's step_start_function here. (step_once): Adjust calls to proceed. (jump_command, signal_command, until_next_command) (finish_backward, finish_forward, proceed_after_attach_callback) (attach_command_post_wait): Adjust calls to proceed. * infrun.c (proceed_after_vfork_done): Adjust call to proceed. (do_target_resume): New function, factored out from ... (resume): ... here. Remove 'step' parameter. Instead, check currently_stepping to determine whether the thread should be single-stepped. (proceed): Remove 'step' parameter and don't set the thread's step_start_function here. Adjust call to 'resume'. (handle_inferior_event): Adjust calls to 'resume'. (switch_back_to_stepped_thread): Use do_target_resume instead of 'resume'. (keep_going): Adjust calls to 'resume'. * infrun.h (proceed): Remove 'step' parameter. (resume): Likewise. * windows-nat.c (do_initial_windows_stuff): Adjust call to 'resume'. * mi/mi-main.c (proceed_thread): Adjust call to 'proceed'. --- gdb/ChangeLog | 31 ++++++++++++++++++++ gdb/breakpoint.c | 2 +- gdb/infcall.c | 2 +- gdb/infcmd.c | 35 +++++++++++++---------- gdb/infrun.c | 84 +++++++++++++++++++++++++++++++------------------------ gdb/infrun.h | 4 +-- gdb/mi/mi-main.c | 2 +- gdb/windows-nat.c | 2 +- 8 files changed, 105 insertions(+), 57 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3695bb3..c2a5520 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,36 @@ 2015-03-24 Pedro Alves + * breakpoint.c (until_break_command): Adjust call to proceed. + * gdbthread.h (struct thread_control_state) : + New field. + * infcall.c (run_inferior_call): Adjust call to proceed. + * infcmd.c (run_command_1, proceed_thread_callback, continue_1): + Adjust calls to proceed. + (set_step_frame): Set the current thread's step_start_function + here. + (step_once): Adjust calls to proceed. + (jump_command, signal_command, until_next_command) + (finish_backward, finish_forward, proceed_after_attach_callback) + (attach_command_post_wait): Adjust calls to proceed. + * infrun.c (proceed_after_vfork_done): Adjust call to proceed. + (do_target_resume): New function, factored out from ... + (resume): ... here. Remove 'step' parameter. Instead, check + currently_stepping to determine whether the thread should be + single-stepped. + (proceed): Remove 'step' parameter and don't set the thread's + step_start_function here. Adjust call to 'resume'. + (handle_inferior_event): Adjust calls to 'resume'. + (switch_back_to_stepped_thread): Use do_target_resume instead of + 'resume'. + (keep_going): Adjust calls to 'resume'. + * infrun.h (proceed): Remove 'step' parameter. + (resume): Likewise. + * windows-nat.c (do_initial_windows_stuff): Adjust call to + 'resume'. + * mi/mi-main.c (proceed_thread): Adjust call to 'proceed'. + +2015-03-24 Pedro Alves + * gdbthread.h (struct thread_control_state) : New field. * infcmd.c (step_once): Pass step=1 to clear_proceed_status. Set diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0724a72..12955cb 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -11523,7 +11523,7 @@ until_break_command (char *arg, int from_tty, int anywhere) stack_frame_id, bp_until); make_cleanup_delete_breakpoint (breakpoint); - proceed (-1, GDB_SIGNAL_DEFAULT, 0); + proceed (-1, GDB_SIGNAL_DEFAULT); /* If we are running asynchronously, and proceed call above has actually managed to start the target, arrange for breakpoints to diff --git a/gdb/infcall.c b/gdb/infcall.c index 705e377..26c64f6 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -405,7 +405,7 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc) { int was_sync = sync_execution; - proceed (real_pc, GDB_SIGNAL_0, 0); + proceed (real_pc, GDB_SIGNAL_0); /* Inferior function calls are always synchronous, even if the target supports asynchronous execution. Do here what diff --git a/gdb/infcmd.c b/gdb/infcmd.c index eddbbff..1c70675 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -634,7 +634,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) /* Start the target running. Do not use -1 continuation as it would skip breakpoint right at the entry point. */ - proceed (regcache_read_pc (get_current_regcache ()), GDB_SIGNAL_0, 0); + proceed (regcache_read_pc (get_current_regcache ()), GDB_SIGNAL_0); /* Since there was no error, there's no need to finish the thread states here. */ @@ -687,7 +687,7 @@ proceed_thread_callback (struct thread_info *thread, void *arg) switch_to_thread (thread->ptid); clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); return 0; } @@ -771,7 +771,7 @@ continue_1 (int all_threads) ensure_valid_thread (); ensure_not_running (); clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } } @@ -867,9 +867,14 @@ static void set_step_frame (void) { struct symtab_and_line sal; + CORE_ADDR pc; + struct frame_info *frame = get_current_frame (); + struct thread_info *tp = inferior_thread (); - find_frame_sal (get_current_frame (), &sal); - set_step_info (get_current_frame (), sal); + find_frame_sal (frame, &sal); + set_step_info (frame, sal); + pc = get_frame_pc (frame); + tp->control.step_start_function = find_pc_function (pc); } /* Step until outside of current statement. */ @@ -1122,7 +1127,7 @@ step_once (int skip_subroutines, int single_inst, int count, int thread) tp->step_multi = (count > 1); tp->control.stepping_command = 1; - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 1); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); /* For async targets, register a continuation to do any additional steps. For sync targets, the caller will handle @@ -1229,7 +1234,7 @@ jump_command (char *arg, int from_tty) } clear_proceed_status (0); - proceed (addr, GDB_SIGNAL_0, 0); + proceed (addr, GDB_SIGNAL_0); } @@ -1342,7 +1347,7 @@ signal_command (char *signum_exp, int from_tty) } clear_proceed_status (0); - proceed ((CORE_ADDR) -1, oursig, 0); + proceed ((CORE_ADDR) -1, oursig); } /* Queue a signal to be delivered to the current thread. */ @@ -1462,7 +1467,7 @@ until_next_command (int from_tty) set_longjmp_breakpoint (tp, get_frame_id (frame)); old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 1); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); if (target_can_async_p () && is_running (inferior_ptid)) { @@ -1746,14 +1751,14 @@ finish_backward (struct symbol *function) insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal, null_frame_id); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } else { /* We're almost there -- we just need to back up by one more single-step. */ tp->control.step_range_start = tp->control.step_range_end = 1; - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 1); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } } @@ -1795,7 +1800,7 @@ finish_forward (struct symbol *function, struct frame_info *frame) cargs->function = function; add_continuation (tp, finish_command_continuation, cargs, finish_command_continuation_free_arg); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); discard_cleanups (old_chain); if (!target_can_async_p ()) @@ -1864,7 +1869,7 @@ finish_command (char *arg, int from_tty) print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); } - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 1); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); return; } @@ -2454,7 +2459,7 @@ proceed_after_attach_callback (struct thread_info *thread, { switch_to_thread (thread->ptid); clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } return 0; @@ -2541,7 +2546,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) if (inferior_thread ()->suspend.stop_signal == GDB_SIGNAL_0) { clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } } } diff --git a/gdb/infrun.c b/gdb/infrun.c index a8c3cce..3c2d2fb 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -859,7 +859,7 @@ proceed_after_vfork_done (struct thread_info *thread, switch_to_thread (thread->ptid); clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } return 0; @@ -2032,6 +2032,33 @@ user_visible_resume_ptid (int step) return resume_ptid; } +/* Wrapper for target_resume, that handles infrun-specific + bookkeeping. */ + +static void +do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig) +{ + struct thread_info *tp = inferior_thread (); + + /* Install inferior's terminal modes. */ + target_terminal_inferior (); + + /* Avoid confusing the next resume, if the next stop/resume + happens to apply to another thread. */ + tp->suspend.stop_signal = GDB_SIGNAL_0; + + /* Advise target which signals may be handled silently. If we have + removed breakpoints because we are stepping over one (in any + thread), we need to receive all signals to avoid accidentally + skipping a breakpoint during execution of a signal handler. */ + if (step_over_info_valid_p ()) + target_pass_signals (0, NULL); + else + target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); + + target_resume (resume_ptid, step, sig); +} + /* Resume the inferior, but allow a QUIT. This is useful if the user wants to interrupt some lengthy single-stepping operation (for child processes, the SIGINT goes to the inferior, and so @@ -2040,7 +2067,7 @@ user_visible_resume_ptid (int step) SIG is the signal to give the inferior (zero for none). */ void -resume (int step, enum gdb_signal sig) +resume (enum gdb_signal sig) { struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0); struct regcache *regcache = get_current_regcache (); @@ -2053,6 +2080,11 @@ resume (int step, enum gdb_signal sig) deciding whether "set scheduler-locking step" applies, it's the user's intention that counts. */ const int user_step = tp->control.stepping_command; + /* This represents what we'll actually request the target to do. + This can decay from a step to a continue, if e.g., we need to + implement single-stepping with breakpoints (software + single-step). */ + int step = currently_stepping (tp); tp->stepped_breakpoint = 0; @@ -2355,24 +2387,7 @@ resume (int step, enum gdb_signal sig) gdb_assert (pc_in_thread_step_range (pc, tp)); } - /* Install inferior's terminal modes. */ - target_terminal_inferior (); - - /* Avoid confusing the next resume, if the next stop/resume - happens to apply to another thread. */ - tp->suspend.stop_signal = GDB_SIGNAL_0; - - /* Advise target which signals may be handled silently. If we have - removed breakpoints because we are stepping over one (in any - thread), we need to receive all signals to avoid accidentally - skipping a breakpoint during execution of a signal handler. */ - if (step_over_info_valid_p ()) - target_pass_signals (0, NULL); - else - target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); - - target_resume (resume_ptid, step, sig); - + do_target_resume (resume_ptid, step, sig); discard_cleanups (old_cleanups); } @@ -2551,7 +2566,7 @@ find_thread_needs_step_over (struct thread_info *except) You should call clear_proceed_status before calling proceed. */ void -proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step) +proceed (CORE_ADDR addr, enum gdb_signal siggnal) { struct regcache *regcache; struct gdbarch *gdbarch; @@ -2580,9 +2595,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step) pc = regcache_read_pc (regcache); tp = inferior_thread (); - if (step) - tp->control.step_start_function = find_pc_function (pc); - /* Fill in with reasonable starting values. */ init_thread_stepping_state (tp); @@ -2625,9 +2637,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step) if (debug_infrun) fprintf_unfiltered (gdb_stdlog, - "infrun: proceed (addr=%s, signal=%s, step=%d)\n", + "infrun: proceed (addr=%s, signal=%s)\n", paddress (gdbarch, addr), - gdb_signal_to_symbol_string (siggnal), step); + gdb_signal_to_symbol_string (siggnal)); if (non_stop) /* In non-stop, each thread is handled individually. The context @@ -2713,8 +2725,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step) tp->prev_pc = regcache_read_pc (get_current_regcache ()); /* Resume inferior. */ - resume (tp->control.trap_expected || step || bpstat_should_step (), - tp->suspend.stop_signal); + resume (tp->suspend.stop_signal); /* Wait for it to stop (if not standalone) and in any case decode why it stopped, and act accordingly. */ @@ -3815,7 +3826,7 @@ handle_inferior_event (struct execution_control_state *ecs) addresses. Make sure new breakpoints are inserted. */ if (stop_soon == NO_STOP_QUIETLY) insert_breakpoints (); - resume (0, GDB_SIGNAL_0); + resume (GDB_SIGNAL_0); prepare_to_wait (ecs); return; } @@ -3839,7 +3850,7 @@ handle_inferior_event (struct execution_control_state *ecs) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SPURIOUS\n"); if (!ptid_equal (ecs->ptid, inferior_ptid)) context_switch (ecs->ptid); - resume (0, GDB_SIGNAL_0); + resume (GDB_SIGNAL_0); prepare_to_wait (ecs); return; @@ -5727,6 +5738,8 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) if (stop_pc != tp->prev_pc) { + ptid_t resume_ptid; + if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: expected thread advanced also\n"); @@ -5742,9 +5755,10 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) insert_single_step_breakpoint (get_frame_arch (frame), get_frame_address_space (frame), stop_pc); - ecs->event_thread->control.trap_expected = 1; - resume (0, GDB_SIGNAL_0); + resume_ptid = user_visible_resume_ptid (tp->control.stepping_command); + do_target_resume (resume_ptid, + currently_stepping (tp), GDB_SIGNAL_0); prepare_to_wait (ecs); } else @@ -6191,8 +6205,7 @@ keep_going (struct execution_control_state *ecs) are supposed to pass through to the inferior. Simply continue. */ discard_cleanups (old_cleanups); - resume (currently_stepping (ecs->event_thread), - ecs->event_thread->suspend.stop_signal); + resume (ecs->event_thread->suspend.stop_signal); } else { @@ -6264,8 +6277,7 @@ keep_going (struct execution_control_state *ecs) ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; discard_cleanups (old_cleanups); - resume (currently_stepping (ecs->event_thread), - ecs->event_thread->suspend.stop_signal); + resume (ecs->event_thread->suspend.stop_signal); } prepare_to_wait (ecs); diff --git a/gdb/infrun.h b/gdb/infrun.h index 7301807..ab97eea 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -88,11 +88,11 @@ extern void start_remote (int from_tty); step/stepi command. */ extern void clear_proceed_status (int step); -extern void proceed (CORE_ADDR, enum gdb_signal, int); +extern void proceed (CORE_ADDR, enum gdb_signal); /* The `resume' routine should only be called in special circumstances. Normally, use `proceed', which handles a lot of bookkeeping. */ -extern void resume (int, enum gdb_signal); +extern void resume (enum gdb_signal); /* Return a ptid representing the set of threads that we will proceed, in the perspective of the user/frontend. */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index acbdb55..2733e80 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -252,7 +252,7 @@ proceed_thread (struct thread_info *thread, int pid) switch_to_thread (thread->ptid); clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT, 0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); } static int diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index e5b28c1..fd31083 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -1673,7 +1673,7 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching) wait_for_inferior (); tp = inferior_thread (); if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP) - resume (0, tp->suspend.stop_signal); + resume (tp->suspend.stop_signal); else break; } -- 2.7.4