This is the mail archive of the gdb-patches@sourceware.cygnus.com 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]

RFA: handle signals correctly while stepping over function calls



infrun.c has no maintainer, so this is an open request for code
review.

The patch below is a prerequisite for LinuxThreads support.  Without
it, you can't step over a call to pthread_create, because GDB will
munge the step resume breakpoint when the signals associated with
thread creation start flying around.

The actual thread support patch, coming later, includes a new set of
tests which catches this bug.


1999-06-28  Jim Blandy  <jimb@savonarola.red-bean.com>

	* infrun.c (handle_inferior_event): Don't try to use the code for
	stepping over a function call to also handle stepping out of a
	sigtramp on HP-UX.  That ends up trashing step-resume breakpoints.
	This change reverts some of David Taylor's change of 31 Dec 1998.
	The HP-UX maintainer needs to submit a new change for whatever
	problem the original change was trying to fix.

	Provide more sanity checking:
	* infrun.c (handle_inferior_event): Before assigning a new
	breakpoint to step_resume_breakpoint, make sure it's not already
	pointing at one; if it is, that's a bug.
	(check_for_old_step_resume_breakpoint): New function.

Index: infrun.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/infrun.c,v
retrieving revision 1.222
diff -c -r1.222 infrun.c
*** infrun.c	1999/06/08 10:07:39	1.222
--- infrun.c	1999/06/29 03:45:12
***************
*** 1075,1080 ****
--- 1075,1091 ----
        *breakpointp = NULL;
      }
  }
+ 
+ /* Call this function before setting step_resume_breakpoint, as a
+    sanity check.  We should never be setting a new
+    step_resume_breakpoint when we have an old one active.  */
+ static void
+ check_for_old_step_resume_breakpoint ()
+ {
+   if (step_resume_breakpoint)
+     warning ("GDB bug: infrun.c (wait_for_inferior): dropping old step_resume breakpoint");
+ }
+ 
  
  /* This enum encodes possible reasons for doing a target_wait, so that
     wfi can call target_wait in one place.  (Ultimately the call will be
***************
*** 1122,1127 ****
--- 1133,1141 ----
  
  void handle_inferior_event PARAMS ((struct execution_control_state *ecs));
  
+ 
+ 
+ 
  /* Wait for control to return from inferior to debugger.
     If inferior gets a signal, we may decide to start it up again
     instead of returning.  That is why there is a loop in this function.
***************
*** 2047,2062 ****
  	  if (signal_program[stop_signal] == 0)
  	    stop_signal = TARGET_SIGNAL_0;
  
! 	  /* If we're in the middle of a "next" command, let the code for
!              stepping over a function handle this. pai/1997-09-10
  
!              A previous comment here suggested it was possible to change
!              this to jump to keep_going in all cases. */
  
! 	  if (step_over_calls > 0)
! 	    goto step_over_function;
! 	  else
! 	    goto check_sigtramp2;
  	}
  
        /* Handle cases caused by hitting a breakpoint.  */
--- 2061,2099 ----
  	  if (signal_program[stop_signal] == 0)
  	    stop_signal = TARGET_SIGNAL_0;
  
!           /* I'm not sure whether this needs to be check_sigtramp2 or
!              whether it could/should be keep_going.
  
!              This used to jump to step_over_function if we are stepping,
!              which is wrong.
  
!              Suppose the user does a `next' over a function call, and while
!              that call is in progress, the inferior receives a signal for
!              which GDB does not stop (i.e., signal_stop[SIG] is false).  In
!              that case, when we reach this point, there is already a
!              step-resume breakpoint established, right where it should be:
!              immediately after the function call the user is "next"-ing
!              over.  If we jump to step_over_function now, two bad things
!              happen:
! 
!              - we'll create a new breakpoint, at wherever the current
!                frame's return address happens to be.  That could be
!                anywhere, depending on what function call happens to be on
!                the top of the stack at that point.  Point is, it's probably
!                not where we need it.
! 
!              - the existing step-resume breakpoint (which is at the correct
!                address) will get orphaned: step_resume_breakpoint will point
!                to the new breakpoint, and the old step-resume breakpoint
!                will never be cleaned up.
! 
!              The old behavior was meant to help HP-UX single-step out of
!              sigtramps.  It would place the new breakpoint at prev_pc, which
!              was certainly wrong.  I don't know the details there, so fixing
!              this probably breaks that.  As with anything else, it's up to
!              the HP-UX maintainer to furnish a fix that doesn't break other
!              platforms.  --JimB, 20 May 1999 */
! 	  goto check_sigtramp2;
  	}
  
        /* Handle cases caused by hitting a breakpoint.  */
***************
*** 2417,2422 ****
--- 2454,2460 ----
  		/* We could probably be setting the frame to
                     step_frame_address; I don't think anyone thought to
                     try it.  */
+ 		check_for_old_step_resume_breakpoint ();
  		step_resume_breakpoint =
  		  set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
  		if (breakpoints_inserted)
***************
*** 2499,2504 ****
--- 2537,2543 ----
  		  INIT_SAL (&xxx);	/* initialize to zeroes */
  		  xxx.pc = tmp;
  		  xxx.section = find_pc_overlay (xxx.pc);
+ 		  check_for_old_step_resume_breakpoint ();
  		  step_resume_breakpoint =
  		    set_momentary_breakpoint (xxx, NULL, bp_step_resume);
  		  insert_breakpoints ();
***************
*** 2523,2575 ****
  	step_over_function:
  	  /* A subroutine call has happened.  */
  	  {
! 	    /* Set a special breakpoint after the return */
  	    struct symtab_and_line sr_sal;
  
! 	    INIT_SAL (&sr_sal);
! 	    sr_sal.symtab = NULL;
! 	    sr_sal.line = 0;
! 
! 	    /* If we came here after encountering a signal in the middle of
!                a "next", use the stashed-away previous frame pc */
! 	    sr_sal.pc
! 	      = stopped_by_random_signal
! 	      ? prev_pc
! 	      : ADDR_BITS_REMOVE (SAVED_PC_AFTER_CALL (get_current_frame ()));
  
  	    step_resume_breakpoint =
! 	      set_momentary_breakpoint (sr_sal,
! 					stopped_by_random_signal ?
! 					NULL : get_current_frame (),
  					bp_step_resume);
! 
! 	    /* We've just entered a callee, and we wish to resume until
!                it returns to the caller.  Setting a step_resume bp on
!                the return PC will catch a return from the callee.
! 
!                However, if the callee is recursing, we want to be
!                careful not to catch returns of those recursive calls,
!                but of THIS instance of the call.
! 
!                To do this, we set the step_resume bp's frame to our
!                current caller's frame (step_frame_address, which is
!                set by the "next" or "until" command, before execution
!                begins).
! 
!                But ... don't do it if we're single-stepping out of a
!                sigtramp, because the reason we're single-stepping is
!                precisely because unwinding is a problem (HP-UX 10.20,
!                e.g.) and the frame address is likely to be incorrect.
!                No danger of sigtramp recursion.  */
! 
! 	    if (ecs->stepping_through_sigtramp)
! 	      {
! 		step_resume_breakpoint->frame = (CORE_ADDR) NULL;
! 		ecs->stepping_through_sigtramp = 0;
! 	      }
! 	    else if (!IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
  	      step_resume_breakpoint->frame = step_frame_address;
- 
  	    if (breakpoints_inserted)
  	      insert_breakpoints ();
  	  }
--- 2562,2592 ----
  	step_over_function:
  	  /* A subroutine call has happened.  */
  	  {
! 	    /* We've just entered a callee, and we wish to resume until it
! 	       returns to the caller.  Setting a step_resume breakpoint on
! 	       the return address will catch a return from the callee.
! 
!                However, if the callee is recursing, we want to be careful
!                not to catch returns of those recursive calls, but only of
!                THIS instance of the call.
! 
!                To do this, we set the step_resume bp's frame to our current
!                caller's frame (step_frame_address, which is set by the "next"
!                or "until" command, before execution begins).  */
! 
  	    struct symtab_and_line sr_sal;
  
! 	    INIT_SAL (&sr_sal);		/* initialize to zeroes */
! 	    sr_sal.pc = 
! 	      ADDR_BITS_REMOVE (SAVED_PC_AFTER_CALL (get_current_frame ()));
! 	    sr_sal.section = find_pc_overlay (sr_sal.pc);
  
+ 	    check_for_old_step_resume_breakpoint ();
  	    step_resume_breakpoint =
! 	      set_momentary_breakpoint (sr_sal, get_current_frame (),
  					bp_step_resume);
! 	    if (!IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
  	      step_resume_breakpoint->frame = step_frame_address;
  	    if (breakpoints_inserted)
  	      insert_breakpoints ();
  	  }
***************
*** 2617,2622 ****
--- 2634,2640 ----
  	      /* Do not specify what the fp should be when we stop
  		 since on some machines the prologue
  		 is where the new fp value is established.  */
+ 	      check_for_old_step_resume_breakpoint ();
  	      step_resume_breakpoint =
  		set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
  	      if (breakpoints_inserted)
***************
*** 2661,2666 ****
--- 2679,2685 ----
  	      /* Do not specify what the fp should be when we stop
  		 since on some machines the prologue
  		 is where the new fp value is established.  */
+ 	      check_for_old_step_resume_breakpoint ();
  	      step_resume_breakpoint =
  		set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
  	      if (breakpoints_inserted)

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