This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 4/4] 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-11  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (until_break_command): Adjust call to proceed.
	* gdbthread.h (struct thread_control_state) <stepping_command>:
	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/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 +-
 7 files changed, 74 insertions(+), 57 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 923523e..b7d8da9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11917,7 +11917,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 9ad7480..6e8f4f0 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 42a6046..61d4842 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;
     }
-- 
1.9.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]