[PATCH 0/5] cleanup infrun.c:handle_inferior_event some more.

[PATCH 0/5] cleanup infrun.c:handle_inferior_event some more.

[Pedro Alves]

Here are some more cleanups to handle_inferior_event, while I'm
looking at it.  It ends up breaking handle_inferior_event in two
halves plus one smaller piece.  handle_inferior_event ends up a bit
over half of its current size (~2100 -> ~1200 lines), which I think
everyone will agree is better than the status quo.

Tested on x86_64 Fedora 17.

Pedro Alves (5):
  infrun.c:handle_inferior_event: Remove some more dead code.
  infrun.c:handle_inferior_event: Put all ecs->random_signal tests
    together.
  infrun.c:handle_inferior_event: Move process_event_stop_test goto
    label.
  infrun.c:handle_inferior_event: Make process_event_stop_test label a
    function.
  infrun.c:process_event_stop_test: Reindent.

 gdb/infrun.c | 585 ++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 297 insertions(+), 288 deletions(-)

-- 
1.7.11.7

[PATCH 1/5] infrun.c:handle_inferior_event: Remove some more dead code.

[Pedro Alves]

'ecs' is always memset before being passed to handle_inferior_event.
The stop func is only filled in later in the flow.  And since "Remove
dead sets/clears of ecs->random signal", nothing ever sets
ecs->random_signal before this part is reached either.

(Also tested with some added assertions in place.)

gdb/
2013-10-23  Pedro Alves  <palves@redhat.com>

	* infrun.c (clear_stop_func): Delete.
	(handle_inferior_event): Don't call clear_stop_func and don't
	clear 'ecs->random_signal'.
---
 gdb/infrun.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4f7068c..8b1b668 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3120,17 +3120,6 @@ handle_syscall_event (struct execution_control_state *ecs)
   return 1;
 }
 
-/* Clear the supplied execution_control_state's stop_func_* fields.  */
-
-static void
-clear_stop_func (struct execution_control_state *ecs)
-{
-  ecs->stop_func_filled_in = 0;
-  ecs->stop_func_start = 0;
-  ecs->stop_func_end = 0;
-  ecs->stop_func_name = NULL;
-}
-
 /* Lazily fill in the execution_control_state's stop_func_* fields.  */
 
 static void
@@ -4089,12 +4078,10 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       return;
     }
 
-  clear_stop_func (ecs);
   ecs->event_thread->stepping_over_breakpoint = 0;
   bpstat_clear (&ecs->event_thread->control.stop_bpstat);
   ecs->event_thread->control.stop_step = 0;
   stop_print_frame = 1;
-  ecs->random_signal = 0;
   stopped_by_random_signal = 0;
 
   /* Hide inlined functions starting here, unless we just performed stepi or
-- 
1.7.11.7

[PATCH 4/5] infrun.c:handle_inferior_event: Make process_event_stop_test label a function.

[Pedro Alves]

Now that all ecs->random_signal handing is always done before the
'process_event_stop_test' label, we can easily make that a real
function and actually give it a describing comment that somewhat makes
sense.

Reindenting the new function will be handled in a follow up patch.

2013-10-23  Pedro Alves  <palves@redhat.com>

	* infrun.c (process_event_stop_test): New function, factored out
	from handle_inferior_event.
	(handle_inferior_event): 'process_event_stop_test' is now a
	function instead of a goto label -- adjust.
---
 gdb/infrun.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4f22456..cd98534 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2439,6 +2439,7 @@ static void check_exception_resume (struct execution_control_state *,
 static void stop_stepping (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
+static void process_event_stop_test (struct execution_control_state *ecs);
 static int switch_back_to_stepped_thread (struct execution_control_state *ecs);
 
 /* Callback for iterate over threads.  If the thread is stopped, but
@@ -3151,7 +3152,6 @@ handle_inferior_event (struct execution_control_state *ecs)
   struct gdbarch *gdbarch;
   int stopped_by_watchpoint;
   int stepped_after_stopped_by_watchpoint = 0;
-  struct symtab_and_line stop_pc_sal;
   enum stop_kind stop_soon;
 
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
@@ -3351,7 +3351,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 	    {
 	      /* A catchpoint triggered.  */
 	      ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_TRAP;
-	      goto process_event_stop_test;
+	      process_event_stop_test (ecs);
+	      return;
 	    }
 
 	  /* If requested, stop when the dynamic linker notifies
@@ -3629,7 +3630,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  return;
 	}
       ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_TRAP;
-      goto process_event_stop_test;
+      process_event_stop_test (ecs);
+      return;
 
     case TARGET_WAITKIND_VFORK_DONE:
       /* Done with the shared memory region.  Re-insert breakpoints in
@@ -3690,7 +3692,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  return;
 	}
       ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_TRAP;
-      goto process_event_stop_test;
+      process_event_stop_test (ecs);
+      return;
 
       /* Be careful not to try to gather much state about a thread
          that's in a syscall.  It's frequently a losing proposition.  */
@@ -3699,9 +3702,9 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
         fprintf_unfiltered (gdb_stdlog,
 			    "infrun: TARGET_WAITKIND_SYSCALL_ENTRY\n");
       /* Getting the current syscall number.  */
-      if (handle_syscall_event (ecs) != 0)
-        return;
-      goto process_event_stop_test;
+      if (handle_syscall_event (ecs) == 0)
+	process_event_stop_test (ecs);
+      return;
 
       /* Before examining the threads further, step this thread to
          get it entirely out of the syscall.  (We get notice of the
@@ -3712,9 +3715,9 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog,
 			    "infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
-      if (handle_syscall_event (ecs) != 0)
-        return;
-      goto process_event_stop_test;
+      if (handle_syscall_event (ecs) == 0)
+	process_event_stop_test (ecs);
+      return;
 
     case TARGET_WAITKIND_STOPPED:
       if (debug_infrun)
@@ -4403,17 +4406,29 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	}
       return;
     }
-  else
+
+  process_event_stop_test (ecs);
+}
+
+/* Come here when we've got some debug event / signal we can explain
+   (IOW, not a random signal), and test whether it should cause a
+   stop, or whether we should resume the inferior (transparently).
+   E.g., could be a breakpoint whose condition evaluates false; we
+   could be still stepping within the line; etc.  */
+
+static void
+process_event_stop_test (struct execution_control_state *ecs)
+{
+  struct symtab_and_line stop_pc_sal;
+  struct frame_info *frame;
+  struct gdbarch *gdbarch;
+
     {
       /* Handle cases caused by hitting a breakpoint.  */
 
       CORE_ADDR jmp_buf_pc;
       struct bpstat_what what;
 
-process_event_stop_test:
-
-      /* Re-fetch current thread's frame in case we did a
-	 "goto process_event_stop_test" above.  */
       frame = get_current_frame ();
       gdbarch = get_frame_arch (frame);
 
-- 
1.7.11.7

[PATCH 2/5] infrun.c:handle_inferior_event: Put all ecs->random_signal tests together.

[Pedro Alves]

I recently added a new ecs->random_signal test after the "switch back to
stepped thread" code, and before the stepping tests.  Looking at
making process_event_stop_test a proper function, I realized it'd be
better to keep ecs->random_signal related code together.  To do that,
I needed to factor out the "switch back to stepped thread" code to a new
function, and call it in both the "random signal" and "not random
signal" paths.

gdb/
2013-10-23  Pedro Alves  <palves@redhat.com>

	* infrun.c (switch_back_to_stepped_thread): New function, factored
	out from handle_inferior_event.
	(handle_inferior_event): Adjust to call
	switch_back_to_stepped_thread.  Call it also at the tail of the
	random signal handling, and return, instead of also handling
	random signals just before the stepping tests.
---
 gdb/infrun.c | 169 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 91 insertions(+), 78 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 8b1b668..0da90ec 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2439,6 +2439,7 @@ static void check_exception_resume (struct execution_control_state *,
 static void stop_stepping (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
+static int switch_back_to_stepped_thread (struct execution_control_state *ecs);
 
 /* Callback for iterate over threads.  If the thread is stopped, but
    the user/frontend doesn't know about that yet, go through
@@ -4398,6 +4399,16 @@ process_event_stop_test:
 	 (leaving the inferior at the step-resume-breakpoint without
 	 actually executing it).  Either way continue until the
 	 breakpoint is really hit.  */
+
+      if (!switch_back_to_stepped_thread (ecs))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: random signal, keep going\n");
+
+	  keep_going (ecs);
+	}
+      return;
     }
   else
     {
@@ -4628,84 +4639,8 @@ process_event_stop_test:
 
   /* In all-stop mode, if we're currently stepping but have stopped in
      some other thread, we need to switch back to the stepped thread.  */
-  if (!non_stop)
-    {
-      struct thread_info *tp;
-
-      tp = iterate_over_threads (currently_stepping_or_nexting_callback,
-				 ecs->event_thread);
-      if (tp)
-	{
-	  /* However, if the current thread is blocked on some internal
-	     breakpoint, and we simply need to step over that breakpoint
-	     to get it going again, do that first.  */
-	  if ((ecs->event_thread->control.trap_expected
-	       && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
-	      || ecs->event_thread->stepping_over_breakpoint)
-	    {
-	      keep_going (ecs);
-	      return;
-	    }
-
-	  /* If the stepping thread exited, then don't try to switch
-	     back and resume it, which could fail in several different
-	     ways depending on the target.  Instead, just keep going.
-
-	     We can find a stepping dead thread in the thread list in
-	     two cases:
-
-	     - The target supports thread exit events, and when the
-	     target tries to delete the thread from the thread list,
-	     inferior_ptid pointed at the exiting thread.  In such
-	     case, calling delete_thread does not really remove the
-	     thread from the list; instead, the thread is left listed,
-	     with 'exited' state.
-
-	     - The target's debug interface does not support thread
-	     exit events, and so we have no idea whatsoever if the
-	     previously stepping thread is still alive.  For that
-	     reason, we need to synchronously query the target
-	     now.  */
-	  if (is_exited (tp->ptid)
-	      || !target_thread_alive (tp->ptid))
-	    {
-	      if (debug_infrun)
-		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: not switching back to "
-				    "stepped thread, it has vanished\n");
-
-	      delete_thread (tp->ptid);
-	      keep_going (ecs);
-	      return;
-	    }
-
-	  /* Otherwise, we no longer expect a trap in the current thread.
-	     Clear the trap_expected flag before switching back -- this is
-	     what keep_going would do as well, if we called it.  */
-	  ecs->event_thread->control.trap_expected = 0;
-
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog,
-				"infrun: switching back to stepped thread\n");
-
-	  ecs->event_thread = tp;
-	  ecs->ptid = tp->ptid;
-	  context_switch (ecs->ptid);
-	  keep_going (ecs);
-	  return;
-	}
-    }
-
-  if (ecs->random_signal)
-    {
-      if (debug_infrun)
-	 fprintf_unfiltered (gdb_stdlog,
-			     "infrun: random signal, keep going\n");
-
-      /* Signal not stepping related.  */
-      keep_going (ecs);
-      return;
-    }
+  if (switch_back_to_stepped_thread (ecs))
+    return;
 
   if (ecs->event_thread->control.step_resume_breakpoint)
     {
@@ -5286,6 +5221,84 @@ process_event_stop_test:
   keep_going (ecs);
 }
 
+/* In all-stop mode, if we're currently stepping but have stopped in
+   some other thread, we may need to switch back to the stepped
+   thread.  Returns true we set the inferior running, false if we left
+   it stopped (and the event needs further processing).  */
+
+static int
+switch_back_to_stepped_thread (struct execution_control_state *ecs)
+{
+  if (!non_stop)
+    {
+      struct thread_info *tp;
+
+      tp = iterate_over_threads (currently_stepping_or_nexting_callback,
+				 ecs->event_thread);
+      if (tp)
+	{
+	  /* However, if the current thread is blocked on some internal
+	     breakpoint, and we simply need to step over that breakpoint
+	     to get it going again, do that first.  */
+	  if ((ecs->event_thread->control.trap_expected
+	       && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
+	      || ecs->event_thread->stepping_over_breakpoint)
+	    {
+	      keep_going (ecs);
+	      return 1;
+	    }
+
+	  /* If the stepping thread exited, then don't try to switch
+	     back and resume it, which could fail in several different
+	     ways depending on the target.  Instead, just keep going.
+
+	     We can find a stepping dead thread in the thread list in
+	     two cases:
+
+	     - The target supports thread exit events, and when the
+	     target tries to delete the thread from the thread list,
+	     inferior_ptid pointed at the exiting thread.  In such
+	     case, calling delete_thread does not really remove the
+	     thread from the list; instead, the thread is left listed,
+	     with 'exited' state.
+
+	     - The target's debug interface does not support thread
+	     exit events, and so we have no idea whatsoever if the
+	     previously stepping thread is still alive.  For that
+	     reason, we need to synchronously query the target
+	     now.  */
+	  if (is_exited (tp->ptid)
+	      || !target_thread_alive (tp->ptid))
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: not switching back to "
+				    "stepped thread, it has vanished\n");
+
+	      delete_thread (tp->ptid);
+	      keep_going (ecs);
+	      return 1;
+	    }
+
+	  /* Otherwise, we no longer expect a trap in the current thread.
+	     Clear the trap_expected flag before switching back -- this is
+	     what keep_going would do as well, if we called it.  */
+	  ecs->event_thread->control.trap_expected = 0;
+
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: switching back to stepped thread\n");
+
+	  ecs->event_thread = tp;
+	  ecs->ptid = tp->ptid;
+	  context_switch (ecs->ptid);
+	  keep_going (ecs);
+	  return 1;
+	}
+    }
+  return 0;
+}
+
 /* Is thread TP in the middle of single-stepping?  */
 
 static int
-- 
1.7.11.7

[PATCH 3/5] infrun.c:handle_inferior_event: Move process_event_stop_test goto label.

[Pedro Alves]

We only ever call "goto process_event_stop_test;" right after checking
that ecs->random_signal is clear.  The code at the
process_event_stop_test label looks like:

  /* For the program's own signals, act according to
     the signal handling tables.  */

  if (ecs->random_signal)
    {
     ... random signal handling ...
     return;
    }
  else
    {
     ... the stop tests that actually matter for the goto callers.
    }

So this moves the label into the else branch.  It'll make converting
process_event_stop_test into a function a bit clearer.

gdb/
2013-10-23  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_inferior_event): Move process_event_stop_test
	goto label to the else branch of the ecs->random_signal check,
	along with FRAME and GDBARCH re-fetching.
---
 gdb/infrun.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0da90ec..4f22456 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4288,13 +4288,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_TRAP;
     }
 
-process_event_stop_test:
-
-  /* Re-fetch current thread's frame in case we did a
-     "goto process_event_stop_test" above.  */
-  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
-
   /* For the program's own signals, act according to
      the signal handling tables.  */
 
@@ -4417,6 +4410,13 @@ process_event_stop_test:
       CORE_ADDR jmp_buf_pc;
       struct bpstat_what what;
 
+process_event_stop_test:
+
+      /* Re-fetch current thread's frame in case we did a
+	 "goto process_event_stop_test" above.  */
+      frame = get_current_frame ();
+      gdbarch = get_frame_arch (frame);
+
       what = bpstat_what (ecs->event_thread->control.stop_bpstat);
 
       if (what.call_dummy)
-- 
1.7.11.7

[PATCH 5/5] infrun.c:process_event_stop_test: Reindent.

[Pedro Alves]

gdb/
2013-10-23  Pedro Alves  <palves@redhat.com>

	* infrun.c (process_event_stop_test): Remove unnecessary scoping
	level and reindent.
---
 gdb/infrun.c | 368 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 181 insertions(+), 187 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index cd98534..17612d7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4422,235 +4422,229 @@ process_event_stop_test (struct execution_control_state *ecs)
   struct symtab_and_line stop_pc_sal;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
+  CORE_ADDR jmp_buf_pc;
+  struct bpstat_what what;
 
-    {
-      /* Handle cases caused by hitting a breakpoint.  */
-
-      CORE_ADDR jmp_buf_pc;
-      struct bpstat_what what;
+  /* Handle cases caused by hitting a breakpoint.  */
 
-      frame = get_current_frame ();
-      gdbarch = get_frame_arch (frame);
+  frame = get_current_frame ();
+  gdbarch = get_frame_arch (frame);
 
-      what = bpstat_what (ecs->event_thread->control.stop_bpstat);
+  what = bpstat_what (ecs->event_thread->control.stop_bpstat);
 
-      if (what.call_dummy)
-	{
-	  stop_stack_dummy = what.call_dummy;
-	}
+  if (what.call_dummy)
+    {
+      stop_stack_dummy = what.call_dummy;
+    }
 
-      /* If we hit an internal event that triggers symbol changes, the
-	 current frame will be invalidated within bpstat_what (e.g.,
-	 if we hit an internal solib event).  Re-fetch it.  */
-      frame = get_current_frame ();
-      gdbarch = get_frame_arch (frame);
+  /* If we hit an internal event that triggers symbol changes, the
+     current frame will be invalidated within bpstat_what (e.g., if we
+     hit an internal solib event).  Re-fetch it.  */
+  frame = get_current_frame ();
+  gdbarch = get_frame_arch (frame);
 
-      switch (what.main_action)
-	{
-	case BPSTAT_WHAT_SET_LONGJMP_RESUME:
-	  /* If we hit the breakpoint at longjmp while stepping, we
-	     install a momentary breakpoint at the target of the
-	     jmp_buf.  */
+  switch (what.main_action)
+    {
+    case BPSTAT_WHAT_SET_LONGJMP_RESUME:
+      /* If we hit the breakpoint at longjmp while stepping, we
+	 install a momentary breakpoint at the target of the
+	 jmp_buf.  */
 
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog,
-				"infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
 
-	  ecs->event_thread->stepping_over_breakpoint = 1;
+      ecs->event_thread->stepping_over_breakpoint = 1;
 
-	  if (what.is_longjmp)
+      if (what.is_longjmp)
+	{
+	  struct value *arg_value;
+
+	  /* If we set the longjmp breakpoint via a SystemTap probe,
+	     then use it to extract the arguments.  The destination PC
+	     is the third argument to the probe.  */
+	  arg_value = probe_safe_evaluate_at_pc (frame, 2);
+	  if (arg_value)
+	    jmp_buf_pc = value_as_address (arg_value);
+	  else if (!gdbarch_get_longjmp_target_p (gdbarch)
+		   || !gdbarch_get_longjmp_target (gdbarch,
+						   frame, &jmp_buf_pc))
 	    {
-	      struct value *arg_value;
-
-	      /* If we set the longjmp breakpoint via a SystemTap
-		 probe, then use it to extract the arguments.  The
-		 destination PC is the third argument to the
-		 probe.  */
-	      arg_value = probe_safe_evaluate_at_pc (frame, 2);
-	      if (arg_value)
-		jmp_buf_pc = value_as_address (arg_value);
-	      else if (!gdbarch_get_longjmp_target_p (gdbarch)
-		       || !gdbarch_get_longjmp_target (gdbarch,
-						       frame, &jmp_buf_pc))
-		{
-		  if (debug_infrun)
-		    fprintf_unfiltered (gdb_stdlog,
-					"infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME "
-					"(!gdbarch_get_longjmp_target)\n");
-		  keep_going (ecs);
-		  return;
-		}
-
-	      /* Insert a breakpoint at resume address.  */
-	      insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME "
+				    "(!gdbarch_get_longjmp_target)\n");
+	      keep_going (ecs);
+	      return;
 	    }
-	  else
-	    check_exception_resume (ecs, frame);
-	  keep_going (ecs);
-	  return;
-
-	case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
-	  {
-	    struct frame_info *init_frame;
 
-	    /* There are several cases to consider.
+	  /* Insert a breakpoint at resume address.  */
+	  insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
+	}
+      else
+	check_exception_resume (ecs, frame);
+      keep_going (ecs);
+      return;
 
-	       1. The initiating frame no longer exists.  In this case
-	       we must stop, because the exception or longjmp has gone
-	       too far.
+    case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
+      {
+	struct frame_info *init_frame;
 
-	       2. The initiating frame exists, and is the same as the
-	       current frame.  We stop, because the exception or
-	       longjmp has been caught.
+	/* There are several cases to consider.
 
-	       3. The initiating frame exists and is different from
-	       the current frame.  This means the exception or longjmp
-	       has been caught beneath the initiating frame, so keep
-	       going.
+	   1. The initiating frame no longer exists.  In this case we
+	   must stop, because the exception or longjmp has gone too
+	   far.
 
-	       4. longjmp breakpoint has been placed just to protect
-	       against stale dummy frames and user is not interested
-	       in stopping around longjmps.  */
+	   2. The initiating frame exists, and is the same as the
+	   current frame.  We stop, because the exception or longjmp
+	   has been caught.
 
-	    if (debug_infrun)
-	      fprintf_unfiltered (gdb_stdlog,
-				  "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+	   3. The initiating frame exists and is different from the
+	   current frame.  This means the exception or longjmp has
+	   been caught beneath the initiating frame, so keep going.
 
-	    gdb_assert (ecs->event_thread->control.exception_resume_breakpoint
-			!= NULL);
-	    delete_exception_resume_breakpoint (ecs->event_thread);
+	   4. longjmp breakpoint has been placed just to protect
+	   against stale dummy frames and user is not interested in
+	   stopping around longjmps.  */
 
-	    if (what.is_longjmp)
-	      {
-		check_longjmp_breakpoint_for_call_dummy (ecs->event_thread->num);
+	if (debug_infrun)
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
 
-		if (!frame_id_p (ecs->event_thread->initiating_frame))
-		  {
-		    /* Case 4.  */
-		    keep_going (ecs);
-		    return;
-		  }
-	      }
+	gdb_assert (ecs->event_thread->control.exception_resume_breakpoint
+		    != NULL);
+	delete_exception_resume_breakpoint (ecs->event_thread);
 
-	    init_frame = frame_find_by_id (ecs->event_thread->initiating_frame);
+	if (what.is_longjmp)
+	  {
+	    check_longjmp_breakpoint_for_call_dummy (ecs->event_thread->num);
 
-	    if (init_frame)
+	    if (!frame_id_p (ecs->event_thread->initiating_frame))
 	      {
-		struct frame_id current_id
-		  = get_frame_id (get_current_frame ());
-		if (frame_id_eq (current_id,
-				 ecs->event_thread->initiating_frame))
-		  {
-		    /* Case 2.  Fall through.  */
-		  }
-		else
-		  {
-		    /* Case 3.  */
-		    keep_going (ecs);
-		    return;
-		  }
+		/* Case 4.  */
+		keep_going (ecs);
+		return;
 	      }
+	  }
 
-	    /* For Cases 1 and 2, remove the step-resume breakpoint,
-	       if it exists.  */
-	    delete_step_resume_breakpoint (ecs->event_thread);
+	init_frame = frame_find_by_id (ecs->event_thread->initiating_frame);
 
-	    ecs->event_thread->control.stop_step = 1;
-	    print_end_stepping_range_reason ();
-	    stop_stepping (ecs);
+	if (init_frame)
+	  {
+	    struct frame_id current_id
+	      = get_frame_id (get_current_frame ());
+	    if (frame_id_eq (current_id,
+			     ecs->event_thread->initiating_frame))
+	      {
+		/* Case 2.  Fall through.  */
+	      }
+	    else
+	      {
+		/* Case 3.  */
+		keep_going (ecs);
+		return;
+	      }
 	  }
-	  return;
-
-	case BPSTAT_WHAT_SINGLE:
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
-	  ecs->event_thread->stepping_over_breakpoint = 1;
-	  /* Still need to check other stuff, at least the case where
-	     we are stepping and step out of the right range.  */
-	  break;
 
-	case BPSTAT_WHAT_STEP_RESUME:
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
+	/* For Cases 1 and 2, remove the step-resume breakpoint, if it
+	   exists.  */
+	delete_step_resume_breakpoint (ecs->event_thread);
 
-	  delete_step_resume_breakpoint (ecs->event_thread);
-	  if (ecs->event_thread->control.proceed_to_finish
-	      && execution_direction == EXEC_REVERSE)
-	    {
-	      struct thread_info *tp = ecs->event_thread;
-
-	      /* We are finishing a function in reverse, and just hit
-		 the step-resume breakpoint at the start address of
-		 the function, and we're almost there -- just need to
-		 back up by one more single-step, which should take us
-		 back to the function call.  */
-	      tp->control.step_range_start = tp->control.step_range_end = 1;
-	      keep_going (ecs);
-	      return;
-	    }
-	  fill_in_stop_func (gdbarch, ecs);
-	  if (stop_pc == ecs->stop_func_start
-	      && execution_direction == EXEC_REVERSE)
-	    {
-	      /* We are stepping over a function call in reverse, and
-		 just hit the step-resume breakpoint at the start
-		 address of the function.  Go back to single-stepping,
-		 which should take us back to the function call.  */
-	      ecs->event_thread->stepping_over_breakpoint = 1;
-	      keep_going (ecs);
-	      return;
-	    }
-	  break;
+	ecs->event_thread->control.stop_step = 1;
+	print_end_stepping_range_reason ();
+	stop_stepping (ecs);
+      }
+      return;
 
-	case BPSTAT_WHAT_STOP_NOISY:
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
-	  stop_print_frame = 1;
+    case BPSTAT_WHAT_SINGLE:
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
+      ecs->event_thread->stepping_over_breakpoint = 1;
+      /* Still need to check other stuff, at least the case where we
+	 are stepping and step out of the right range.  */
+      break;
 
-	  /* We are about to nuke the step_resume_breakpointt via the
-	     cleanup chain, so no need to worry about it here.  */
+    case BPSTAT_WHAT_STEP_RESUME:
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
 
-	  stop_stepping (ecs);
+      delete_step_resume_breakpoint (ecs->event_thread);
+      if (ecs->event_thread->control.proceed_to_finish
+	  && execution_direction == EXEC_REVERSE)
+	{
+	  struct thread_info *tp = ecs->event_thread;
+
+	  /* We are finishing a function in reverse, and just hit the
+	     step-resume breakpoint at the start address of the
+	     function, and we're almost there -- just need to back up
+	     by one more single-step, which should take us back to the
+	     function call.  */
+	  tp->control.step_range_start = tp->control.step_range_end = 1;
+	  keep_going (ecs);
 	  return;
+	}
+      fill_in_stop_func (gdbarch, ecs);
+      if (stop_pc == ecs->stop_func_start
+	  && execution_direction == EXEC_REVERSE)
+	{
+	  /* We are stepping over a function call in reverse, and just
+	     hit the step-resume breakpoint at the start address of
+	     the function.  Go back to single-stepping, which should
+	     take us back to the function call.  */
+	  ecs->event_thread->stepping_over_breakpoint = 1;
+	  keep_going (ecs);
+	  return;
+	}
+      break;
 
-	case BPSTAT_WHAT_STOP_SILENT:
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
-	  stop_print_frame = 0;
+    case BPSTAT_WHAT_STOP_NOISY:
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
+      stop_print_frame = 1;
 
-	  /* We are about to nuke the step_resume_breakpoin via the
-	     cleanup chain, so no need to worry about it here.  */
+      /* We are about to nuke the step_resume_breakpointt via the
+	 cleanup chain, so no need to worry about it here.  */
 
-	  stop_stepping (ecs);
-	  return;
+      stop_stepping (ecs);
+      return;
 
-	case BPSTAT_WHAT_HP_STEP_RESUME:
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_HP_STEP_RESUME\n");
+    case BPSTAT_WHAT_STOP_SILENT:
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
+      stop_print_frame = 0;
 
-	  delete_step_resume_breakpoint (ecs->event_thread);
-	  if (ecs->event_thread->step_after_step_resume_breakpoint)
-	    {
-	      /* Back when the step-resume breakpoint was inserted, we
-		 were trying to single-step off a breakpoint.  Go back
-		 to doing that.  */
-	      ecs->event_thread->step_after_step_resume_breakpoint = 0;
-	      ecs->event_thread->stepping_over_breakpoint = 1;
-	      keep_going (ecs);
-	      return;
-	    }
-	  break;
+      /* We are about to nuke the step_resume_breakpoin via the
+	 cleanup chain, so no need to worry about it here.  */
 
-	case BPSTAT_WHAT_KEEP_CHECKING:
-	  break;
+      stop_stepping (ecs);
+      return;
+
+    case BPSTAT_WHAT_HP_STEP_RESUME:
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_HP_STEP_RESUME\n");
+
+      delete_step_resume_breakpoint (ecs->event_thread);
+      if (ecs->event_thread->step_after_step_resume_breakpoint)
+	{
+	  /* Back when the step-resume breakpoint was inserted, we
+	     were trying to single-step off a breakpoint.  Go back to
+	     doing that.  */
+	  ecs->event_thread->step_after_step_resume_breakpoint = 0;
+	  ecs->event_thread->stepping_over_breakpoint = 1;
+	  keep_going (ecs);
+	  return;
 	}
+      break;
+
+    case BPSTAT_WHAT_KEEP_CHECKING:
+      break;
     }
 
-  /* We come here if we hit a breakpoint but should not
-     stop for it.  Possibly we also were stepping
-     and should stop for that.  So fall through and
-     test for stepping.  But, if not stepping,
-     do not stop.  */
+  /* We come here if we hit a breakpoint but should not stop for it.
+     Possibly we also were stepping and should stop for that.  So fall
+     through and test for stepping.  But, if not stepping, do not
+     stop.  */
 
   /* In all-stop mode, if we're currently stepping but have stopped in
      some other thread, we need to switch back to the stepped thread.  */
-- 
1.7.11.7

Re: [PATCH 1/5] infrun.c:handle_inferior_event: Remove some more dead code.

[Gary Benson]

Pedro Alves wrote:
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 4f7068c..8b1b668 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3120,17 +3120,6 @@ handle_syscall_event (struct execution_control_state *ecs)
>    return 1;
>  }
>  
> -/* Clear the supplied execution_control_state's stop_func_* fields.  */
> -
> -static void
> -clear_stop_func (struct execution_control_state *ecs)
> -{
> -  ecs->stop_func_filled_in = 0;
> -  ecs->stop_func_start = 0;
> -  ecs->stop_func_end = 0;
> -  ecs->stop_func_name = NULL;
> -}
> -

I added this one a couple of years ago:

  https://sourceware.org/ml/gdb-patches/2011-07/msg00460.html

I think the reason I put that stuff in a separate function was
as a partner function to fill_in_stop_func introduced in the
same commit.  I didn't think to check whether the clears were
necessary, so good catch :)

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/5] cleanup infrun.c:handle_inferior_event some more.

[Pedro Alves]

I pushed this.

-- 
Pedro Alves