[RFC PATCH] normalize "thread hop" handling

Michael Snyder msnyder@cygnus.com
Mon Jun 11 18:43:00 GMT 2001


This patch normalizes the infrun handling of the "thread hop" event, 
when the wrong thread hits a thread-specific breakpoint and we need to
singlestep that thread (only) before re-inserting breakpoints.

The big benefit of this patch is that when we get the subsequent
singlestep-trap, we process it normally in handle_inferior_event
instead of just calling resume.  This prevents bad things like
failing to stop if the step takes us out of a stepping range.

If nobody yells, I'll check it in in a few days.

Michael
2001-06-11  Michael Snyder  <msnyder@redhat.com>

	* infrun.c (context_switch): New function.  Abstract the operation
	of saving and restoring infrun's state when switching threads.
	(handle_inferior_event): Normalize the handling of the 'thread hop'
	event (when the wrong thread hits a thread-specific breakpoint, 
	and we need to solo-step that thread past the breakpoint).
	Call keep_going, instead of target_resume.  Handle the subsequent
	singlestep-trap as a normal event instead of just resuming.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.35
diff -c -3 -p -r1.35 infrun.c
*** infrun.c	2001/06/02 00:36:20	1.35
--- infrun.c	2001/06/12 01:34:46
*************** get_last_target_status(ptid_t *ptidp, st
*** 1433,1438 ****
--- 1433,1477 ----
    *status = target_last_waitstatus;
  }
  
+ /* Switch thread contexts, maintaining "infrun state". */
+ 
+ static void
+ context_switch (struct execution_control_state *ecs)
+ {
+   /* Caution: it may happen that the new thread (or the old one!)
+      is not in the thread list.  In this case we must not attempt
+      to "switch context", or we run the risk that our context may
+      be lost.  This may happen as a result of the target module
+      mishandling thread creation.  */
+ 
+   if (in_thread_list (inferior_ptid) && in_thread_list (ecs->ptid))
+     { /* Perform infrun state context switch: */
+       /* Save infrun state for the old thread.  */
+       save_infrun_state (inferior_ptid, prev_pc, 
+ 			 prev_func_start, prev_func_name, 
+ 			 trap_expected, step_resume_breakpoint,
+ 			 through_sigtramp_breakpoint, step_range_start, 
+ 			 step_range_end, step_frame_address, 
+ 			 ecs->handling_longjmp, ecs->another_trap,
+ 			 ecs->stepping_through_solib_after_catch,
+ 			 ecs->stepping_through_solib_catchpoints,
+ 			 ecs->stepping_through_sigtramp);
+ 
+       /* Load infrun state for the new thread.  */
+       load_infrun_state (ecs->ptid, &prev_pc, 
+ 			 &prev_func_start, &prev_func_name, 
+ 			 &trap_expected, &step_resume_breakpoint,
+ 			 &through_sigtramp_breakpoint, &step_range_start, 
+ 			 &step_range_end, &step_frame_address, 
+ 			 &ecs->handling_longjmp, &ecs->another_trap,
+ 			 &ecs->stepping_through_solib_after_catch,
+ 			 &ecs->stepping_through_solib_catchpoints,
+ 			 &ecs->stepping_through_sigtramp);
+     }
+   inferior_ptid = ecs->ptid;
+ }
+ 
+ 
  /* Given an execution control state that has been freshly filled in
     by an event from the inferior, figure out what it means and take
     appropriate action.  */
*************** handle_inferior_event (struct execution_
*** 1451,1456 ****
--- 1490,1500 ----
    {
      switch (ecs->infwait_state)
        {
+       case infwait_thread_hop_state:
+ 	/* Cancel the waiton_ptid. */
+ 	ecs->waiton_ptid = pid_to_ptid (-1);
+ 	/* Fall thru to the normal_state case. */
+ 
        case infwait_normal_state:
  	/* Since we've done a wait, we have a new event.  Don't
  	   carry over any expectations about needing to step over a
*************** handle_inferior_event (struct execution_
*** 1467,1491 ****
  	stepped_after_stopped_by_watchpoint = 0;
  	break;
  
-       case infwait_thread_hop_state:
- 	insert_breakpoints ();
- 
- 	/* We need to restart all the threads now,
- 	 * unless we're running in scheduler-locked mode. 
- 	 * Use currently_stepping to determine whether to 
- 	 * step or continue.
- 	 */
- 
- 	if (scheduler_mode == schedlock_on)
- 	  target_resume (ecs->ptid, 
- 			 currently_stepping (ecs), TARGET_SIGNAL_0);
- 	else
- 	  target_resume (RESUME_ALL, 
- 			 currently_stepping (ecs), TARGET_SIGNAL_0);
- 	ecs->infwait_state = infwait_normal_state;
- 	prepare_to_wait (ecs);
- 	return;
- 
        case infwait_nullified_state:
  	break;
  
--- 1511,1516 ----
*************** handle_inferior_event (struct execution_
*** 1888,1893 ****
--- 1913,1919 ----
  		     * Use currently_stepping to determine whether to 
  		     * step or continue.
  		     */
+ 		    /* FIXME MVS: is there any reason not to call resume()? */
  		    if (scheduler_mode == schedlock_on)
  		      target_resume (ecs->ptid, 
  				     currently_stepping (ecs), 
*************** handle_inferior_event (struct execution_
*** 1901,1914 ****
  		  }
  		else
  		  {		/* Single step */
! 		    target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
! 		    /* FIXME: What if a signal arrives instead of the
! 		       single-step happening?  */
! 
  		    ecs->waiton_ptid = ecs->ptid;
  		    ecs->wp = &(ecs->ws);
  		    ecs->infwait_state = infwait_thread_hop_state;
! 		    prepare_to_wait (ecs);
  		    return;
  		  }
  	      }
--- 1927,1949 ----
  		  }
  		else
  		  {		/* Single step */
! 		    breakpoints_inserted = 0;
! 		    if (!ptid_equal (inferior_ptid, ecs->ptid))
! 		      context_switch (ecs);
  		    ecs->waiton_ptid = ecs->ptid;
  		    ecs->wp = &(ecs->ws);
+ 		    thread_step_needed = 1;
+ 		    ecs->another_trap = 1;
+ 
+ 		    /* keep_stepping will call resume, and the
+ 		       combination of "thread_step_needed" and
+ 		       "ecs->another_trap" will cause resume to
+ 		       solo-step the thread.  The subsequent trap
+ 		       event will be handled like any other singlestep
+ 		       event. */
+ 
  		    ecs->infwait_state = infwait_thread_hop_state;
! 		    keep_going (ecs);
  		    return;
  		  }
  	      }
*************** handle_inferior_event (struct execution_
*** 1983,2022 ****
  	/* It's a SIGTRAP or a signal we're interested in.  Switch threads,
  	   and fall into the rest of wait_for_inferior().  */
  
! 	/* Caution: it may happen that the new thread (or the old one!)
! 	   is not in the thread list.  In this case we must not attempt
! 	   to "switch context", or we run the risk that our context may
! 	   be lost.  This may happen as a result of the target module
! 	   mishandling thread creation.  */
! 
! 	if (in_thread_list (inferior_ptid) && in_thread_list (ecs->ptid))
! 	  { /* Perform infrun state context switch: */
! 	    /* Save infrun state for the old thread.  */
! 	    save_infrun_state (inferior_ptid, prev_pc,
! 			       prev_func_start, prev_func_name,
! 			       trap_expected, step_resume_breakpoint,
! 			       through_sigtramp_breakpoint,
! 			       step_range_start, step_range_end,
! 			       step_frame_address, ecs->handling_longjmp,
! 			       ecs->another_trap,
! 			       ecs->stepping_through_solib_after_catch,
! 			       ecs->stepping_through_solib_catchpoints,
! 			       ecs->stepping_through_sigtramp);
! 
! 	    /* Load infrun state for the new thread.  */
! 	    load_infrun_state (ecs->ptid, &prev_pc,
! 			       &prev_func_start, &prev_func_name,
! 			       &trap_expected, &step_resume_breakpoint,
! 			       &through_sigtramp_breakpoint,
! 			       &step_range_start, &step_range_end,
! 			       &step_frame_address, &ecs->handling_longjmp,
! 			       &ecs->another_trap,
! 			       &ecs->stepping_through_solib_after_catch,
! 			       &ecs->stepping_through_solib_catchpoints,
! 			       &ecs->stepping_through_sigtramp);
! 	  }
! 
! 	inferior_ptid = ecs->ptid;
  
  	if (context_hook)
  	  context_hook (pid_to_thread_id (ecs->ptid));
--- 2018,2024 ----
  	/* It's a SIGTRAP or a signal we're interested in.  Switch threads,
  	   and fall into the rest of wait_for_inferior().  */
  
! 	context_switch (ecs);
  
  	if (context_hook)
  	  context_hook (pid_to_thread_id (ecs->ptid));


More information about the Gdb-patches mailing list