Re: [RFC] if (INNER_THAN (read_sp(), step_sp - 16))

["Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de>]

Yes, that fixes it for Linux, which is ptrace based, but the problem persists
on Solaris x86, which is procfs based.

From procfs.c:

static int
register_gdb_signals (procinfo *pi, gdb_sigset_t *signals)
{
  int signo;

  for (signo = 0; signo < NSIG; signo ++)
    if (signal_stop_state  (target_signal_from_host (signo)) == 0 &&
        signal_print_state (target_signal_from_host (signo)) == 0 &&
        signal_pass_state  (target_signal_from_host (signo)) == 1)
      prdelset (signals, signo);
    else
      praddset (signals, signo);

  return proc_set_traced_signals (pi, signals);
}

This is a good thing, because it avoids unnecessary program stops, but as
a consequence there is no intervening stop in the signal trampoline, and
stepping_with_random_signal never gets set for the SIGALRM signal on procfs
based systems.

As I said, I think this can only be solved by looking at the new stack
pointer value, which should be way below the previous stack pointer value,
due to the pushed signal context.

Well, another solution might be to set the traced signal set unconditionally
to all signals before taking a step, but that might slow down things
considerably.

> This is a multi-part message in MIME format.
> --------------28602BB2E928B6CED30B543F
> Content-Type: text/plain; charset=us-ascii
> Content-Transfer-Encoding: 7bit
> 
> "Peter.Schauer" wrote:
> > 
> > Hellooooooo.
> > 
> > This ugly hack is coming back to haunt me every other year, and if I'd
> > know about a cleaner way to solve this, I'd already have done it long ago.
> 
> OK, would you review the attached change please?
> It's meant to replace the (step_sp - 16) code by
> using a state variable.  I set the state variable
> when we resume the target with a random signal while
> singlestepping, and I clear it if we enter a trampoline.
> This passes signals.exp, and does not create any new failures
> on i386-linux.
> 
> Michael
> --------------28602BB2E928B6CED30B543F
> Content-Type: text/plain; charset=us-ascii;
>  name="step_sp.diff"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline;
>  filename="step_sp.diff"
> 
> 2001-06-27  Michael Snyder  <msnyder@redhat.com>
> 
> 	* infrun.c (struct execution_control_state): Add new field, 
> 	stepping_with_random_signal.
> 	(init_execution_control_state): Initialize new field.
> 	(handle_inferior_event): If stepping and passing a random
> 	signal to the inferior, set 'stepping_with_random_signal'.
> 	Use this state variable to detect hitting a breakpoint in
> 	a signal handler without an intervening stop in sigtramp.
> 	(check_sigtramp2): Clear stepping_with_random_signal if we
> 	enter a signal trampoline.
> 	(prepare_to_wait): Clear stepping_with_random_signal.
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.39
> diff -c -3 -p -r1.39 infrun.c
> *** infrun.c	2001/06/26 00:26:42	1.39
> --- infrun.c	2001/06/27 21:55:10
> *************** struct execution_control_state
> *** 1200,1205 ****
> --- 1200,1206 ----
>       enum infwait_states infwait_state;
>       ptid_t waiton_ptid;
>       int wait_some_more;
> +     int stepping_with_random_signal;
>     };
>   
>   void init_execution_control_state (struct execution_control_state * ecs);
> *************** fetch_inferior_event (void *client_data)
> *** 1339,1344 ****
> --- 1340,1347 ----
>   void
>   init_execution_control_state (struct execution_control_state *ecs)
>   {
> +   /* FIXME: memset?  We wouldn't have to keep adding inits for new fields. */
> +   ecs->stepping_with_random_signal = 0;
>     /* ecs->another_trap? */
>     ecs->random_signal = 0;
>     ecs->remove_breakpoints_on_following_step = 0;
> *************** handle_inferior_event (struct execution_
> *** 2097,2114 ****
>   	else
>   	  {
>   	    /* See if there is a breakpoint at the current PC.  */
> ! 	    stop_bpstat = bpstat_stop_status
>   	      (&stop_pc,
> ! 	    /* Pass TRUE if our reason for stopping is something other
> ! 	       than hitting a breakpoint.  We do this by checking that
> ! 	       1) stepping is going on and 2) we didn't hit a breakpoint
> ! 	       in a signal handler without an intervening stop in
> ! 	       sigtramp, which is detected by a new stack pointer value
> ! 	       below any usual function calling stack adjustments.  */
> ! 		(currently_stepping (ecs)
> ! 		 && !(step_range_end
> ! 		      && INNER_THAN (read_sp (), (step_sp - 16))))
> ! 	      );
>   	    /* Following in case break condition called a
>   	       function.  */
>   	    stop_print_frame = 1;
> --- 2100,2114 ----
>   	else
>   	  {
>   	    /* See if there is a breakpoint at the current PC.  */
> ! 	    stop_bpstat = bpstat_stop_status 
>   	      (&stop_pc,
> ! 	       /* Pass TRUE if our reason for stopping is something other
> ! 		  than hitting a breakpoint.  We do this by checking that
> ! 		  1) stepping is going on and 2) we didn't hit a breakpoint
> ! 		  in a signal handler without an intervening stop in
> ! 		  sigtramp, which is detected by a state variable.  */
> ! 	       currently_stepping (ecs)
> ! 	       && !ecs->stepping_with_random_signal);
>   	    /* Following in case break condition called a
>   	       function.  */
>   	    stop_print_frame = 1;
> *************** handle_inferior_event (struct execution_
> *** 2257,2262 ****
> --- 2257,2266 ----
>              platforms.  --JimB, 20 May 1999 */
>   	check_sigtramp2 (ecs);
>   	keep_going (ecs);
> + 	/* Remember if we are stepping with a random signal. */
> + 	if (currently_stepping (ecs) &&
> + 	    signal_stop[stop_signal] == 0)
> + 	  ecs->stepping_with_random_signal = 1;
>   	return;
>         }
>   
> *************** check_sigtramp2 (struct execution_contro
> *** 2965,2970 ****
> --- 2969,2975 ----
>   
>         ecs->remove_breakpoints_on_following_step = 1;
>         ecs->another_trap = 1;
> +       ecs->stepping_with_random_signal = 0;
>       }
>   }
>   
> *************** prepare_to_wait (struct execution_contro
> *** 3232,3237 ****
> --- 3237,3243 ----
>         registers_changed ();
>         ecs->waiton_ptid = pid_to_ptid (-1);
>         ecs->wp = &(ecs->ws);
> +       ecs->stepping_with_random_signal = 0;
>       }
>     /* This is the old end of the while loop.  Let everybody know we
>        want to wait for the inferior some more and get called again
> 
> --------------28602BB2E928B6CED30B543F--
> 
> 
> 


-- 
Peter Schauer			pes@regent.e-technik.tu-muenchen.de