Re: [RFA] enable software single step on alpha-osf

[Andrew Cagney <ac131313 at ges dot redhat dot com>]

(Not just a patch involving DECR_PC_AFTER_BREAK but also something that 
involves WFI, outch!)

Can you confirm that the code is encountering a situtation where both 
breakpoints_inserted and singlestep_breakpoints_inserted_p are true.  I 
think this occures when doing a single step after stepping off of a 
breakpoint.  When single stepping off a breakpoint, only 
singlestep_breakpoints_inserted_p would be true.

If this is the case then the comments should make mention of it.  It 
also makes the re-ordered if statement part of the patch correct.

The second part of the change is more tricky:
 +           stop_pc -= DECR_PC_AFTER_BREAK;
Is it fixing any failures?  Software singlestep can be handled in two 
different ways:
- as a breakpoint
- as a hardware single step
and which is prefered decides if/when there should be a decrement.

Anyway, the thing I'm having trouble convincing myself that there can't 
be a double decrement -- eg for a hardware watchpoint or similar.

Andrew

PS: A patch to purge the macro SHIFT_INST_REGS is pre-approved :-)

> Other problems surfaced, however. The SIGSEGVs and the SIGTRAPs
> disappeared, but "next" sometimes stopped at the wrong location.
>
> The following patch enables software single stepping. It also fixes
> all the problems I found when comparing the testsuite results before and
> after the switch. The test results are now identical before and after my
> changes. For the record, here is a summary of the results I get:
>
>         # of expected passes            7246
>         # of unexpected failures        680
>         # of unexpected successes       5
>         # of expected failures          149
>         # of unresolved testcases       59
>         # of untested testcases         3
>         # of unsupported tests          2
>
> I also verified on a linux machine, where software
> single-stepping is not enabled, that no regression was introduced.
>
> It would be interesting to see how this change influences the results
> of alpha-netbsd. It should improve them.
>
> Ok to commit?
>
> 2002-07-18  Joel Brobecker  <brobecker@gnat.com>
>
>         * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
>         procfs appears to be broken when debugging on multi-processor
>         machines. So enable software single stepping in order to avoid
>         using the procfs interface to do next/step operations, using
>         internal breakpoints instead.
>
>         * infrun.c (handle_inferior_event): When receiving a SIGTRAP
>         signal, check whether we hit a breakpoint before checking for a
>         single step breakpoint. Otherwise, GDB fails to notice that a
>         breakpoint has been hit when stepping onto a breakpoint.
>         Readjust the stop_pc by DECR_PC_AFTER_BREAK when hitting a
>         single step breakpoint, to make this pc address equal to the
>         value it would have if the system stepping capability was used.
>
>         * breakpoint.c (bpstat_stop_status): Do not adjust the PC
>         address by DECR_PC_AFTER_BREAK when software single step is
>         in use for this architecture, as this has already been taken
>         care of in handle_inferior_event().
>
> -- Joel
>
>
>
> Index: alpha-osf1-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/alpha-osf1-tdep.c,v
> retrieving revision 1.5
> diff -c -3 -p -r1.5 alpha-osf1-tdep.c
> *** alpha-osf1-tdep.c	21 May 2002 15:36:02 -0000	1.5
> --- alpha-osf1-tdep.c	18 Jul 2002 20:29:34 -0000
> *************** alpha_osf1_init_abi (struct gdbarch_info
> *** 58,63 ****
> --- 58,67 ----
>     struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>   
>     set_gdbarch_pc_in_sigtramp (gdbarch, alpha_osf1_pc_in_sigtramp);
> +   /* The next/step support via procfs on OSF1 is broken when running
> +      on multi-processor machines. We need to use software single stepping
> +      instead.  */
> +   set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
>   
>     tdep->skip_sigtramp_frame = alpha_osf1_skip_sigtramp_frame;
>     tdep->sigcontext_addr = alpha_osf1_sigcontext_addr;
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.63
> diff -c -3 -p -r1.63 infrun.c
> *** infrun.c	18 Jul 2002 17:53:49 -0000	1.63
> --- infrun.c	18 Jul 2002 20:29:36 -0000
> *************** handle_inferior_event (struct execution_
> *** 1826,1835 ****
>   
>     if (stop_signal == TARGET_SIGNAL_TRAP)
>       {
> !       if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
> ! 	ecs->random_signal = 0;
> !       else if (breakpoints_inserted
> ! 	       && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
>   	{
>   	  ecs->random_signal = 0;
>   	  if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
> --- 1826,1836 ----
>   
>     if (stop_signal == TARGET_SIGNAL_TRAP)
>       {
> !       /* Check if a regular breakpoint has been hit before checking
> !          for a potential single step breakpoint. Otherwise, GDB will
> !          not see this breakpoint hit when stepping onto breakpoints.  */
> !       if (breakpoints_inserted
> !           && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
>   	{
>   	  ecs->random_signal = 0;
>   	  if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
> *************** handle_inferior_event (struct execution_
> *** 1885,1890 ****
> --- 1886,1901 ----
>   		}
>   	    }
>   	}
> +       else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
> +         {
> +           /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
> +              compared to the value it would have if the system stepping
> +              capability was used. This allows the rest of the code in
> +              this function to use this address without having to worry
> +              whether software single step is in use or not.  */
> +           stop_pc -= DECR_PC_AFTER_BREAK;
> +           ecs->random_signal = 0;
> +         }
>       }
>     else
>       ecs->random_signal = 1;
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.78
> diff -c -3 -p -r1.78 breakpoint.c
> *** breakpoint.c	26 Jun 2002 05:20:04 -0000	1.78
> --- breakpoint.c	18 Jul 2002 20:29:42 -0000
> *************** bpstat_stop_status (CORE_ADDR *pc, int n
> *** 2429,2436 ****
>        trap event.  For a trace/singlestep trap event, we would
>        not want to subtract DECR_PC_AFTER_BREAK from the PC. */
>   
> !   bp_addr = *pc - (not_a_breakpoint && !SOFTWARE_SINGLE_STEP_P () ? 
> !                    0 : DECR_PC_AFTER_BREAK);
>   
>     ALL_BREAKPOINTS_SAFE (b, temp)
>     {
> --- 2429,2435 ----
>        trap event.  For a trace/singlestep trap event, we would
>        not want to subtract DECR_PC_AFTER_BREAK from the PC. */
>   
> !   bp_addr = *pc - (not_a_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
>   
>     ALL_BREAKPOINTS_SAFE (b, temp)
>     {