[rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Yao Qi]

From: Yao Qi <yao.qi@linaro.org>

Nowadays, in infrun.c:resume, the setting to 'step' variable is like:

  if (use_displaced_stepping (gdbarch)
      && tp->control.trap_expected
      && sig == GDB_SIGNAL_0
      && !current_inferior ()->waiting_for_vfork_done)
    {
    }
  /* Do we need to do it the hard way, w/temp breakpoints?  */
  else if (step)
    step = maybe_software_singlestep (gdbarch, pc); <-- [1]

  ...

  if (execution_direction != EXEC_REVERSE
      && step && breakpoint_inserted_here_p (aspace, pc))
    {
      ...
      if (gdbarch_cannot_step_breakpoint (gdbarch)) <-- [2]
        step = 0;
    }

spu doesn't have displaced stepping and uses software single step,
so 'step' is set to zero in [1], and [2] becomes unreachable as a
result.  So don't have to call set_gdbarch_cannot_step_breakpoint
in spu_gdbarch_init.

On the other hand, we either have hardware single step or software
single step, do we still need gdbarch method cannot_step_breakpoint?
CANNOT_STEP_BREAKPOINT was introduced in 1993 by commit
cef4c2e7a5f2d3426a8255f74b6c7f4e795fd9a4 for alpha OSF/1 native
support.

I don't have spu machine to test this patch.

gdb:

2015-03-17  Yao Qi  <yao.qi@linaro.org>

	* spu-tdep.c (spu_gdbarch_init): Don't call
	set_gdbarch_cannot_step_breakpoint.
---
 gdb/spu-tdep.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 36ad312..870cf32 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2794,7 +2794,6 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_decr_pc_after_break (gdbarch, 4);
   set_gdbarch_breakpoint_from_pc (gdbarch, spu_breakpoint_from_pc);
   set_gdbarch_memory_remove_breakpoint (gdbarch, spu_memory_remove_breakpoint);
-  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
   set_gdbarch_software_single_step (gdbarch, spu_software_single_step);
   set_gdbarch_get_longjmp_target (gdbarch, spu_get_longjmp_target);
 
-- 
1.9.1

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Pedro Alves]

+Ulrich

Ulrich, any idea why cannot_step_breakpoint was ever needed?

Yao's change makes sense to me.

Thanks,
Pedro Alves

On 03/17/2015 02:52 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
> 
> Nowadays, in infrun.c:resume, the setting to 'step' variable is like:
> 
>   if (use_displaced_stepping (gdbarch)
>       && tp->control.trap_expected
>       && sig == GDB_SIGNAL_0
>       && !current_inferior ()->waiting_for_vfork_done)
>     {
>     }
>   /* Do we need to do it the hard way, w/temp breakpoints?  */
>   else if (step)
>     step = maybe_software_singlestep (gdbarch, pc); <-- [1]
> 
>   ...
> 
>   if (execution_direction != EXEC_REVERSE
>       && step && breakpoint_inserted_here_p (aspace, pc))
>     {
>       ...
>       if (gdbarch_cannot_step_breakpoint (gdbarch)) <-- [2]
>         step = 0;
>     }
> 
> spu doesn't have displaced stepping and uses software single step,
> so 'step' is set to zero in [1], and [2] becomes unreachable as a
> result.  So don't have to call set_gdbarch_cannot_step_breakpoint
> in spu_gdbarch_init.
> 
> On the other hand, we either have hardware single step or software
> single step, do we still need gdbarch method cannot_step_breakpoint?
> CANNOT_STEP_BREAKPOINT was introduced in 1993 by commit
> cef4c2e7a5f2d3426a8255f74b6c7f4e795fd9a4 for alpha OSF/1 native
> support.
> 
> I don't have spu machine to test this patch.
> 
> gdb:
> 
> 2015-03-17  Yao Qi  <yao.qi@linaro.org>
> 
> 	* spu-tdep.c (spu_gdbarch_init): Don't call
> 	set_gdbarch_cannot_step_breakpoint.
> ---
>  gdb/spu-tdep.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
> index 36ad312..870cf32 100644
> --- a/gdb/spu-tdep.c
> +++ b/gdb/spu-tdep.c
> @@ -2794,7 +2794,6 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    set_gdbarch_decr_pc_after_break (gdbarch, 4);
>    set_gdbarch_breakpoint_from_pc (gdbarch, spu_breakpoint_from_pc);
>    set_gdbarch_memory_remove_breakpoint (gdbarch, spu_memory_remove_breakpoint);
> -  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
>    set_gdbarch_software_single_step (gdbarch, spu_software_single_step);
>    set_gdbarch_get_longjmp_target (gdbarch, spu_get_longjmp_target);
>  
> 

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Peter Schauer]

> +Ulrich
> 
> Ulrich, any idea why cannot_step_breakpoint was ever needed?

This was needed for alpha OSF/1.

Back then it was the only architecture which would not ptrace step
over an inserted breakpoint, causing an infinite loop while trying
to single step over an inserted breakpoint.

The diff back then was

+ #ifdef CANNOT_STEP_BREAKPOINT
+   /* If the target doesn't support stepping over a breakpoint, simply
+      continue, we will then hit the breakpoint anyway.  */
+   if (step && breakpoints_inserted && breakpoint_here_p (read_pc ()))
+     step = 0;
+ #endif

I do not know if GDB ever tries to ptrace step over an inserted
breakpoint nowadays, sorry.
Meanwhile the alpha OSF/1 port is dead anyways...

> Yao's change makes sense to me.
> 
> Thanks,
> Pedro Alves
> 
> On 03/17/2015 02:52 PM, Yao Qi wrote:
> > From: Yao Qi <yao.qi@linaro.org>
> > 
> > Nowadays, in infrun.c:resume, the setting to 'step' variable is like:
> > 
> >   if (use_displaced_stepping (gdbarch)
> >       && tp->control.trap_expected
> >       && sig == GDB_SIGNAL_0
> >       && !current_inferior ()->waiting_for_vfork_done)
> >     {
> >     }
> >   /* Do we need to do it the hard way, w/temp breakpoints?  */
> >   else if (step)
> >     step = maybe_software_singlestep (gdbarch, pc); <-- [1]
> > 
> >   ...
> > 
> >   if (execution_direction != EXEC_REVERSE
> >       && step && breakpoint_inserted_here_p (aspace, pc))
> >     {
> >       ...
> >       if (gdbarch_cannot_step_breakpoint (gdbarch)) <-- [2]
> >         step = 0;
> >     }
> > 
> > spu doesn't have displaced stepping and uses software single step,
> > so 'step' is set to zero in [1], and [2] becomes unreachable as a
> > result.  So don't have to call set_gdbarch_cannot_step_breakpoint
> > in spu_gdbarch_init.
> > 
> > On the other hand, we either have hardware single step or software
> > single step, do we still need gdbarch method cannot_step_breakpoint?
> > CANNOT_STEP_BREAKPOINT was introduced in 1993 by commit
> > cef4c2e7a5f2d3426a8255f74b6c7f4e795fd9a4 for alpha OSF/1 native
> > support.
> > 
> > I don't have spu machine to test this patch.
> > 
> > gdb:
> > 
> > 2015-03-17  Yao Qi  <yao.qi@linaro.org>
> > 
> > 	* spu-tdep.c (spu_gdbarch_init): Don't call
> > 	set_gdbarch_cannot_step_breakpoint.
> > ---
> >  gdb/spu-tdep.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
> > index 36ad312..870cf32 100644
> > --- a/gdb/spu-tdep.c
> > +++ b/gdb/spu-tdep.c
> > @@ -2794,7 +2794,6 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> >    set_gdbarch_decr_pc_after_break (gdbarch, 4);
> >    set_gdbarch_breakpoint_from_pc (gdbarch, spu_breakpoint_from_pc);
> >    set_gdbarch_memory_remove_breakpoint (gdbarch, spu_memory_remove_breakpoint);
> > -  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
> >    set_gdbarch_software_single_step (gdbarch, spu_software_single_step);
> >    set_gdbarch_get_longjmp_target (gdbarch, spu_get_longjmp_target);
> >  

-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Pedro Alves]

On 04/01/2015 09:35 PM, Peter Schauer wrote:

> This was needed for alpha OSF/1.
> 
> Back then it was the only architecture which would not ptrace step
> over an inserted breakpoint, causing an infinite loop while trying
> to single step over an inserted breakpoint.

OOC, do you recall whether the infinite loop was that the step didn't
make progress, and gdb would continuously issue a single-step forever,
or whether the infinite loop was all in the kernel?

> 
> The diff back then was
> 
> + #ifdef CANNOT_STEP_BREAKPOINT
> +   /* If the target doesn't support stepping over a breakpoint, simply
> +      continue, we will then hit the breakpoint anyway.  */
> +   if (step && breakpoints_inserted && breakpoint_here_p (read_pc ()))
> +     step = 0;
> + #endif
> 
> I do not know if GDB ever tries to ptrace step over an inserted
> breakpoint nowadays, sorry.

It does in some cases when we have a signal to deliver at the
same time we are trying to step over a breakpoint.  Look for
"signal arrived while stepping over" in infrun.c.

> Meanwhile the alpha OSF/1 port is dead anyways...

The setting ended up done for all alpha ports today though, in:

  alpha-tdep.c:  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);

OSF/1 is gone, but we still support Alpha GNU/Linux, which is also
taking that code path,. If this was OSF/1 specific, then we could
get rid of that too, and then get rid of gdbarch_cannot_step_breakpoint
completely.  Anyone have access to Alpha GNU/Linux to try that out?

Ulrich, any idea why cannot_step_breakpoint was ever needed
for the SPU?

Thanks,
Pedro Alves

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Peter Schauer]

> On 04/01/2015 09:35 PM, Peter Schauer wrote:
> 
> > This was needed for alpha OSF/1.
> > 
> > Back then it was the only architecture which would not ptrace step
> > over an inserted breakpoint, causing an infinite loop while trying
> > to single step over an inserted breakpoint.
> 
> OOC, do you recall whether the infinite loop was that the step didn't
> make progress, and gdb would continuously issue a single-step forever,
> or whether the infinite loop was all in the kernel?

The step didn't make progress and GDB would have continuously issued
a single-step forever.

> > The diff back then was
> > 
> > + #ifdef CANNOT_STEP_BREAKPOINT
> > +   /* If the target doesn't support stepping over a breakpoint, simply
> > +      continue, we will then hit the breakpoint anyway.  */
> > +   if (step && breakpoints_inserted && breakpoint_here_p (read_pc ()))
> > +     step = 0;
> > + #endif
> > 
> > I do not know if GDB ever tries to ptrace step over an inserted
> > breakpoint nowadays, sorry.
> 
> It does in some cases when we have a signal to deliver at the
> same time we are trying to step over a breakpoint.  Look for
> "signal arrived while stepping over" in infrun.c.

Yeah, that was also the reason why we had to keep the breakpoint
inserted back then.

> > Meanwhile the alpha OSF/1 port is dead anyways...
> 
> The setting ended up done for all alpha ports today though, in:
> 
>   alpha-tdep.c:  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
> 
> OSF/1 is gone, but we still support Alpha GNU/Linux, which is also
> taking that code path,. If this was OSF/1 specific, then we could
> get rid of that too, and then get rid of gdbarch_cannot_step_breakpoint
> completely.  Anyone have access to Alpha GNU/Linux to try that out?

If it really happens on Alpha GNU/Linux, we could request a fix from the
kernel folks and phase out this ugly gdbarch_cannot_step_breakpoint hack
slowly.

> Ulrich, any idea why cannot_step_breakpoint was ever needed
> for the SPU?
> 
> Thanks,
> Pedro Alves

-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Pedro Alves]

On 04/02/2015 10:09 AM, Peter Schauer wrote:
>> On 04/01/2015 09:35 PM, Peter Schauer wrote:
>>
>>> This was needed for alpha OSF/1.
>>>
>>> Back then it was the only architecture which would not ptrace step
>>> over an inserted breakpoint, causing an infinite loop while trying
>>> to single step over an inserted breakpoint.
>>
>> OOC, do you recall whether the infinite loop was that the step didn't
>> make progress, and gdb would continuously issue a single-step forever,
>> or whether the infinite loop was all in the kernel?
> 
> The step didn't make progress and GDB would have continuously issued
> a single-step forever.

OK, thanks.

> 
>>> The diff back then was
>>>
>>> + #ifdef CANNOT_STEP_BREAKPOINT
>>> +   /* If the target doesn't support stepping over a breakpoint, simply
>>> +      continue, we will then hit the breakpoint anyway.  */
>>> +   if (step && breakpoints_inserted && breakpoint_here_p (read_pc ()))
>>> +     step = 0;
>>> + #endif
>>>
>>> I do not know if GDB ever tries to ptrace step over an inserted
>>> breakpoint nowadays, sorry.
>>
>> It does in some cases when we have a signal to deliver at the
>> same time we are trying to step over a breakpoint.  Look for
>> "signal arrived while stepping over" in infrun.c.
> 
> Yeah, that was also the reason why we had to keep the breakpoint
> inserted back then.
> 
>>> Meanwhile the alpha OSF/1 port is dead anyways...
>>
>> The setting ended up done for all alpha ports today though, in:
>>
>>   alpha-tdep.c:  set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
>>
>> OSF/1 is gone, but we still support Alpha GNU/Linux, which is also
>> taking that code path,. If this was OSF/1 specific, then we could
>> get rid of that too, and then get rid of gdbarch_cannot_step_breakpoint
>> completely.  Anyone have access to Alpha GNU/Linux to try that out?
> 
> If it really happens on Alpha GNU/Linux, we could request a fix from the
> kernel folks and phase out this ugly gdbarch_cannot_step_breakpoint hack
> slowly.

Yeah.  I think the hack is probably breaking the case of nested
signals while stepping over a breakpoint (gdb.base/signest.exp), as nothing
is forcing the insertion of breakpoints when the hack triggers.  If
needed, it should probably be merged with the code below for
software-step targets:

  /* Currently, our software single-step implementation leads to different
     results than hardware single-stepping in one situation: when stepping
     into delivering a signal which has an associated signal handler,
     hardware single-step will stop at the first instruction of the handler,
     while software single-step will simply skip execution of the handler.
     ...

Thanks,
Pedro Alves

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Yao Qi]

On 02/04/15 09:58, Pedro Alves wrote:
> If this was OSF/1 specific, then we could
> get rid of that too, and then get rid of gdbarch_cannot_step_breakpoint
> completely.  Anyone have access to Alpha GNU/Linux to try that out?

Yes, that is what I want to do too, however, I don't have access to
alpha/linux box :(

-- 
Yao (齐尧)

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Ulrich Weigand]

Pedro Alves wrote:

> Ulrich, any idea why cannot_step_breakpoint was ever needed
> for the SPU?

It's been set since spu-tdep.c was committed upstream, and in
fact it's set in every single version in our internal repositories
of the code before it went upstream, too.  (Going back over 10 years.)

But I don't know why it was added either, since SPU has been using
software single-step forever too, and even the very first version of
GDB we patched to support SPU would ignore cannot_step_breakpoint
for software single-step architectures ...

So I'd say just go ahead and remove it.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [rfc, spu] Don't call set_gdbarch_cannot_step_breakpoint in spu_gdbarch_init

[Yao Qi]

On 07/04/15 13:45, Ulrich Weigand wrote:
> It's been set since spu-tdep.c was committed upstream, and in
> fact it's set in every single version in our internal repositories
> of the code before it went upstream, too.  (Going back over 10 years.)
>
> But I don't know why it was added either, since SPU has been using
> software single-step forever too, and even the very first version of
> GDB we patched to support SPU would ignore cannot_step_breakpoint
> for software single-step architectures ...
>
> So I'd say just go ahead and remove it.

Patch is pushed in.

-- 
Yao (齐尧)