eliminate deprecated_insert_raw_breakpoint. what's left.

eliminate deprecated_insert_raw_breakpoint. what's left.

[Pedro Alves]

Hey,

I'm working on a patch that converts software single-step breakpoints
to real breakpoints.  It's largely done, but needs a bit of cleaning up
before I'll post it.  One issue is that software single-step breakpoints
were implemented on top of deprecated_insert_raw_breakpoint.  They no
longer are after my patch, but we're still left with some ugly code in
breakpoint.c related to deprecated_insert_raw_breakpoints on top of other
breakpoints that I'd like to eliminate completely, but that can only
be done once all users of deprecated_insert_raw_breakpoint are either converted
to some more modern mechanism, or eliminated.

Before I go spend some effort blindly trying to do these remaining
conversions, I thought I'd run them through the list first, in case some
of this code can be considered dead already, or in case someone can
lend a hand (which would be awesome).

The remaining users are:

 procfs.c:      dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL,

This is only compiled if SYS_syssgi is defined, which I believe means
MIPS IRIX.  Do we still care about MIPS IRIX?

 solib-irix.c:      base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (),

This could/should probably be converted to use create_solib_event_breakpoint.
Though see above.

 rs6000-nat.c:  bp = deprecated_insert_raw_breakpoint (gdbarch, NULL, DUMMY_INSN_ADDR);

This is AIX code.  Looks like this can easily be converted to a momentary breakpoint?

AFAIR, I have no way to test either AIX or IRIX.

Thanks,
Pedro Alves

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Joel Brobecker]

Hi Pedro,

> The remaining users are:
> 
>  procfs.c:      dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL,
> 
> This is only compiled if SYS_syssgi is defined, which I believe means
> MIPS IRIX.  Do we still care about MIPS IRIX?
> 
>  solib-irix.c:      base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (),

I would be surprised if anyone still cared about IRIX anymore.
I enjoyed working on that system, but I no longer have access
to it, so can't support it anymore.

> This could/should probably be converted to use create_solib_event_breakpoint.
> Though see above.
> 
>  rs6000-nat.c:  bp = deprecated_insert_raw_breakpoint (gdbarch, NULL, DUMMY_INSN_ADDR);
> 
> This is AIX code.  Looks like this can easily be converted to a
> momentary breakpoint?

I am actually wondering whether this is still needed. It could!
So, the first thing I wanted to do was to run the testsuite without.
I'm currently building GDB, which is taking forever, and will then
run AdaCore's testsuite.

I can certainly assist in making the change regardless!

In terms of testing, I can only run AdaCore's testsuite because
expect isn't working for me on AIX, and I lack the time to
research it further. Although not ideal, it's been good enough
for us.

-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Joel Brobecker]

> > This is AIX code.  Looks like this can easily be converted to a
> > momentary breakpoint?
> 
> I am actually wondering whether this is still needed. It could!
> So, the first thing I wanted to do was to run the testsuite without.
> I'm currently building GDB, which is taking forever, and will then
> run AdaCore's testsuite.

Looking at the code, it should be executed each time the SP register
gets changed, which means it should trigger when calling functions
from GDB. Deactivating exec_one_dummy_insn entirely did not result
in any regression I could notice. That was on AIX 7.1.

That being said, I'm hesitant of removing the code regardless,
since this could only be needed in specific situations which might
not be covered by the testsuites we have.

I was looking at how to replace that call, but I am not sure
how to fix the code up, though. Perhaps we could just write
the breakpoint instruction in by hand, rather than go through
the breakpoint module? After all, it is already doing almost
everything else by hand!

In fact, looking at the code again now, I'm a little more tempted
to see what happens if we remove it ;-).

-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Pedro Alves]

On 09/08/2014 10:34 PM, Joel Brobecker wrote:

> I was looking at how to replace that call, but I am not sure
> how to fix the code up, though. Perhaps we could just write
> the breakpoint instruction in by hand, rather than go through
> the breakpoint module? After all, it is already doing almost
> everything else by hand!

Indeed.

> In fact, looking at the code again now, I'm a little more tempted
> to see what happens if we remove it ;-).

Me too.  And seriously.  :-)

I traced it back to I think the original rs6000 port, in 1991...

commit 41abdfbd2de07837ba8088092765154eaa66351d
Author: John Gilmore <gnu@cygnus>
Date:   Tue Nov 12 15:50:47 1991 +0000

    * rs6000-pinsn.c, rs6000-tdep.c, rs6000-xdep.c, tm-rs6000.h,
    xm-rs6000.h:  New files.
    * xcoffexec.c:  New file for handling AIX shared libraries.



We already see this then, in rs6000-xdep.c:

+       /* execute one dummy instruction (which is a breakpoint) in inferior
+          process. So give kernel a chance to do internal house keeping.
+         Otherwise the following ptrace(2) calls will mess up user stack
+         since kernel will get confused about the bottom of the stack (%sp) */
+
+       exec_one_dummy_insn ();


This sounds like working around a (very) old kernel bug...
I can't believe anything resembling a modern system would need
such a monstrosity!  :-)  I vote just removing all that.

Thanks,
Pedro Alves

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Peter Schauer]

> 
> On 09/08/2014 10:34 PM, Joel Brobecker wrote:
> 
> > I was looking at how to replace that call, but I am not sure
> > how to fix the code up, though. Perhaps we could just write
> > the breakpoint instruction in by hand, rather than go through
> > the breakpoint module? After all, it is already doing almost
> > everything else by hand!
> 
> Indeed.
> 
> > In fact, looking at the code again now, I'm a little more tempted
> > to see what happens if we remove it ;-).
> 
> Me too.  And seriously.  :-)
> 
> I traced it back to I think the original rs6000 port, in 1991...
> 
> commit 41abdfbd2de07837ba8088092765154eaa66351d
> Author: John Gilmore <gnu@cygnus>
> Date:   Tue Nov 12 15:50:47 1991 +0000
> 
>     * rs6000-pinsn.c, rs6000-tdep.c, rs6000-xdep.c, tm-rs6000.h,
>     xm-rs6000.h:  New files.
>     * xcoffexec.c:  New file for handling AIX shared libraries.
> 
> 
> 
> We already see this then, in rs6000-xdep.c:
> 
> +       /* execute one dummy instruction (which is a breakpoint) in inferior
> +          process. So give kernel a chance to do internal house keeping.
> +         Otherwise the following ptrace(2) calls will mess up user stack
> +         since kernel will get confused about the bottom of the stack (%sp) */
> +
> +       exec_one_dummy_insn ();
> 
> 
> This sounds like working around a (very) old kernel bug...

No, not a bug, but consequences of a very peculiar ptrace implementation,
at least up until and including AIX 4.3.

See my other post for a more detailed explanation.

> I can't believe anything resembling a modern system would need
> such a monstrosity!  :-)  I vote just removing all that.

Me too, provided that newer AIX versions have a better ptrace implementation,
making this  monstrosity unnecessary.

-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Peter Schauer]

> 
> > > This is AIX code.  Looks like this can easily be converted to a
> > > momentary breakpoint?
> > 
> > I am actually wondering whether this is still needed. It could!
> > So, the first thing I wanted to do was to run the testsuite without.
> > I'm currently building GDB, which is taking forever, and will then
> > run AdaCore's testsuite.
> 
> Looking at the code, it should be executed each time the SP register
> gets changed, which means it should trigger when calling functions
> from GDB. Deactivating exec_one_dummy_insn entirely did not result
> in any regression I could notice. That was on AIX 7.1.
> 
> That being said, I'm hesitant of removing the code regardless,
> since this could only be needed in specific situations which might
> not be covered by the testsuites we have.
> 
> I was looking at how to replace that call, but I am not sure
> how to fix the code up, though. Perhaps we could just write
> the breakpoint instruction in by hand, rather than go through
> the breakpoint module? After all, it is already doing almost
> everything else by hand!
> 
> In fact, looking at the code again now, I'm a little more tempted
> to see what happens if we remove it ;-).

I hope to be able to shed some light on this problem, although it
is more than fifteen years ago that I did some work for GDB on AIX.

From my notes back then, AIX 3 and AIX 4 had a very peculiar ptrace
implementation, where the current ptrace state of the inferior process
(including the current process registers) was maintained approximately
512 bytes below the current user stack pointer of the process.

This resulted in problems with AIX inferior function calls.
If the called function takes one or more large aggregate parameters
by value, or if you pass a large amount of parameters, the ptrace
area gets corrupted, when the dummy function call parameters are
pushed on the user stack, due to this awkward AIX stack layout.

To work around this problem, the execution of a dummy instruction
(when altering the stack pointer) caused the kernel to move the ptrace
state area further below on the user stack, allowing GDB to write below
the current user stack safely.
In GDB 6.x, rs6000_push_dummy_call even secured the stack partially during
pushing of the arguments, via an additional call of
regcache_raw_write_signed to gdbarch_sp_regnum (gdbarch), which is
no longer present in current versions of GDB.

Executing the dummy instruction is very fragile, especially if signals
get involved during the execution, and it didn't even help, if more
than ~100 bytes of parameters were pushed on the user stack on AIX 4.
Back then, there was no other choice though.

Unfortunately I do not know, if this peculiar AIX stack layout is still
used in AIX 5 or later, maybe Ulrich Weigand could tell you more about it.

I think you could/should zap exec_one_dummy_insn, provided that you test
a dummy function call on the oldest AIX version that GDB has to support,
with a large aggregate parameter, which is passed by value.

-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Peter Schauer wrote:

> I hope to be able to shed some light on this problem, although it
> is more than fifteen years ago that I did some work for GDB on AIX.
> 
> From my notes back then, AIX 3 and AIX 4 had a very peculiar ptrace
> implementation, where the current ptrace state of the inferior process
> (including the current process registers) was maintained approximately
> 512 bytes below the current user stack pointer of the process.
> 
> This resulted in problems with AIX inferior function calls.
> If the called function takes one or more large aggregate parameters
> by value, or if you pass a large amount of parameters, the ptrace
> area gets corrupted, when the dummy function call parameters are
> pushed on the user stack, due to this awkward AIX stack layout.

Thanks for providing this background!

> To work around this problem, the execution of a dummy instruction
> (when altering the stack pointer) caused the kernel to move the ptrace
> state area further below on the user stack, allowing GDB to write below
> the current user stack safely.
> In GDB 6.x, rs6000_push_dummy_call even secured the stack partially during
> pushing of the arguments, via an additional call of
> regcache_raw_write_signed to gdbarch_sp_regnum (gdbarch), which is
> no longer present in current versions of GDB.

Well, I still see this:
  /* Set the stack pointer.  According to the ABI, the SP is meant to
     be set _before_ the corresponding stack space is used.  On AIX,
     this even applies when the target has been completely stopped!
     Not doing this can lead to conflicts with the kernel which thinks
     that it still has control over this not-yet-allocated stack
     region.  */
  regcache_raw_write_signed (regcache, gdbarch_sp_regnum (gdbarch), sp);

and:
      /* This is another instance we need to be concerned about
         securing our stack space.  If we write anything underneath %sp
         (r1), we might conflict with the kernel who thinks he is free
         to use this area.  So, update %sp first before doing anything
         else.  */

      regcache_raw_write_signed (regcache,
                                 gdbarch_sp_regnum (gdbarch), sp);

Are there other instances where this is missing?

> Executing the dummy instruction is very fragile, especially if signals
> get involved during the execution, and it didn't even help, if more
> than ~100 bytes of parameters were pushed on the user stack on AIX 4.
> Back then, there was no other choice though.
> 
> Unfortunately I do not know, if this peculiar AIX stack layout is still
> used in AIX 5 or later, maybe Ulrich Weigand could tell you more about it.

I don't know off-hand.  I'll try to find out.
 
> I think you could/should zap exec_one_dummy_insn, provided that you test
> a dummy function call on the oldest AIX version that GDB has to support,
> with a large aggregate parameter, which is passed by value.

The only version I have ready access to is AIX 7.1, and on this there
are no testsuite regression (and in fact, quite a number of failures
seem to go away!) when zapping exec_one_dummy_insn.

I'm not sure which versions we need to / should support in GDB; I guess
the oldest version where the OS itself is still supported by IBM is 6.1.

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Peter Schauer]

> Peter Schauer wrote:
> 
> > I hope to be able to shed some light on this problem, although it
> > is more than fifteen years ago that I did some work for GDB on AIX.
> > 
> > From my notes back then, AIX 3 and AIX 4 had a very peculiar ptrace
> > implementation, where the current ptrace state of the inferior process
> > (including the current process registers) was maintained approximately
> > 512 bytes below the current user stack pointer of the process.
> > 
> > This resulted in problems with AIX inferior function calls.
> > If the called function takes one or more large aggregate parameters
> > by value, or if you pass a large amount of parameters, the ptrace
> > area gets corrupted, when the dummy function call parameters are
> > pushed on the user stack, due to this awkward AIX stack layout.
> 
> Thanks for providing this background!
> 
> > To work around this problem, the execution of a dummy instruction
> > (when altering the stack pointer) caused the kernel to move the ptrace
> > state area further below on the user stack, allowing GDB to write below
> > the current user stack safely.
> > In GDB 6.x, rs6000_push_dummy_call even secured the stack partially during
> > pushing of the arguments, via an additional call of
> > regcache_raw_write_signed to gdbarch_sp_regnum (gdbarch), which is
> > no longer present in current versions of GDB.
> 
> Well, I still see this:
>   /* Set the stack pointer.  According to the ABI, the SP is meant to
>      be set _before_ the corresponding stack space is used.  On AIX,
>      this even applies when the target has been completely stopped!
>      Not doing this can lead to conflicts with the kernel which thinks
>      that it still has control over this not-yet-allocated stack
>      region.  */
>   regcache_raw_write_signed (regcache, gdbarch_sp_regnum (gdbarch), sp);
> 
> and:
>       /* This is another instance we need to be concerned about
>          securing our stack space.  If we write anything underneath %sp
>          (r1), we might conflict with the kernel who thinks he is free
>          to use this area.  So, update %sp first before doing anything
>          else.  */
> 
>       regcache_raw_write_signed (regcache,
>                                  gdbarch_sp_regnum (gdbarch), sp);
> 
> Are there other instances where this is missing?

Ok, my bad, I was looking at the wrong push_dummy_call implementation
in the current GDB source.
rs6000_push_dummy_call in the new rs6000-aix-tdep.c file in the current GDB
source still contains the code in question from GDB 6.x, there is
nothing missing.

> > Executing the dummy instruction is very fragile, especially if signals
> > get involved during the execution, and it didn't even help, if more
> > than ~100 bytes of parameters were pushed on the user stack on AIX 4.
> > Back then, there was no other choice though.
> > 
> > Unfortunately I do not know, if this peculiar AIX stack layout is still
> > used in AIX 5 or later, maybe Ulrich Weigand could tell you more about it.
> 
> I don't know off-hand.  I'll try to find out.
>  
> > I think you could/should zap exec_one_dummy_insn, provided that you test
> > a dummy function call on the oldest AIX version that GDB has to support,
> > with a large aggregate parameter, which is passed by value.
> 
> The only version I have ready access to is AIX 7.1, and on this there
> are no testsuite regression (and in fact, quite a number of failures
> seem to go away!) when zapping exec_one_dummy_insn.

+1 for zapping exec_one_dummy_insn.

> I'm not sure which versions we need to / should support in GDB; I guess
> the oldest version where the OS itself is still supported by IBM is 6.1.

Maybe somebody could test if zapping exec_one_dummy_insn on AIX 6.1
has any negative effect, and then be done with it.

But even if that can't be tested, I am all in favour of getting rid
of it, perhaps with a detailed comment in the commit message for the
removal (or adding a link to this thread).

-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Peter Schauer wrote:

> > > I think you could/should zap exec_one_dummy_insn, provided that you test
> > > a dummy function call on the oldest AIX version that GDB has to support,
> > > with a large aggregate parameter, which is passed by value.
> > 
> > The only version I have ready access to is AIX 7.1, and on this there
> > are no testsuite regression (and in fact, quite a number of failures
> > seem to go away!) when zapping exec_one_dummy_insn.
> 
> +1 for zapping exec_one_dummy_insn.
> 
> > I'm not sure which versions we need to / should support in GDB; I guess
> > the oldest version where the OS itself is still supported by IBM is 6.1.
> 
> Maybe somebody could test if zapping exec_one_dummy_insn on AIX 6.1
> has any negative effect, and then be done with it.
> 
> But even if that can't be tested, I am all in favour of getting rid
> of it, perhaps with a detailed comment in the commit message for the
> removal (or adding a link to this thread).

I've now got access to an AIX 6.1 machine and repeated the experiment --
with the same result.  No regressions when zapping exec_one_dummy_insn,
and in fact about 100 FAILs fixed.

So I think we should probably just do it at this point.

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Joel Brobecker]

> I've now got access to an AIX 6.1 machine and repeated the experiment --
> with the same result.  No regressions when zapping exec_one_dummy_insn,
> and in fact about 100 FAILs fixed.
> 
> So I think we should probably just do it at this point.

Agreed! Would you do the honors, or would you like me to?

Thanks for doing the testing, Ulrich.
-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Joel Brobecker wrote:
> > I've now got access to an AIX 6.1 machine and repeated the experiment --
> > with the same result.  No regressions when zapping exec_one_dummy_insn,
> > and in fact about 100 FAILs fixed.
> > 
> > So I think we should probably just do it at this point.
> 
> Agreed! Would you do the honors, or would you like me to?
> 
> Thanks for doing the testing, Ulrich.

I'll check it in shortly.

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Pedro Alves]

On 09/10/2014 02:15 PM, Ulrich Weigand wrote:
> Joel Brobecker wrote:
>>> I've now got access to an AIX 6.1 machine and repeated the experiment --
>>> with the same result.  No regressions when zapping exec_one_dummy_insn,
>>> and in fact about 100 FAILs fixed.
>>>
>>> So I think we should probably just do it at this point.
>>
>> Agreed! Would you do the honors, or would you like me to?
>>
>> Thanks for doing the testing, Ulrich.
> 
> I'll check it in shortly.

Thanks guys!

One down, two to go.  :-)

Thanks,
Pedro Alves

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Joel Brobecker wrote:

> >  procfs.c:      dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL,
> > 
> > This is only compiled if SYS_syssgi is defined, which I believe means
> > MIPS IRIX.  Do we still care about MIPS IRIX?
> > 
> >  solib-irix.c:      base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (),
> 
> I would be surprised if anyone still cared about IRIX anymore.
> I enjoyed working on that system, but I no longer have access
> to it, so can't support it anymore.

I see that IRIX seems to be in "Retired Mode" as of 12/31/2013:
http://www.sgi.com/tech/irix/?_mips_support.html

Do we have a current process to formally obsolete/remove support
for old systems?

[ I guess another candidate to remove might be Alpha OSF/1 ... ]

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Joel Brobecker]

> > I would be surprised if anyone still cared about IRIX anymore.
> > I enjoyed working on that system, but I no longer have access
> > to it, so can't support it anymore.
> 
> I see that IRIX seems to be in "Retired Mode" as of 12/31/2013:
> http://www.sgi.com/tech/irix/?_mips_support.html

And I saw yesterday on the binutils mailing-list that IRIX support
was removed from GCC already. I think these are all pretty strong
indicators.

> Do we have a current process to formally obsolete/remove support
> for old systems?
> 
> [ I guess another candidate to remove might be Alpha OSF/1 ... ]

Here is what I found:
https://sourceware.org/gdb/wiki/Internals%20Obsoleting-code

I agree we can obsolete Alpha OSF/1 as well.

-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Joel Brobecker wrote:
> > > I would be surprised if anyone still cared about IRIX anymore.
> > > I enjoyed working on that system, but I no longer have access
> > > to it, so can't support it anymore.
> > 
> > I see that IRIX seems to be in "Retired Mode" as of 12/31/2013:
> > http://www.sgi.com/tech/irix/?_mips_support.html
> 
> And I saw yesterday on the binutils mailing-list that IRIX support
> was removed from GCC already. I think these are all pretty strong
> indicators.
> 
> > Do we have a current process to formally obsolete/remove support
> > for old systems?
> > 
> > [ I guess another candidate to remove might be Alpha OSF/1 ... ]
> 
> Here is what I found:
> https://sourceware.org/gdb/wiki/Internals%20Obsoleting-code
> 
> I agree we can obsolete Alpha OSF/1 as well.

Once OSF/1 and IRIX are gone, I hope all of the ECOFF/mdebug debug
format support can go as well (mipsread.c, mdebugread.c etc.) ...

The process documented in the Wiki is a bit weird (adding OBSOLETE
to *every line* of those files ???) and it seems we didn't follow
it in the last major round of obsoleting code either:
https://sourceware.org/ml/gdb-announce/2007/msg00000.html

I think using a process along similar lines might be best.

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Pedro Alves]

On 09/10/2014 03:45 PM, Ulrich Weigand wrote:
> Joel Brobecker wrote:
>>>> I would be surprised if anyone still cared about IRIX anymore.
>>>> I enjoyed working on that system, but I no longer have access
>>>> to it, so can't support it anymore.
>>>
>>> I see that IRIX seems to be in "Retired Mode" as of 12/31/2013:
>>> http://www.sgi.com/tech/irix/?_mips_support.html
>>
>> And I saw yesterday on the binutils mailing-list that IRIX support
>> was removed from GCC already. I think these are all pretty strong
>> indicators.
>>
>>> Do we have a current process to formally obsolete/remove support
>>> for old systems?
>>>
>>> [ I guess another candidate to remove might be Alpha OSF/1 ... ]
>>
>> Here is what I found:
>> https://sourceware.org/gdb/wiki/Internals%20Obsoleting-code
>>
>> I agree we can obsolete Alpha OSF/1 as well.
> 
> Once OSF/1 and IRIX are gone, I hope all of the ECOFF/mdebug debug
> format support can go as well (mipsread.c, mdebugread.c etc.) ...
> 
> The process documented in the Wiki is a bit weird (adding OBSOLETE
> to *every line* of those files ???)

Yeah, I recall seeing such OBSOLETE-marked files long ago.  I agree
it's weird.

> and it seems we didn't follow
> it in the last major round of obsoleting code either:
> https://sourceware.org/ml/gdb-announce/2007/msg00000.html
> 
> I think using a process along similar lines might be best.

Definitely agreed.

I think that obsoleting page is obsolete.  :-)

Meanwhile, I'd prefer removing deprecated_insert_raw_breakpoint
now rather than keeping it just for IRIX.  I may send a
best-effort-but-untested patch for IRIX.  If things break, it'd be
the job of whoever shows up as wanting to maintain IRIX to
fix and modernize things further.  WDYT?

Thanks,
Pedro Alves

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Joel Brobecker]

> > and it seems we didn't follow
> > it in the last major round of obsoleting code either:
> > https://sourceware.org/ml/gdb-announce/2007/msg00000.html
> > 
> > I think using a process along similar lines might be best.
> 
> Definitely agreed.
> 
> I think that obsoleting page is obsolete.  :-)

OK with me as well. I suspect this was to allow for easy reviving
if someone stepped up, but completely OBE now that we've moved
to git.

So let's discuss the new obsoleting procedure, so we can document it:

 . I think that the first 4 steps (post email on gdb@, wait a week,
   then on gdb-announce, wait another week) are fine. Anyone thinks
   we should go straight to gdb-announce?

   My thinking is that people interested in maintaining a port
   with enough skills to do so are likely to already be on gdb@,
   so we can avoid sending an extra mail to gdb-announce. But
   the traffic on gdb-announce being very low, and the frequency
   at which we deprecate targets being fairly small as well,
   I wouldn't object to a simpler procedure where we email
   gdb-announce directly.

 . Remove steps 5 & 6 that mark the code as obsolete, only keeping
   the last test, which removes the code. I'd add a note to add
   a NEWS entry.

> Meanwhile, I'd prefer removing deprecated_insert_raw_breakpoint
> now rather than keeping it just for IRIX.  I may send a
> best-effort-but-untested patch for IRIX.  If things break, it'd be
> the job of whoever shows up as wanting to maintain IRIX to
> fix and modernize things further.  WDYT?

Sounds like a plan.

-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Sergio Durigan Junior]

On Wednesday, September 10 2014, Joel Brobecker wrote:

> So let's discuss the new obsoleting procedure, so we can document it:
>
>  . I think that the first 4 steps (post email on gdb@, wait a week,
>    then on gdb-announce, wait another week) are fine. Anyone thinks
>    we should go straight to gdb-announce?
>
>    My thinking is that people interested in maintaining a port
>    with enough skills to do so are likely to already be on gdb@,
>    so we can avoid sending an extra mail to gdb-announce. But
>    the traffic on gdb-announce being very low, and the frequency
>    at which we deprecate targets being fairly small as well,
>    I wouldn't object to a simpler procedure where we email
>    gdb-announce directly.

Not sure if it is implicit, but it would be good to send an email to
gdb-patches as well.  I have a feeling that some people just read/pay
attention to gdb-patches...

Other than that, I totally agree with the plan.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Joel Brobecker wrote:

> So let's discuss the new obsoleting procedure, so we can document it:
> 
>  . I think that the first 4 steps (post email on gdb@, wait a week,
>    then on gdb-announce, wait another week) are fine. Anyone thinks
>    we should go straight to gdb-announce?
> 
>    My thinking is that people interested in maintaining a port
>    with enough skills to do so are likely to already be on gdb@,
>    so we can avoid sending an extra mail to gdb-announce. But
>    the traffic on gdb-announce being very low, and the frequency
>    at which we deprecate targets being fairly small as well,
>    I wouldn't object to a simpler procedure where we email
>    gdb-announce directly.
> 
>  . Remove steps 5 & 6 that mark the code as obsolete, only keeping
>    the last test, which removes the code. I'd add a note to add
>    a NEWS entry.

Sounds all good to me.

Bye,
Ulrich

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

New deprecation procedure

[Pedro Alves]

On 09/10/2014 05:35 PM, Ulrich Weigand wrote:
> Joel Brobecker wrote:
> 
>> So let's discuss the new obsoleting procedure, so we can document it:
>>
>>  . I think that the first 4 steps (post email on gdb@, wait a week,
>>    then on gdb-announce, wait another week) are fine. Anyone thinks
>>    we should go straight to gdb-announce?
>>
>>    My thinking is that people interested in maintaining a port
>>    with enough skills to do so are likely to already be on gdb@,
>>    so we can avoid sending an extra mail to gdb-announce. But
>>    the traffic on gdb-announce being very low, and the frequency
>>    at which we deprecate targets being fairly small as well,
>>    I wouldn't object to a simpler procedure where we email
>>    gdb-announce directly.
>>
>>  . Remove steps 5 & 6 that mark the code as obsolete, only keeping
>>    the last test, which removes the code. I'd add a note to add
>>    a NEWS entry.
> 
> Sounds all good to me.

To me too.

Thanks,
Pedro Alves

Re: New deprecation procedure

[Joel Brobecker]

> >> So let's discuss the new obsoleting procedure, so we can document it:
> >>
> >>  . I think that the first 4 steps (post email on gdb@, wait a week,
> >>    then on gdb-announce, wait another week) are fine. Anyone thinks
> >>    we should go straight to gdb-announce?
> >>
> >>    My thinking is that people interested in maintaining a port
> >>    with enough skills to do so are likely to already be on gdb@,
> >>    so we can avoid sending an extra mail to gdb-announce. But
> >>    the traffic on gdb-announce being very low, and the frequency
> >>    at which we deprecate targets being fairly small as well,
> >>    I wouldn't object to a simpler procedure where we email
> >>    gdb-announce directly.
> >>
> >>  . Remove steps 5 & 6 that mark the code as obsolete, only keeping
> >>    the last test, which removes the code. I'd add a note to add
> >>    a NEWS entry.
> > 
> > Sounds all good to me.
> 
> To me too.

OK, thanks all! Wiki page updated accordingly.

Sharing a thought that crossed my mind: I thought about increasing
the amount of time we wait between steps, from 1 week to say, 2 weeks,
giving anyone about a month to step up. I eventually dropped the idea
because someone stepping up late should easily be able to revert
the removal, particularly now that we've switched to git. In the
meantime, since we suspect no-one is usually going to step up,
waiting longer just defers the corresponding cleanups we want to do.
If you guys agree with that, I'll add something to the wiki page
to explain the thought process.

-- 
Joel

Re: New deprecation procedure

[Ulrich Weigand]

Joel Brobecker wrote:

> Sharing a thought that crossed my mind: I thought about increasing
> the amount of time we wait between steps, from 1 week to say, 2 weeks,
> giving anyone about a month to step up. I eventually dropped the idea
> because someone stepping up late should easily be able to revert
> the removal, particularly now that we've switched to git. In the
> meantime, since we suspect no-one is usually going to step up,
> waiting longer just defers the corresponding cleanups we want to do.
> If you guys agree with that, I'll add something to the wiki page
> to explain the thought process.

Makes sense to me.

Thanks,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Maciej W. Rozycki]

On Wed, 10 Sep 2014, Pedro Alves wrote:

> Meanwhile, I'd prefer removing deprecated_insert_raw_breakpoint
> now rather than keeping it just for IRIX.  I may send a
> best-effort-but-untested patch for IRIX.  If things break, it'd be
> the job of whoever shows up as wanting to maintain IRIX to
> fix and modernize things further.  WDYT?

 FWIW I have no objections to this plan.  I wonder if there's anything 
special here actually or if it's just that nobody bothered to convert this 
code to using `create_internal_breakpoint' or suchlike.

  Maciej

[IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Pedro Alves]

On 09/10/2014 04:50 PM, Maciej W. Rozycki wrote:
> On Wed, 10 Sep 2014, Pedro Alves wrote:
> 
>> Meanwhile, I'd prefer removing deprecated_insert_raw_breakpoint
>> now rather than keeping it just for IRIX.  I may send a
>> best-effort-but-untested patch for IRIX.  If things break, it'd be
>> the job of whoever shows up as wanting to maintain IRIX to
>> fix and modernize things further.  WDYT?
> 
>  FWIW I have no objections to this plan.  I wonder if there's anything 
> special here actually or if it's just that nobody bothered to convert this 
> code to using `create_internal_breakpoint' or suchlike.

There are definitely things here that could/should be modernized.
The loop in irix_solib_create_inferior_hook (and similars in other ports)
is something that's an old way of doing things and gotten in the way
before.

I think I understand this enough now.  See patch below, which
gets rid of that loop completely.

Untested, of course.

--------
From 70c27f4a9a26c46d5ac356258ba2f76a6ac04269 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 10 Sep 2014 14:56:51 +0100
Subject: [PATCH] [IRIX] eliminate deprecated_insert_raw_breakpoint uses

The IRIX support wants to set a breakpoint to be hit when the startup
phase is complete, which is where shared libraries have been mapped
in.  AFAIU, for most IRIX ports, that location is the entry point.

For MIPS IRIX however, GDB needs to set a breakpoint earlier, in
__dbx_link, as explained by:

#ifdef SYS_syssgi
  /* On mips-irix, we need to stop the inferior early enough during
     the startup phase in order to be able to load the shared library
     symbols and insert the breakpoints that are located in these shared
     libraries.  Stopping at the program entry point is not good enough
     because the -init code is executed before the execution reaches
     that point.

     So what we need to do is to insert a breakpoint in the runtime
     loader (rld), more precisely in __dbx_link().  This procedure is
     called by rld once all shared libraries have been mapped, but before
     the -init code is executed.  Unfortuantely, this is not straightforward,
     as rld is not part of the executable we are running, and thus we need
     the inferior to run until rld itself has been mapped in memory.

     For this, we trace all syssgi() syscall exit events.  Each time
     we detect such an event, we iterate over each text memory maps,
     get its associated fd, and scan the symbol table for __dbx_link().
     When found, we know that rld has been mapped, and that we can insert
     the breakpoint at the symbol address.  Once the dbx_link() breakpoint
     has been inserted, the syssgi() notifications are no longer necessary,
     so they should be canceled.  */
  proc_trace_syscalls_1 (pi, SYS_syssgi, PR_SYSEXIT, FLAG_SET, 0);
#endif

The loop in irix_solib_create_inferior_hook then runs until whichever
breakpoint is hit first, the one set by solib-irix.c or the one set by
procfs.c.

Note the comment in disable_break talks about __dbx_init, but I think
that's a typo for __dbx_link:

 -  /* Note that it is possible that we have stopped at a location that
 -     is different from the location where we inserted our breakpoint.
 -     On mips-irix, we can actually land in __dbx_init(), so we should
 -     not check the PC against our breakpoint address here.  See procfs.c
 -     for more details.  */

This looks very much like referring to the loop in
irix_solib_create_inferior_hook stopping at __dbx_link instead of at
the entry point.

What this patch does is convert these deprecated raw breakpoints to
standard solib_event breakpoints.  When the first solib-event
breakpoint is hit, we delete all solib-event breakpoints.  We do that
in the so_ops->handle_event hook.

This allows getting rid of the loop in irix_solib_create_inferior_hook
completely, which should allow properly handling signals and other
events in the early startup phase, like in SVR4.

Built on x86_64 Fedora 20 with --enable-targets=all (builds
solib-irix.c), but otherwise completely untested.

gdb/ChangeLog:
2014-09-10  Pedro Alves  <palves@redhat.com>

	* procfs.c (dbx_link_bpt_addr, dbx_link_bpt): Delete globals.
	(remove_dbx_link_breakpoint): Delete function.
	(insert_dbx_link_bpt_in_file): Use create_solib_event_breakpoint
	instead of deprecated_insert_raw_breakpoint.  Don't check whether
	we hit __dbx_link here.
	(procfs_mourn_inferior): Don't delete the __dbx_link breakpoint
	here.
	* solib-irix.c (base_breakpoint): Delete global.
	(disable_break): Delete function.
	(enable_break): Use create_solib_event_breakpoint
	instead of deprecated_insert_raw_breakpoint.
	(irix_solib_handle_event): New function.
	(irix_solib_create_inferior_hook): Don't run the target or disable
	the mapping-complete breakpoint here.
	(_initialize_irix_solib): Install irix_solib_handle_event as
	so_ops->handle_event hook.
---
 gdb/procfs.c     |  51 +++++----------------------
 gdb/solib-irix.c | 104 +++++++++++++++----------------------------------------
 2 files changed, 35 insertions(+), 120 deletions(-)

diff --git a/gdb/procfs.c b/gdb/procfs.c
index 3465bc5..37ef05c 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2902,13 +2902,6 @@ static void do_detach (int signo);
 static void proc_trace_syscalls_1 (procinfo *pi, int syscallnum,
 				   int entry_or_exit, int mode, int from_tty);
 
-/* On mips-irix, we need to insert a breakpoint at __dbx_link during
-   the startup phase.  The following two variables are used to record
-   the address of the breakpoint, and the code that was replaced by
-   a breakpoint.  */
-static int dbx_link_bpt_addr = 0;
-static void *dbx_link_bpt;
-
 /* Sets up the inferior to be debugged.  Registers to trace signals,
    hardware faults, and syscalls.  Note: does not set RLC flag: caller
    may want to customize that.  Returns zero for success (note!
@@ -3380,23 +3373,6 @@ syscall_is_lwp_create (procinfo *pi, int scall)
   return 0;
 }
 
-/* Remove the breakpoint that we inserted in __dbx_link().
-   Does nothing if the breakpoint hasn't been inserted or has already
-   been removed.  */
-
-static void
-remove_dbx_link_breakpoint (void)
-{
-  if (dbx_link_bpt_addr == 0)
-    return;
-
-  if (deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt) != 0)
-    warning (_("Unable to remove __dbx_link breakpoint."));
-
-  dbx_link_bpt_addr = 0;
-  dbx_link_bpt = NULL;
-}
-
 #ifdef SYS_syssgi
 /* Return the address of the __dbx_link() function in the file
    refernced by ABFD by scanning its symbol table.  Return 0 if
@@ -3461,13 +3437,17 @@ insert_dbx_link_bpt_in_file (int fd, CORE_ADDR ignored)
   sym_addr = dbx_link_addr (abfd);
   if (sym_addr != 0)
     {
+      struct breakpoint *dbx_link_bpt;
+
       /* Insert the breakpoint.  */
-      dbx_link_bpt_addr = sym_addr;
-      dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL,
-						       sym_addr);
-      if (dbx_link_bpt == NULL)
+      dbx_link_bpt = create_solib_event_breakpoint (target_gdbarch (), sym_addr);
+
+      if (!breakpoints_always_inserted_mode ())
+	insert_breakpoint_locations ();
+      if (!dbx_link_bpt->loc->inserted)
 	{
 	  warning (_("Failed to insert dbx_link breakpoint."));
+	  delete_breakpoint (dbx_link_bpt);
 	  gdb_bfd_unref (abfd);
 	  return 0;
 	}
@@ -3894,14 +3874,6 @@ wait_again:
 #if (FLTTRACE != FLTBPT)	/* Avoid "duplicate case" error.  */
 		case FLTTRACE:
 #endif
-		  /* If we hit our __dbx_link() internal breakpoint,
-		     then remove it.  See comments in procfs_init_inferior()
-		     for more details.	*/
-		  if (dbx_link_bpt_addr != 0
-		      && dbx_link_bpt_addr
-			 == regcache_read_pc (get_current_regcache ()))
-		    remove_dbx_link_breakpoint ();
-
 		  wstat = (SIGTRAP << 8) | 0177;
 		  break;
 		case FLTSTACK:
@@ -4340,13 +4312,6 @@ procfs_mourn_inferior (struct target_ops *ops)
 
   generic_mourn_inferior ();
 
-  if (dbx_link_bpt != NULL)
-    {
-      deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt);
-      dbx_link_bpt_addr = 0;
-      dbx_link_bpt = NULL;
-    }
-
   inf_child_maybe_unpush_target (ops);
 }
 
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 12ed766..e6fada9 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -240,8 +240,6 @@ fetch_lm_info (CORE_ADDR addr)
 /* The symbol which starts off the list of shared libraries.  */
 #define DEBUG_BASE "__rld_obj_head"
 
-static void *base_breakpoint;
-
 static CORE_ADDR debug_base;	/* Base of dynamic linker structures.  */
 
 /* Locate the base address of dynamic linker structs.
@@ -293,34 +291,6 @@ locate_base (void)
   return (address);
 }
 
-/* Remove the "mapping changed" breakpoint.
-
-   Removes the breakpoint that gets hit when the dynamic linker
-   completes a mapping change.  */
-
-static int
-disable_break (void)
-{
-  int status = 1;
-
-  /* Note that breakpoint address and original contents are in our address
-     space, so we just need to write the original contents back.  */
-
-  if (deprecated_remove_raw_breakpoint (target_gdbarch (), base_breakpoint) != 0)
-    {
-      status = 0;
-    }
-
-  base_breakpoint = NULL;
-
-  /* Note that it is possible that we have stopped at a location that
-     is different from the location where we inserted our breakpoint.
-     On mips-irix, we can actually land in __dbx_init(), so we should
-     not check the PC against our breakpoint address here.  See procfs.c
-     for more details.  */
-
-  return (status);
-}
 
 /* Arrange for dynamic linker to hit breakpoint.
 
@@ -332,23 +302,35 @@ enable_break (void)
 {
   if (symfile_objfile != NULL && has_stack_frames ())
     {
-      struct frame_info *frame = get_current_frame ();
-      struct address_space *aspace = get_frame_address_space (frame);
       CORE_ADDR entry_point;
 
-      if (!entry_point_address_query (&entry_point))
-	return 0;
-
-      base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (),
-							  aspace, entry_point);
-
-      if (base_breakpoint != NULL)
-	return 1;
+      if (entry_point_address_query (&entry_point))
+	{
+	  create_solib_event_breakpoint (target_gdbarch (), entry_point);
+	  return 1;
+	}
     }
 
   return 0;
 }
 
+/* Implement the "handle_event" target_solib_ops method.  */
+
+static void
+irix_solib_handle_event (void)
+{
+  /* We are now at the "mapping complete" breakpoint, we no longer
+     need it.  Note that it is possible that we have stopped at a
+     location that is different from the location where we inserted
+     our breakpoint: On mips-irix, we can actually land in
+     __dbx_link(), so we should not check the PC against our
+     breakpoint address here.  See procfs.c for more details.  */
+  remove_solib_event_breakpoints ();
+
+  /* The caller calls solib_add, which will add any shared libraries
+     that were mapped in.  */
+}
+
 /* Implement the "create_inferior_hook" target_solib_ops method.
 
    For SunOS executables, this first instruction is typically the
@@ -409,43 +391,10 @@ irix_solib_create_inferior_hook (int from_tty)
       return;
     }
 
-  /* Now run the target.  It will eventually hit the breakpoint, at
-     which point all of the libraries will have been mapped in and we
-     can go groveling around in the dynamic linker structures to find
-     out what we need to know about them.  */
-
-  tp = inferior_thread ();
-
-  clear_proceed_status (0);
-
-  inf->control.stop_soon = STOP_QUIETLY;
-  tp->suspend.stop_signal = GDB_SIGNAL_0;
-
-  do
-    {
-      target_resume (pid_to_ptid (-1), 0, tp->suspend.stop_signal);
-      wait_for_inferior ();
-    }
-  while (tp->suspend.stop_signal != GDB_SIGNAL_TRAP);
-
-  /* We are now either at the "mapping complete" breakpoint (or somewhere
-     else, a condition we aren't prepared to deal with anyway), so adjust
-     the PC as necessary after a breakpoint, disable the breakpoint, and
-     add any shared libraries that were mapped in.  */
-
-  if (!disable_break ())
-    {
-      warning (_("shared library handler failed to disable breakpoint"));
-    }
-
-  /* solib_add will call reinit_frame_cache.
-     But we are stopped in the startup code and we might not have symbols
-     for the startup code, so heuristic_proc_start could be called
-     and will put out an annoying warning.
-     Delaying the resetting of stop_soon until after symbol loading
-     suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
-  inf->control.stop_soon = NO_STOP_QUIETLY;
+  /* The target will eventually hit the breakpoint, at which point all
+     of the libraries will have been mapped in and we can go groveling
+     around in the dynamic linker structures to find out what we need
+     to know about them.  */
 }
 
 /* Implement the "current_sos" target_so_ops method.  */
@@ -653,4 +602,5 @@ _initialize_irix_solib (void)
   irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
   irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
   irix_so_ops.bfd_open = solib_bfd_open;
+  irix_so_ops.handle_event = irix_solib_handle_event;
 }
-- 
1.9.3

Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Joel Brobecker]

> I think I understand this enough now.  See patch below, which
> gets rid of that loop completely.

Thanks for walking me by memory lane :-)

> Untested, of course.

I found an IRIX machine that's still up and running, and I was able
to rebuild it. Unfortunately, two issues occured:
  1. I had to make insert_breakpoint_locations non-static;
  2. GDB now SIGSEGV-s when starting a program:

Program received signal SIGSEGV, Segmentation fault.
0x10163180 in bpstat_stop_status (aspace=0x108b97b0, bp_addr=268791248, 
    ptid=..., ws=0x7ffd7870)
    at /nfs/nas/homes/brobecke/act/gdb-public/gdb/breakpoint.c:5536
5536          b->ops->check_status (bs);

The problem is that b->ops is NULL. I can continue investigating,
but I have this feeling that this is just a waste of time, since
I strongly suspect that IRIX support is going to be removed
a couple of weeks from now (I can start the procedure if you'd like).

-- 
Joel

Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Pedro Alves]

On 09/10/2014 11:43 PM, Joel Brobecker wrote:
>> I think I understand this enough now.  See patch below, which
>> gets rid of that loop completely.
> 
> Thanks for walking me by memory lane :-)
> 

:-)

>> Untested, of course.
> 
> I found an IRIX machine that's still up and running, and I was able
> to rebuild it. Unfortunately, two issues occured:
>   1. I had to make insert_breakpoint_locations non-static;
>   2. GDB now SIGSEGV-s when starting a program:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x10163180 in bpstat_stop_status (aspace=0x108b97b0, bp_addr=268791248, 
>     ptid=..., ws=0x7ffd7870)
>     at /nfs/nas/homes/brobecke/act/gdb-public/gdb/breakpoint.c:5536
> 5536          b->ops->check_status (bs);
> 
> The problem is that b->ops is NULL. 

Blah.  Probably due to wiping the solib breakpoints in
the so_ops->handle_event() handler.

If you have the patience, could you try quickly commenting out
the remove_solib_event_breakpoints call in solib-irix.c and retrying?

> I can continue investigating,
> but I have this feeling that this is just a waste of time, since

Yeah...

> I strongly suspect that IRIX support is going to be removed
> a couple of weeks from now (I can start the procedure if you'd like).

That'd be super and much appreciated!

Thanks,
Pedro Alves

Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Joel Brobecker]

> Blah.  Probably due to wiping the solib breakpoints in
> the so_ops->handle_event() handler.
> 
> If you have the patience, could you try quickly commenting out
> the remove_solib_event_breakpoints call in solib-irix.c and retrying?

Small glitch that got me kicked out of the machine, but I eventually
manage to run that experiment, and I was able to debug again.
"info shared" after starting a program gave the same list of
shared libraries.

> > I strongly suspect that IRIX support is going to be removed
> > a couple of weeks from now (I can start the procedure if you'd like).
> 
> That'd be super and much appreciated!

OK - I will add that to my list and try to get to it tomorrow.
I don't think I'll be as specific as Dan was, if that's OK.
I think I'll just announce the impending removal of support
for mips-irix and alpha-tru64 in GDB. We can figure out what
to remove when we get there...

-- 
Joel

Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Pedro Alves]

On 09/11/2014 04:27 AM, Joel Brobecker wrote:
>> Blah.  Probably due to wiping the solib breakpoints in
>> the so_ops->handle_event() handler.
>>
>> If you have the patience, could you try quickly commenting out
>> the remove_solib_event_breakpoints call in solib-irix.c and retrying?
> 
> Small glitch that got me kicked out of the machine, but I eventually
> manage to run that experiment, and I was able to debug again.
> "info shared" after starting a program gave the same list of
> shared libraries.

Thanks Joel.  I've addressed that issue by marking the breakpoints
as disp_del_at_next_stop instead of deleting them immediately.

Locally, I added a call to remove_solib_event_breakpoints_at_next_stop
to solib-svr4.c:svr4_handle_solib_event, to check that the breakpoint
does indeed end up removed, and that we don't crash.  Adding a
call to remove_solib_event_breakpoints instead would crash in the
same way you saw on IRIX with the previous version of this patch.

I avoided exposing insert_breakpoint_locations out of breakpoint.c by
adding a new create_and_insert_solib_event_breakpoint function
that is like create_solib_event_breakpoint but tries to insert the
breakpoint immediately.

I figure that if someone wants to keep IRIX alive (either now, by
bringing it back from git), this will be a better base to start
from.

So in interest of not having to wait for IRIX to be dropped,
I've gone ahead and pushed the patch below.

Thanks.

---------
[IRIX] eliminate deprecated_insert_raw_breakpoint uses

The IRIX support wants to set a breakpoint to be hit when the startup
phase is complete, which is where shared libraries have been mapped
in.  AFAIU, for most IRIX ports, that location is the entry point.

For MIPS IRIX however, GDB needs to set a breakpoint earlier, in
__dbx_link, as explained by:

 #ifdef SYS_syssgi
   /* On mips-irix, we need to stop the inferior early enough during
      the startup phase in order to be able to load the shared library
      symbols and insert the breakpoints that are located in these shared
      libraries.  Stopping at the program entry point is not good enough
      because the -init code is executed before the execution reaches
      that point.

      So what we need to do is to insert a breakpoint in the runtime
      loader (rld), more precisely in __dbx_link().  This procedure is
      called by rld once all shared libraries have been mapped, but before
      the -init code is executed.  Unfortuantely, this is not straightforward,
      as rld is not part of the executable we are running, and thus we need
      the inferior to run until rld itself has been mapped in memory.

      For this, we trace all syssgi() syscall exit events.  Each time
      we detect such an event, we iterate over each text memory maps,
      get its associated fd, and scan the symbol table for __dbx_link().
      When found, we know that rld has been mapped, and that we can insert
      the breakpoint at the symbol address.  Once the dbx_link() breakpoint
      has been inserted, the syssgi() notifications are no longer necessary,
      so they should be canceled.  */
   proc_trace_syscalls_1 (pi, SYS_syssgi, PR_SYSEXIT, FLAG_SET, 0);
 #endif

The loop in irix_solib_create_inferior_hook then runs until whichever
breakpoint is hit first, the one set by solib-irix.c or the one set by
procfs.c.

Note the comment in disable_break talks about __dbx_init, but I think
that's a typo for __dbx_link:

 -  /* Note that it is possible that we have stopped at a location that
 -     is different from the location where we inserted our breakpoint.
 -     On mips-irix, we can actually land in __dbx_init(), so we should
 -     not check the PC against our breakpoint address here.  See procfs.c
 -     for more details.  */

This looks very much like referring to the loop in
irix_solib_create_inferior_hook stopping at __dbx_link instead of at
the entry point.

What this patch does is convert these deprecated raw breakpoints to
standard solib_event breakpoints.  When the first solib-event
breakpoint is hit, we delete all solib-event breakpoints.  We do that
in the so_ops->handle_event hook.

This allows getting rid of the loop in irix_solib_create_inferior_hook
completely, which should allow properly handling signals and other
events in the early startup phase, like in SVR4.

Built on x86_64 Fedora 20 with --enable-targets=all (builds
solib-irix.c).

Joel tested that with an earlier version of this patch "info shared"
after starting a program gave the same list of shared libraries as
before.

gdb/ChangeLog:
2014-09-12  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (remove_solib_event_breakpoints_at_next_stop)
	(create_and_insert_solib_event_breakpoint): New functions.
	* breakpoint.h (create_and_insert_solib_event_breakpoint)
	(remove_solib_event_breakpoints_at_next_stop): New declarations.
	* procfs.c (dbx_link_bpt_addr, dbx_link_bpt): Delete globals.
	(remove_dbx_link_breakpoint): Delete function.
	(insert_dbx_link_bpt_in_file): Use
	create_and_insert_solib_event_breakpoint instead of
	deprecated_insert_raw_breakpoint.
	(procfs_wait): Don't check whether we hit __dbx_link here.
	(procfs_mourn_inferior): Don't delete the __dbx_link breakpoint
	here.
	* solib-irix.c (base_breakpoint): Delete global.
	(disable_break): Delete function.
	(enable_break): Use create_solib_event_breakpoint
	instead of deprecated_insert_raw_breakpoint.
	(irix_solib_handle_event): New function.
	(irix_solib_create_inferior_hook): Don't run the target or disable
	the mapping-complete breakpoint here.
	(_initialize_irix_solib): Install irix_solib_handle_event as
	so_ops->handle_event hook.
---
 gdb/ChangeLog    |  24 +++++++++++++
 gdb/breakpoint.c |  31 ++++++++++++++++
 gdb/breakpoint.h |  11 ++++++
 gdb/procfs.c     |  47 +++---------------------
 gdb/solib-irix.c | 108 ++++++++++++++++---------------------------------------
 5 files changed, 102 insertions(+), 119 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0ec71d4..0b9e140 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,27 @@
+2014-09-12  Pedro Alves  <palves@redhat.com>
+
+	* breakpoint.c (remove_solib_event_breakpoints_at_next_stop)
+	(create_and_insert_solib_event_breakpoint): New functions.
+	* breakpoint.h (create_and_insert_solib_event_breakpoint)
+	(remove_solib_event_breakpoints_at_next_stop): New declarations.
+	* procfs.c (dbx_link_bpt_addr, dbx_link_bpt): Delete globals.
+	(remove_dbx_link_breakpoint): Delete function.
+	(insert_dbx_link_bpt_in_file): Use
+	create_and_insert_solib_event_breakpoint instead of
+	deprecated_insert_raw_breakpoint.
+	(procfs_wait): Don't check whether we hit __dbx_link here.
+	(procfs_mourn_inferior): Don't delete the __dbx_link breakpoint
+	here.
+	* solib-irix.c (base_breakpoint): Delete global.
+	(disable_break): Delete function.
+	(enable_break): Use create_solib_event_breakpoint
+	instead of deprecated_insert_raw_breakpoint.
+	(irix_solib_handle_event): New function.
+	(irix_solib_create_inferior_hook): Don't run the target or disable
+	the mapping-complete breakpoint here.
+	(_initialize_irix_solib): Install irix_solib_handle_event as
+	so_ops->handle_event hook.
+
 2014-09-12  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
 	    Ulrich Weigand  <uweigand@de.ibm.com>

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 683ed2b..f990d97 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7620,6 +7620,19 @@ remove_solib_event_breakpoints (void)
       delete_breakpoint (b);
 }

+/* See breakpoint.h.  */
+
+void
+remove_solib_event_breakpoints_at_next_stop (void)
+{
+  struct breakpoint *b, *b_tmp;
+
+  ALL_BREAKPOINTS_SAFE (b, b_tmp)
+    if (b->type == bp_shlib_event
+	&& b->loc->pspace == current_program_space)
+      b->disposition = disp_del_at_next_stop;
+}
+
 struct breakpoint *
 create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 {
@@ -7631,6 +7644,24 @@ create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
   return b;
 }

+/* See breakpoint.h.  */
+
+struct breakpoint *
+create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
+{
+  struct breakpoint *b;
+
+  b = create_solib_event_breakpoint (gdbarch, address);
+  if (!breakpoints_always_inserted_mode ())
+    insert_breakpoint_locations ();
+  if (!b->loc->inserted)
+    {
+      delete_breakpoint (b);
+      return NULL;
+    }
+  return b;
+}
+
 /* Disable any breakpoints that are on code in shared libraries.  Only
    apply to enabled breakpoints, disabled ones can just stay disabled.  */

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index f6d06ce..8abb5ea 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1419,6 +1419,13 @@ extern struct breakpoint *create_jit_event_breakpoint (struct gdbarch *,
 extern struct breakpoint *create_solib_event_breakpoint (struct gdbarch *,
 							 CORE_ADDR);

+/* Create an solib event breakpoint at ADDRESS in the current program
+   space, and immediately try to insert it.  Returns a pointer to the
+   breakpoint on success.  Deletes the new breakpoint and returns NULL
+   if inserting the breakpoint fails.  */
+extern struct breakpoint *create_and_insert_solib_event_breakpoint
+  (struct gdbarch *gdbarch, CORE_ADDR address);
+
 extern struct breakpoint *create_thread_event_breakpoint (struct gdbarch *,
 							  CORE_ADDR);

@@ -1426,6 +1433,10 @@ extern void remove_jit_event_breakpoints (void);

 extern void remove_solib_event_breakpoints (void);

+/* Mark solib event breakpoints of the current program space with
+   delete at next stop disposition.  */
+extern void remove_solib_event_breakpoints_at_next_stop (void);
+
 extern void remove_thread_event_breakpoints (void);

 extern void disable_breakpoints_in_shlibs (void);
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 3465bc5..699fcd9 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2902,13 +2902,6 @@ static void do_detach (int signo);
 static void proc_trace_syscalls_1 (procinfo *pi, int syscallnum,
 				   int entry_or_exit, int mode, int from_tty);

-/* On mips-irix, we need to insert a breakpoint at __dbx_link during
-   the startup phase.  The following two variables are used to record
-   the address of the breakpoint, and the code that was replaced by
-   a breakpoint.  */
-static int dbx_link_bpt_addr = 0;
-static void *dbx_link_bpt;
-
 /* Sets up the inferior to be debugged.  Registers to trace signals,
    hardware faults, and syscalls.  Note: does not set RLC flag: caller
    may want to customize that.  Returns zero for success (note!
@@ -3380,23 +3373,6 @@ syscall_is_lwp_create (procinfo *pi, int scall)
   return 0;
 }

-/* Remove the breakpoint that we inserted in __dbx_link().
-   Does nothing if the breakpoint hasn't been inserted or has already
-   been removed.  */
-
-static void
-remove_dbx_link_breakpoint (void)
-{
-  if (dbx_link_bpt_addr == 0)
-    return;
-
-  if (deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt) != 0)
-    warning (_("Unable to remove __dbx_link breakpoint."));
-
-  dbx_link_bpt_addr = 0;
-  dbx_link_bpt = NULL;
-}
-
 #ifdef SYS_syssgi
 /* Return the address of the __dbx_link() function in the file
    refernced by ABFD by scanning its symbol table.  Return 0 if
@@ -3461,10 +3437,12 @@ insert_dbx_link_bpt_in_file (int fd, CORE_ADDR ignored)
   sym_addr = dbx_link_addr (abfd);
   if (sym_addr != 0)
     {
+      struct breakpoint *dbx_link_bpt;
+
       /* Insert the breakpoint.  */
-      dbx_link_bpt_addr = sym_addr;
-      dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL,
-						       sym_addr);
+      dbx_link_bpt
+	= create_and_insert_solib_event_breakpoint (target_gdbarch (),
+						    sym_addr);
       if (dbx_link_bpt == NULL)
 	{
 	  warning (_("Failed to insert dbx_link breakpoint."));
@@ -3894,14 +3872,6 @@ wait_again:
 #if (FLTTRACE != FLTBPT)	/* Avoid "duplicate case" error.  */
 		case FLTTRACE:
 #endif
-		  /* If we hit our __dbx_link() internal breakpoint,
-		     then remove it.  See comments in procfs_init_inferior()
-		     for more details.	*/
-		  if (dbx_link_bpt_addr != 0
-		      && dbx_link_bpt_addr
-			 == regcache_read_pc (get_current_regcache ()))
-		    remove_dbx_link_breakpoint ();
-
 		  wstat = (SIGTRAP << 8) | 0177;
 		  break;
 		case FLTSTACK:
@@ -4340,13 +4310,6 @@ procfs_mourn_inferior (struct target_ops *ops)

   generic_mourn_inferior ();

-  if (dbx_link_bpt != NULL)
-    {
-      deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt);
-      dbx_link_bpt_addr = 0;
-      dbx_link_bpt = NULL;
-    }
-
   inf_child_maybe_unpush_target (ops);
 }

diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 12ed766..00236bf 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -240,8 +240,6 @@ fetch_lm_info (CORE_ADDR addr)
 /* The symbol which starts off the list of shared libraries.  */
 #define DEBUG_BASE "__rld_obj_head"

-static void *base_breakpoint;
-
 static CORE_ADDR debug_base;	/* Base of dynamic linker structures.  */

 /* Locate the base address of dynamic linker structs.
@@ -293,34 +291,6 @@ locate_base (void)
   return (address);
 }

-/* Remove the "mapping changed" breakpoint.
-
-   Removes the breakpoint that gets hit when the dynamic linker
-   completes a mapping change.  */
-
-static int
-disable_break (void)
-{
-  int status = 1;
-
-  /* Note that breakpoint address and original contents are in our address
-     space, so we just need to write the original contents back.  */
-
-  if (deprecated_remove_raw_breakpoint (target_gdbarch (), base_breakpoint) != 0)
-    {
-      status = 0;
-    }
-
-  base_breakpoint = NULL;
-
-  /* Note that it is possible that we have stopped at a location that
-     is different from the location where we inserted our breakpoint.
-     On mips-irix, we can actually land in __dbx_init(), so we should
-     not check the PC against our breakpoint address here.  See procfs.c
-     for more details.  */
-
-  return (status);
-}

 /* Arrange for dynamic linker to hit breakpoint.

@@ -332,23 +302,39 @@ enable_break (void)
 {
   if (symfile_objfile != NULL && has_stack_frames ())
     {
-      struct frame_info *frame = get_current_frame ();
-      struct address_space *aspace = get_frame_address_space (frame);
       CORE_ADDR entry_point;

-      if (!entry_point_address_query (&entry_point))
-	return 0;
-
-      base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (),
-							  aspace, entry_point);
-
-      if (base_breakpoint != NULL)
-	return 1;
+      if (entry_point_address_query (&entry_point))
+	{
+	  create_solib_event_breakpoint (target_gdbarch (), entry_point);
+	  return 1;
+	}
     }

   return 0;
 }

+/* Implement the "handle_event" target_solib_ops method.  */
+
+static void
+irix_solib_handle_event (void)
+{
+  /* We are now at the "mapping complete" breakpoint, we no longer
+     need it.  Note that it is possible that we have stopped at a
+     location that is different from the location where we inserted
+     our breakpoint: On mips-irix, we can actually land in
+     __dbx_link(), so we should not check the PC against our
+     breakpoint address here.  See procfs.c for more details.  Note
+     we're being called by the bpstat handling code, and so can't
+     delete the breakpoint immediately.  Mark it for later deletion,
+     which has the same effect (it'll be removed before we next resume
+     or if we're stopping).  */
+  remove_solib_event_breakpoints_at_next_stop ();
+
+  /* The caller calls solib_add, which will add any shared libraries
+     that were mapped in.  */
+}
+
 /* Implement the "create_inferior_hook" target_solib_ops method.

    For SunOS executables, this first instruction is typically the
@@ -409,43 +395,10 @@ irix_solib_create_inferior_hook (int from_tty)
       return;
     }

-  /* Now run the target.  It will eventually hit the breakpoint, at
-     which point all of the libraries will have been mapped in and we
-     can go groveling around in the dynamic linker structures to find
-     out what we need to know about them.  */
-
-  tp = inferior_thread ();
-
-  clear_proceed_status (0);
-
-  inf->control.stop_soon = STOP_QUIETLY;
-  tp->suspend.stop_signal = GDB_SIGNAL_0;
-
-  do
-    {
-      target_resume (pid_to_ptid (-1), 0, tp->suspend.stop_signal);
-      wait_for_inferior ();
-    }
-  while (tp->suspend.stop_signal != GDB_SIGNAL_TRAP);
-
-  /* We are now either at the "mapping complete" breakpoint (or somewhere
-     else, a condition we aren't prepared to deal with anyway), so adjust
-     the PC as necessary after a breakpoint, disable the breakpoint, and
-     add any shared libraries that were mapped in.  */
-
-  if (!disable_break ())
-    {
-      warning (_("shared library handler failed to disable breakpoint"));
-    }
-
-  /* solib_add will call reinit_frame_cache.
-     But we are stopped in the startup code and we might not have symbols
-     for the startup code, so heuristic_proc_start could be called
-     and will put out an annoying warning.
-     Delaying the resetting of stop_soon until after symbol loading
-     suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
-  inf->control.stop_soon = NO_STOP_QUIETLY;
+  /* The target will eventually hit the breakpoint, at which point all
+     of the libraries will have been mapped in and we can go groveling
+     around in the dynamic linker structures to find out what we need
+     to know about them.  */
 }

 /* Implement the "current_sos" target_so_ops method.  */
@@ -653,4 +606,5 @@ _initialize_irix_solib (void)
   irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
   irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
   irix_so_ops.bfd_open = solib_bfd_open;
+  irix_so_ops.handle_event = irix_solib_handle_event;
 }
-- 
1.9.3

Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses

[Joel Brobecker]

> Thanks Joel.  I've addressed that issue by marking the breakpoints
> as disp_del_at_next_stop instead of deleting them immediately.
> 
> Locally, I added a call to remove_solib_event_breakpoints_at_next_stop
> to solib-svr4.c:svr4_handle_solib_event, to check that the breakpoint
> does indeed end up removed, and that we don't crash.  Adding a
> call to remove_solib_event_breakpoints instead would crash in the
> same way you saw on IRIX with the previous version of this patch.
> 
> I avoided exposing insert_breakpoint_locations out of breakpoint.c by
> adding a new create_and_insert_solib_event_breakpoint function
> that is like create_solib_event_breakpoint but tries to insert the
> breakpoint immediately.
> 
> I figure that if someone wants to keep IRIX alive (either now, by
> bringing it back from git), this will be a better base to start
> from.
> 
> So in interest of not having to wait for IRIX to be dropped,
> I've gone ahead and pushed the patch below.

I've just rebuilt GDB again, and I can confirm that the problem
is fixed. After starting a program, I can still see the same
list of shared libraries as before your change.

Cheers!

-- 
Joel

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Stan Shebs]

On 9/10/14, 7:45 AM, Ulrich Weigand wrote:

> The process documented in the Wiki is a bit weird (adding OBSOLETE
> to *every line* of those files ???) and it seems we didn't follow
> it in the last major round of obsoleting code either:
> https://sourceware.org/ml/gdb-announce/2007/msg00000.html

Well, my thought was to forestall grep'ers from being misled by the
presence of code that was slated to go away... it perhaps made more
sense in the pre-public-repo age, when many developers hacked on
copies of releases or snapshots, rather than a checkout.

Stan
stan@codesourcery.com

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Pedro Alves]

On 09/08/2014 06:46 PM, Pedro Alves wrote:
> Hey,
> 
> I'm working on a patch that converts software single-step breakpoints
> to real breakpoints.  It's largely done, but needs a bit of cleaning up
> before I'll post it.  One issue is that software single-step breakpoints
> were implemented on top of deprecated_insert_raw_breakpoint.  They no
> longer are after my patch, but we're still left with some ugly code in
> breakpoint.c related to deprecated_insert_raw_breakpoints on top of other
> breakpoints that I'd like to eliminate completely, but that can only
> be done once all users of deprecated_insert_raw_breakpoint are either converted
> to some more modern mechanism, or eliminated.

Here's my current WIP patch.  This runs regression free on x86_64 Fedora 20,
and also on top of my software-single-step-on-x86 series.  I've tested this on
ppc64 F18 as well, to cover the step-over-watchpoints changes.

The ultimate goal was to eliminate a bunch of globals and make it possible
to have multiple threads in parallel doing software single-steps (it works!).
Currently in non-stop, on software single-step targets, we force all
single-steps as displaced-stepping operations, which means single-step requests
are serialized, because there's only one scratch pad.

Note this also removes the limitation that you can only have at most two
software single-step breakpoints, because each thread's single sss
breakpoint can hold an arbitrary number of locations.

As mentioned before, this removes deprecated_insert_raw_breakpoint
completely, which we can't so while AIX and IRIX aren't cleaned up.
Or maybe we can.  :-)

If something horrible jumps back at you, please shout!  :-)

---
From c62b21283aef99bd495aea9b6c6680a4219ca2b2 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 9 Sep 2014 17:28:45 +0100
Subject: [PATCH] make single-step breakpoints real breakpoints

---
 gdb/breakpoint.c  | 418 ++++++++++++++----------------------------------------
 gdb/breakpoint.h  |  33 ++---
 gdb/gdbthread.h   |  25 ++++
 gdb/infrun.c      | 360 +++++++++++++++++++++++-----------------------
 gdb/infrun.h      |   4 +
 gdb/record-full.c |   8 +-
 gdb/thread.c      |  73 +++++++---
 7 files changed, 387 insertions(+), 534 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 683ed2b..93f4ad2 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -226,11 +226,6 @@ static void stopat_command (char *arg, int from_tty);
 
 static void tcatch_command (char *arg, int from_tty);
 
-static void detach_single_step_breakpoints (void);
-
-static int find_single_step_breakpoint (struct address_space *aspace,
-					CORE_ADDR pc);
-
 static void free_bp_location (struct bp_location *loc);
 static void incref_bp_location (struct bp_location *loc);
 static void decref_bp_location (struct bp_location **loc);
@@ -296,12 +291,6 @@ static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 /* Dynamic printf class type.  */
 struct breakpoint_ops dprintf_breakpoint_ops;
 
-/* One (or perhaps two) breakpoints used for software single
-   stepping.  */
-
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
-
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
    if GDB does the printing, there is better error handling if there
@@ -1616,21 +1605,6 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
     one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
 				memaddr, len, &bl->target_info, bl->gdbarch);
   }
-
-  /* Now process single-step breakpoints.  These are not found in the
-     bp_location array.  */
-  for (i = 0; i < 2; i++)
-    {
-      struct bp_target_info *bp_tgt = single_step_breakpoints[i];
-
-      if (bp_tgt != NULL)
-	{
-	  struct gdbarch *gdbarch = single_step_gdbarch[i];
-
-	  one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
-				      memaddr, len, bp_tgt, gdbarch);
-	}
-    }
 }
 
 \f
@@ -2089,7 +2063,32 @@ should_be_inserted (struct bp_location *bl)
        || bl->loc_type == bp_loc_hardware_breakpoint)
       && stepping_past_instruction_at (bl->pspace->aspace,
 				       bl->address))
-    return 0;
+    {
+      if (debug_infrun)
+	{
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: skipping breakpoint: "
+			      "stepping past insn at: %s\n",
+			      paddress (bl->gdbarch, bl->address));
+	}
+      return 0;
+    }
+
+  /* Don't insert watchpoints if we're trying to step past the
+     instruction that triggered one.  */
+  if ((bl->loc_type == bp_loc_hardware_watchpoint)
+      && stepping_past_nonsteppable_watchpoint ())
+    {
+      if (debug_infrun)
+	{
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: stepping past non-steppable watchpoint. "
+			      "skipping watchpoint at %s:%d\n",
+			      paddress (bl->gdbarch, bl->address),
+			      bl->length);
+	}
+      return 0;
+    }
 
   return 1;
 }
@@ -3753,9 +3752,6 @@ detach_breakpoints (ptid_t ptid)
       val |= remove_breakpoint_1 (bl, mark_inserted);
   }
 
-  /* Detach single-step breakpoints as well.  */
-  detach_single_step_breakpoints ();
-
   do_cleanups (old_chain);
   return val;
 }
@@ -4025,6 +4021,10 @@ breakpoint_init_inferior (enum inf_context context)
 
 	/* Also remove step-resume breakpoints.  */
 
+      case bp_single_step:
+
+	/* Also remove single-step breakpoints.  */
+
 	delete_breakpoint (b);
 	break;
 
@@ -4121,14 +4121,10 @@ moribund_breakpoint_here_p (struct address_space *aspace, CORE_ADDR pc)
   return 0;
 }
 
-/* Returns non-zero if there's a breakpoint inserted at PC, which is
-   inserted using regular breakpoint_chain / bp_location array
-   mechanism.  This does not check for single-step breakpoints, which
-   are inserted and removed using direct target manipulation.  */
+/* Returns non-zero iff there's a breakpoint inserted at PC.  */
 
 int
-regular_breakpoint_inserted_here_p (struct address_space *aspace, 
-				    CORE_ADDR pc)
+breakpoint_inserted_here_p (struct address_space *aspace, CORE_ADDR pc)
 {
   struct bp_location *bl, **blp_tmp;
 
@@ -4152,27 +4148,12 @@ regular_breakpoint_inserted_here_p (struct address_space *aspace,
   return 0;
 }
 
-/* Returns non-zero iff there's either regular breakpoint
-   or a single step breakpoint inserted at PC.  */
+/* This function returns non-zero iff there is a software breakpoint
+   inserted at PC.  */
 
 int
-breakpoint_inserted_here_p (struct address_space *aspace, CORE_ADDR pc)
-{
-  if (regular_breakpoint_inserted_here_p (aspace, pc))
-    return 1;
-
-  if (single_step_breakpoint_inserted_here_p (aspace, pc))
-    return 1;
-
-  return 0;
-}
-
-/* Ignoring deprecated raw breakpoints, return non-zero iff there is a
-   software breakpoint inserted at PC.  */
-
-static struct bp_location *
-find_non_raw_software_breakpoint_inserted_here (struct address_space *aspace,
-						CORE_ADDR pc)
+software_breakpoint_inserted_here_p (struct address_space *aspace,
+				     CORE_ADDR pc)
 {
   struct bp_location *bl, **blp_tmp;
 
@@ -4190,27 +4171,10 @@ find_non_raw_software_breakpoint_inserted_here (struct address_space *aspace,
 	      && !section_is_mapped (bl->section))
 	    continue;		/* unmapped overlay -- can't be a match */
 	  else
-	    return bl;
+	    return 1;
 	}
     }
 
-  return NULL;
-}
-
-/* This function returns non-zero iff there is a software breakpoint
-   inserted at PC.  */
-
-int
-software_breakpoint_inserted_here_p (struct address_space *aspace,
-				     CORE_ADDR pc)
-{
-  if (find_non_raw_software_breakpoint_inserted_here (aspace, pc) != NULL)
-    return 1;
-
-  /* Also check for software single-step breakpoints.  */
-  if (single_step_breakpoint_inserted_here_p (aspace, pc))
-    return 1;
-
   return 0;
 }
 
@@ -5498,6 +5462,7 @@ bpstat_stop_status (struct address_space *aspace,
 	}
     }
 
+  /* Check if a moribund breakpoint explains the stop.  */
   for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
     {
       if (breakpoint_location_address_match (loc, aspace, bp_addr))
@@ -5653,6 +5618,7 @@ bpstat_what (bpstat bs_head)
 	  break;
 	case bp_breakpoint:
 	case bp_hardware_breakpoint:
+	case bp_single_step:
 	case bp_until:
 	case bp_finish:
 	case bp_shlib_event:
@@ -6011,6 +5977,7 @@ bptype_string (enum bptype type)
     {bp_none, "?deleted?"},
     {bp_breakpoint, "breakpoint"},
     {bp_hardware_breakpoint, "hw breakpoint"},
+    {bp_single_step, "sw single-step"},
     {bp_until, "until"},
     {bp_finish, "finish"},
     {bp_watchpoint, "watchpoint"},
@@ -6202,6 +6169,7 @@ print_one_breakpoint_location (struct breakpoint *b,
 
       case bp_breakpoint:
       case bp_hardware_breakpoint:
+      case bp_single_step:
       case bp_until:
       case bp_finish:
       case bp_longjmp:
@@ -7080,6 +7048,7 @@ init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops,
   switch (owner->type)
     {
     case bp_breakpoint:
+    case bp_single_step:
     case bp_until:
     case bp_finish:
     case bp_longjmp:
@@ -9020,10 +8989,31 @@ enable_breakpoints_after_startup (void)
   breakpoint_re_set ();
 }
 
+/* Create a new single-step breakpoint for thread THREAD, with no
+   locations.  */
+
+static struct breakpoint *
+new_single_step_breakpoint (int thread, struct gdbarch *gdbarch)
+{
+  struct breakpoint *b = XNEW (struct breakpoint);
+
+  init_raw_breakpoint_without_location (b, gdbarch, bp_single_step,
+					&momentary_breakpoint_ops);
 
-/* Set a breakpoint that will evaporate an end of command
-   at address specified by SAL.
-   Restrict it to frame FRAME if FRAME is nonzero.  */
+  b->disposition = disp_donttouch;
+  b->frame_id = null_frame_id;
+
+  b->thread = thread;
+  gdb_assert (b->thread != 0);
+
+  add_to_breakpoint_chain (b);
+
+  return b;
+}
+
+/* Set a momentary breakpoint of type TYPE at address specified by
+   SAL.  If FRAME_ID is valid, the breakpoint is restricted to that
+   frame.  */
 
 struct breakpoint *
 set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
@@ -13129,45 +13119,13 @@ bkpt_re_set (struct breakpoint *b)
   breakpoint_re_set_default (b);
 }
 
-/* Copy SRC's shadow buffer and whatever else we'd set if we actually
-   inserted DEST, so we can remove it later, in case SRC is removed
-   first.  */
-
-static void
-bp_target_info_copy_insertion_state (struct bp_target_info *dest,
-				     const struct bp_target_info *src)
-{
-  dest->shadow_len = src->shadow_len;
-  memcpy (dest->shadow_contents, src->shadow_contents, src->shadow_len);
-  dest->placed_size = src->placed_size;
-}
-
 static int
 bkpt_insert_location (struct bp_location *bl)
 {
   if (bl->loc_type == bp_loc_hardware_breakpoint)
-    return target_insert_hw_breakpoint (bl->gdbarch,
-					&bl->target_info);
+    return target_insert_hw_breakpoint (bl->gdbarch, &bl->target_info);
   else
-    {
-      struct bp_target_info *bp_tgt = &bl->target_info;
-      int ret;
-      int sss_slot;
-
-      /* There is no need to insert a breakpoint if an unconditional
-	 raw/sss breakpoint is already inserted at that location.  */
-      sss_slot = find_single_step_breakpoint (bp_tgt->placed_address_space,
-					      bp_tgt->placed_address);
-      if (sss_slot >= 0)
-	{
-	  struct bp_target_info *sss_bp_tgt = single_step_breakpoints[sss_slot];
-
-	  bp_target_info_copy_insertion_state (bp_tgt, sss_bp_tgt);
-	  return 0;
-	}
-
-      return target_insert_breakpoint (bl->gdbarch, bp_tgt);
-    }
+    return target_insert_breakpoint (bl->gdbarch, &bl->target_info);
 }
 
 static int
@@ -13176,19 +13134,7 @@ bkpt_remove_location (struct bp_location *bl)
   if (bl->loc_type == bp_loc_hardware_breakpoint)
     return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info);
   else
-    {
-      struct bp_target_info *bp_tgt = &bl->target_info;
-      struct address_space *aspace = bp_tgt->placed_address_space;
-      CORE_ADDR address = bp_tgt->placed_address;
-
-      /* Only remove the breakpoint if there is no raw/sss breakpoint
-	 still inserted at this location.  Otherwise, we would be
-	 effectively disabling the raw/sss breakpoint.  */
-      if (single_step_breakpoint_inserted_here_p (aspace, address))
-	return 0;
-
-      return target_remove_breakpoint (bl->gdbarch, bp_tgt);
-    }
+    return target_remove_breakpoint (bl->gdbarch, &bl->target_info);
 }
 
 static int
@@ -15186,217 +15132,73 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
       }
 }
 
-/* Create and insert a raw software breakpoint at PC.  Return an
-   identifier, which should be used to remove the breakpoint later.
-   In general, places which call this should be using something on the
-   breakpoint chain instead; this function should be eliminated
-   someday.  */
-
-void *
-deprecated_insert_raw_breakpoint (struct gdbarch *gdbarch,
-				  struct address_space *aspace, CORE_ADDR pc)
-{
-  struct bp_target_info *bp_tgt;
-  struct bp_location *bl;
-
-  bp_tgt = XCNEW (struct bp_target_info);
-
-  bp_tgt->placed_address_space = aspace;
-  bp_tgt->placed_address = pc;
-
-  /* If an unconditional non-raw breakpoint is already inserted at
-     that location, there's no need to insert another.  However, with
-     target-side evaluation of breakpoint conditions, if the
-     breakpoint that is currently inserted on the target is
-     conditional, we need to make it unconditional.  Note that a
-     breakpoint with target-side commands is not reported even if
-     unconditional, so we need to remove the commands from the target
-     as well.  */
-  bl = find_non_raw_software_breakpoint_inserted_here (aspace, pc);
-  if (bl != NULL
-      && VEC_empty (agent_expr_p, bl->target_info.conditions)
-      && VEC_empty (agent_expr_p, bl->target_info.tcommands))
-    {
-      bp_target_info_copy_insertion_state (bp_tgt, &bl->target_info);
-      return bp_tgt;
-    }
-
-  if (target_insert_breakpoint (gdbarch, bp_tgt) != 0)
-    {
-      /* Could not insert the breakpoint.  */
-      xfree (bp_tgt);
-      return NULL;
-    }
-
-  return bp_tgt;
-}
-
-/* Remove a breakpoint BP inserted by
-   deprecated_insert_raw_breakpoint.  */
-
-int
-deprecated_remove_raw_breakpoint (struct gdbarch *gdbarch, void *bp)
-{
-  struct bp_target_info *bp_tgt = bp;
-  struct address_space *aspace = bp_tgt->placed_address_space;
-  CORE_ADDR address = bp_tgt->placed_address;
-  struct bp_location *bl;
-  int ret;
-
-  bl = find_non_raw_software_breakpoint_inserted_here (aspace, address);
-
-  /* Only remove the raw breakpoint if there are no other non-raw
-     breakpoints still inserted at this location.  Otherwise, we would
-     be effectively disabling those breakpoints.  */
-  if (bl == NULL)
-    ret = target_remove_breakpoint (gdbarch, bp_tgt);
-  else if (!VEC_empty (agent_expr_p, bl->target_info.conditions)
-	   || !VEC_empty (agent_expr_p, bl->target_info.tcommands))
-    {
-      /* The target is evaluating conditions, and when we inserted the
-	 software single-step breakpoint, we had made the breakpoint
-	 unconditional and command-less on the target side.  Reinsert
-	 to restore the conditions/commands.  */
-      ret = target_insert_breakpoint (bl->gdbarch, &bl->target_info);
-    }
-  else
-    ret = 0;
-
-  xfree (bp_tgt);
-
-  return ret;
-}
-
-/* Create and insert a breakpoint for software single step.  */
+/* See breakpoint.h.  */
 
 void
 insert_single_step_breakpoint (struct gdbarch *gdbarch,
 			       struct address_space *aspace, 
 			       CORE_ADDR next_pc)
 {
-  void **bpt_p;
+  struct thread_info *tp = inferior_thread ();
+  struct symtab_and_line sal;
+  CORE_ADDR pc = next_pc;
 
-  if (single_step_breakpoints[0] == NULL)
-    {
-      bpt_p = &single_step_breakpoints[0];
-      single_step_gdbarch[0] = gdbarch;
-    }
-  else
+  if (tp->control.single_step_breakpoints == NULL)
     {
-      gdb_assert (single_step_breakpoints[1] == NULL);
-      bpt_p = &single_step_breakpoints[1];
-      single_step_gdbarch[1] = gdbarch;
+      tp->control.single_step_breakpoints
+	= new_single_step_breakpoint (tp->num, gdbarch);
     }
 
-  /* NOTE drow/2006-04-11: A future improvement to this function would
-     be to only create the breakpoints once, and actually put them on
-     the breakpoint chain.  That would let us use set_raw_breakpoint.
-     We could adjust the addresses each time they were needed.  Doing
-     this requires corresponding changes elsewhere where single step
-     breakpoints are handled, however.  So, for now, we use this.  */
-
-  *bpt_p = deprecated_insert_raw_breakpoint (gdbarch, aspace, next_pc);
-  if (*bpt_p == NULL)
-    error (_("Could not insert single-step breakpoint at %s"),
-	     paddress (gdbarch, next_pc));
-}
-
-/* Check if the breakpoints used for software single stepping
-   were inserted or not.  */
-
-int
-single_step_breakpoints_inserted (void)
-{
-  return (single_step_breakpoints[0] != NULL
-          || single_step_breakpoints[1] != NULL);
-}
-
-/* Remove and delete any breakpoints used for software single step.  */
-
-void
-remove_single_step_breakpoints (void)
-{
-  gdb_assert (single_step_breakpoints[0] != NULL);
+  sal = find_pc_line (pc, 0);
+  sal.pc = pc;
+  sal.section = find_pc_overlay (pc);
+  sal.explicit_pc = 1;
+  add_location_to_breakpoint (tp->control.single_step_breakpoints, &sal);
 
-  /* See insert_single_step_breakpoint for more about this deprecated
-     call.  */
-  deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
-				    single_step_breakpoints[0]);
-  single_step_gdbarch[0] = NULL;
-  single_step_breakpoints[0] = NULL;
+  update_global_location_list_nothrow (1);
 
-  if (single_step_breakpoints[1] != NULL)
-    {
-      deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
-					single_step_breakpoints[1]);
-      single_step_gdbarch[1] = NULL;
-      single_step_breakpoints[1] = NULL;
-    }
+  /* update_global_location_list does not insert breakpoints when
+     always_inserted_mode is not enabled.  Explicitly insert them
+     now.  */
+  if (!breakpoints_always_inserted_mode ())
+    insert_breakpoint_locations ();
 }
 
-/* Delete software single step breakpoints without removing them from
-   the inferior.  This is intended to be used if the inferior's address
-   space where they were inserted is already gone, e.g. after exit or
-   exec.  */
+/* See breakpoint.h.  */
 
-void
-cancel_single_step_breakpoints (void)
+int
+breakpoint_is_inserted_here (struct breakpoint *bp,
+			     struct address_space *aspace, CORE_ADDR pc)
 {
-  int i;
-
-  for (i = 0; i < 2; i++)
-    if (single_step_breakpoints[i])
-      {
-	xfree (single_step_breakpoints[i]);
-	single_step_breakpoints[i] = NULL;
-	single_step_gdbarch[i] = NULL;
-      }
-}
-
-/* Detach software single-step breakpoints from INFERIOR_PTID without
-   removing them.  */
+  struct bp_location *loc;
 
-static void
-detach_single_step_breakpoints (void)
-{
-  int i;
+  for (loc = bp->loc; loc != NULL; loc = loc->next)
+    if (loc->inserted
+	&& breakpoint_location_address_match (loc, aspace, pc))
+      return 1;
 
-  for (i = 0; i < 2; i++)
-    if (single_step_breakpoints[i])
-      target_remove_breakpoint (single_step_gdbarch[i],
-				single_step_breakpoints[i]);
+  return 0;
 }
 
-/* Find the software single-step breakpoint that inserted at PC.
-   Returns its slot if found, and -1 if not found.  */
+/* See breakpoint.h.  */
 
-static int
-find_single_step_breakpoint (struct address_space *aspace,
-			     CORE_ADDR pc)
+int
+single_step_breakpoint_inserted_here_p (struct address_space *aspace,
+					CORE_ADDR pc)
 {
-  int i;
+  struct breakpoint *bpt;
 
-  for (i = 0; i < 2; i++)
+  ALL_BREAKPOINTS (bpt)
     {
-      struct bp_target_info *bp_tgt = single_step_breakpoints[i];
-      if (bp_tgt
-	  && breakpoint_address_match (bp_tgt->placed_address_space,
-				       bp_tgt->placed_address,
-				       aspace, pc))
-	return i;
-    }
-
-  return -1;
-}
+      struct bp_location *loc;
 
-/* Check whether a software single-step breakpoint is inserted at
-   PC.  */
+      if (bpt->type != bp_single_step)
+	continue;
 
-int
-single_step_breakpoint_inserted_here_p (struct address_space *aspace,
-					CORE_ADDR pc)
-{
-  return find_single_step_breakpoint (aspace, pc) >= 0;
+      if (breakpoint_is_inserted_here (bpt, aspace, pc))
+	return 1;
+    }
+  return 0;
 }
 
 /* Returns 0 if 'bp' is NOT a syscall catchpoint,
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index f6d06ce..5920181 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -47,18 +47,13 @@ struct linespec_sals;
 \f
 
 /* Type of breakpoint.  */
-/* FIXME In the future, we should fold all other breakpoint-like
-   things into here.  This includes:
-
-   * single-step (for machines where we have to simulate single
-   stepping) (probably, though perhaps it is better for it to look as
-   much as possible like a single-step to wait_for_inferior).  */
 
 enum bptype
   {
     bp_none = 0,		/* Eventpoint has been deleted */
     bp_breakpoint,		/* Normal breakpoint */
     bp_hardware_breakpoint,	/* Hardware assisted breakpoint */
+    bp_single_step,		/* Software single-step */
     bp_until,			/* used by until command */
     bp_finish,			/* used by finish command */
     bp_watchpoint,		/* Watchpoint */
@@ -1126,6 +1121,15 @@ extern int regular_breakpoint_inserted_here_p (struct address_space *,
 extern int software_breakpoint_inserted_here_p (struct address_space *, 
 						CORE_ADDR);
 
+/* Check whether any location of BP is inserted at PC.  */
+
+extern int breakpoint_is_inserted_here (struct breakpoint *bp,
+					struct address_space *aspace,
+					CORE_ADDR pc);
+
+/* Check whether any software single-step breakpoint is inserted at
+   PC.  */
+
 extern int single_step_breakpoint_inserted_here_p (struct address_space *,
 						   CORE_ADDR);
 
@@ -1442,22 +1446,13 @@ extern void add_solib_catchpoint (char *arg, int is_load, int is_temp,
    deletes all breakpoints.  */
 extern void delete_command (char *arg, int from_tty);
 
-/* Manage a software single step breakpoint (or two).  Insert may be
-   called twice before remove is called.  */
+/* Insert a new software single step breakpoint for the current
+   thread.  Insert may be called multiple times; each time will add a
+   new location to the set of potential addresses the next instruction
+   is at.  */
 extern void insert_single_step_breakpoint (struct gdbarch *,
 					   struct address_space *, 
 					   CORE_ADDR);
-extern int single_step_breakpoints_inserted (void);
-extern void remove_single_step_breakpoints (void);
-extern void cancel_single_step_breakpoints (void);
-
-/* Manage manual breakpoints, separate from the normal chain of
-   breakpoints.  These functions are used in murky target-specific
-   ways.  Please do not add more uses!  */
-extern void *deprecated_insert_raw_breakpoint (struct gdbarch *,
-					       struct address_space *, 
-					       CORE_ADDR);
-extern int deprecated_remove_raw_breakpoint (struct gdbarch *, void *);
 
 /* Check if any hardware watchpoints have triggered, according to the
    target.  */
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 522b674..77bbbde 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -52,6 +52,13 @@ struct thread_control_state
   /* Exception-resume breakpoint.  */
   struct breakpoint *exception_resume_breakpoint;
 
+  /* Breakpoints used for software single stepping.  Plural, because
+     it may have multiple locations.  E.g., if stepping over a
+     conditional branch instruction we can't decode the condition for,
+     we'll need to put a breakpoint at the branch destination, and
+     another at the instruction after the branch.  */
+  struct breakpoint *single_step_breakpoints;
+
   /* Range to single step within.
 
      If this is nonzero, respond to a single-step signal by continuing
@@ -197,6 +204,11 @@ struct thread_info
   /* Should we step over breakpoint next time keep_going is called?  */
   int stepping_over_breakpoint;
 
+  /* Should we step over a watchpoint next time keep_going is called?
+     This is needed on targets with non-continuable and non-steppable
+     watchpoints.  */
+  int stepping_over_watchpoint;
+
   /* Set to TRUE if we should finish single-stepping over a breakpoint
      after hitting the current step-resume breakpoint.  The context here
      is that GDB is to do `next' or `step' while signal arrives.
@@ -280,6 +292,19 @@ extern void delete_step_resume_breakpoint (struct thread_info *);
 /* Delete an exception_resume_breakpoint from the thread database.  */
 extern void delete_exception_resume_breakpoint (struct thread_info *);
 
+/* Delete the single-step breakpoints of thread TP, if any.  */
+extern void delete_single_step_breakpoints (struct thread_info *tp);
+
+/* Check if the thread has software single stepping breakpoints
+   set.  */
+extern int thread_has_single_step_breakpoints_set (struct thread_info *tp);
+
+/* Check whether the thread has software single stepping breakpoints
+   set at PC.  */
+extern int thread_has_single_step_breakpoint_here (struct thread_info *tp,
+						   struct address_space *aspace,
+						   CORE_ADDR addr);
+
 /* Translate the integer thread id (GDB's homegrown id, not the system's)
    into a "pid" (which may be overloaded with extra thread information).  */
 extern ptid_t thread_id_to_pid (int);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c18267f..2f3a848 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -373,8 +373,6 @@ static void context_switch (ptid_t ptid);
 
 void init_thread_stepping_state (struct thread_info *tss);
 
-static void init_infwait_state (void);
-
 static const char follow_fork_mode_child[] = "child";
 static const char follow_fork_mode_parent[] = "parent";
 
@@ -844,6 +842,7 @@ follow_exec (ptid_t pid, char *execd_pathname)
      statement through an exec().  */
   th->control.step_resume_breakpoint = NULL;
   th->control.exception_resume_breakpoint = NULL;
+  th->control.single_step_breakpoints = NULL;
   th->control.step_range_start = 0;
   th->control.step_range_end = 0;
 
@@ -957,17 +956,6 @@ follow_exec (ptid_t pid, char *execd_pathname)
      matically get reset there in the new process.).  */
 }
 
-/* Non-zero if we just simulating a single-step.  This is needed
-   because we cannot remove the breakpoints in the inferior process
-   until after the `wait' in `wait_for_inferior'.  */
-static int singlestep_breakpoints_inserted_p = 0;
-
-/* The thread we inserted single-step breakpoints for.  */
-static ptid_t singlestep_ptid;
-
-/* PC when we started this single-step.  */
-static CORE_ADDR singlestep_pc;
-
 /* Info about an instruction that is being stepped over.  Invalid if
    ASPACE is NULL.  */
 
@@ -978,6 +966,10 @@ struct step_over_info
 
   /* The instruction's address.  */
   CORE_ADDR address;
+
+  /* The instruction being stepped over triggers a nonsteppable
+     watchpoint.  */
+  int nonsteppable_watchpoint;
 };
 
 /* The step-over info of the location that is being stepped over.
@@ -1010,10 +1002,12 @@ static struct step_over_info step_over_info;
    stepping over.  */
 
 static void
-set_step_over_info (struct address_space *aspace, CORE_ADDR address)
+set_step_over_info (struct address_space *aspace, CORE_ADDR address,
+		    int nonsteppable_watchpoint)
 {
   step_over_info.aspace = aspace;
   step_over_info.address = address;
+  step_over_info.nonsteppable_watchpoint = nonsteppable_watchpoint;
 }
 
 /* Called when we're not longer stepping over a breakpoint / an
@@ -1024,6 +1018,7 @@ clear_step_over_info (void)
 {
   step_over_info.aspace = NULL;
   step_over_info.address = 0;
+  step_over_info.nonsteppable_watchpoint = 0;
 }
 
 /* See inferior.h.  */
@@ -1032,10 +1027,35 @@ int
 stepping_past_instruction_at (struct address_space *aspace,
 			      CORE_ADDR address)
 {
+  if (step_over_info.aspace != NULL
+      && breakpoint_address_match (aspace, address,
+				   step_over_info.aspace,
+				   step_over_info.address))
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: stepping over breakpoint's address: %s\n",
+			    paddress (target_gdbarch (), address));
+      return 1;
+    }
+
+  return 0;
+}
+
+int
+stepping_past_nonsteppable_watchpoint (void)
+{
+  return step_over_info.nonsteppable_watchpoint;
+}
+
+/* Returns true if any breakpoint/watchpoint that would normally be
+   inserted can't be inserted due to run control.  */
+
+static int
+any_breakpoint_lifted (void)
+{
   return (step_over_info.aspace != NULL
-	  && breakpoint_address_match (aspace, address,
-				       step_over_info.aspace,
-				       step_over_info.address));
+	  || stepping_past_nonsteppable_watchpoint ());
 }
 
 \f
@@ -1633,9 +1653,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
   if (ptid_equal (inferior_ptid, old_ptid))
     inferior_ptid = new_ptid;
 
-  if (ptid_equal (singlestep_ptid, old_ptid))
-    singlestep_ptid = new_ptid;
-
   for (displaced = displaced_step_inferior_states;
        displaced;
        displaced = displaced->next)
@@ -1694,29 +1711,17 @@ set_schedlock_func (char *args, int from_tty, struct cmd_list_element *c)
    process.  */
 int sched_multi = 0;
 
-/* Try to setup for software single stepping over the specified location.
-   Return 1 if target_resume() should use hardware single step.
-
-   GDBARCH the current gdbarch.
-   PC the location to step over.  */
+/* Try to setup for software single stepping over the specified
+   location.  Return 0 if target_resume() should use hardware single
+   step.  GDBARCH is the current frame's gdbarch.  PC is the location
+   to step over.  */
 
 static int
 maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  int hw_step = 1;
-
-  if (execution_direction == EXEC_FORWARD
-      && gdbarch_software_single_step_p (gdbarch)
-      && gdbarch_software_single_step (gdbarch, get_current_frame ()))
-    {
-      hw_step = 0;
-      /* Do not pull these breakpoints until after a `wait' in
-	 `wait_for_inferior'.  */
-      singlestep_breakpoints_inserted_p = 1;
-      singlestep_ptid = inferior_ptid;
-      singlestep_pc = pc;
-    }
-  return hw_step;
+  return (execution_direction == EXEC_FORWARD
+	  && gdbarch_software_single_step_p (gdbarch)
+	  && gdbarch_software_single_step (gdbarch, get_current_frame ()));
 }
 
 ptid_t
@@ -1739,8 +1744,7 @@ user_visible_resume_ptid (int step)
       resume_ptid = inferior_ptid;
     }
   else if ((scheduler_mode == schedlock_on)
-	   || (scheduler_mode == schedlock_step
-	       && (step || singlestep_breakpoints_inserted_p)))
+	   || (scheduler_mode == schedlock_step && step))
     {
       /* User-settable 'scheduler' mode requires solo thread resume.  */
       resume_ptid = inferior_ptid;
@@ -1843,8 +1847,7 @@ a command like `return' or `jump' to continue execution."));
      event, displaced stepping breaks the vfork child similarly as single
      step software breakpoint.  */
   if (use_displaced_stepping (gdbarch)
-      && (tp->control.trap_expected
-	  || (step && gdbarch_software_single_step_p (gdbarch)))
+      && tp->control.trap_expected
       && sig == GDB_SIGNAL_0
       && !current_inferior ()->waiting_for_vfork_done)
     {
@@ -1877,7 +1880,7 @@ a command like `return' or `jump' to continue execution."));
 
   /* Do we need to do it the hard way, w/temp breakpoints?  */
   else if (step)
-    step = maybe_software_singlestep (gdbarch, pc);
+    step = !maybe_software_singlestep (gdbarch, pc);
 
   /* Currently, our software single-step implementation leads to different
      results than hardware single-stepping in one situation: when stepping
@@ -1903,9 +1906,17 @@ a command like `return' or `jump' to continue execution."));
      at the current address, deliver the signal without stepping, and
      once we arrive back at the step-resume breakpoint, actually step
      over the breakpoint we originally wanted to step over.  */
-  if (singlestep_breakpoints_inserted_p
-      && tp->control.trap_expected && sig != GDB_SIGNAL_0)
+  if (thread_has_single_step_breakpoints_set (tp)
+      && sig != GDB_SIGNAL_0
+      && any_breakpoint_lifted ())
     {
+      if (debug_infrun)
+	{
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: delivering signal with breakpoints lifted. "
+			      "replacing sss bp and letting handler run\n");
+	}
+
       /* If we have nested signals or a pending signal is delivered
 	 immediately after a handler returns, might might already have
 	 a step-resume breakpoint set on the earlier handler.  We cannot
@@ -1917,8 +1928,7 @@ a command like `return' or `jump' to continue execution."));
 	  tp->step_after_step_resume_breakpoint = 1;
 	}
 
-      remove_single_step_breakpoints ();
-      singlestep_breakpoints_inserted_p = 0;
+      delete_single_step_breakpoints (tp);
 
       clear_step_over_info ();
       tp->control.trap_expected = 0;
@@ -1929,7 +1939,7 @@ a command like `return' or `jump' to continue execution."));
   /* If STEP is set, it's a request to use hardware stepping
      facilities.  But in that case, we should never
      use singlestep breakpoint.  */
-  gdb_assert (!(singlestep_breakpoints_inserted_p && step));
+  gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
 
   /* Decide the set of threads to ask the target to resume.  Start
      by assuming everything will be resumed, than narrow the set
@@ -1945,7 +1955,7 @@ a command like `return' or `jump' to continue execution."));
     set_running (resume_ptid, 1);
 
   /* Maybe resume a single thread after all.  */
-  if ((step || singlestep_breakpoints_inserted_p)
+  if ((step || thread_has_single_step_breakpoints_set (tp))
       && tp->control.trap_expected)
     {
       /* We're allowing a thread to run past a breakpoint it has
@@ -1998,13 +2008,10 @@ a command like `return' or `jump' to continue execution."));
   tp->suspend.stop_signal = GDB_SIGNAL_0;
 
   /* Advise target which signals may be handled silently.  If we have
-     removed breakpoints because we are stepping over one (which can
-     happen only if we are not using displaced stepping), we need to
-     receive all signals to avoid accidentally skipping a breakpoint
-     during execution of a signal handler.  */
-  if ((step || singlestep_breakpoints_inserted_p)
-      && tp->control.trap_expected
-      && !use_displaced_stepping (gdbarch))
+     removed breakpoints, we need to receive all signals to avoid
+     accidentally skipping a breakpoint during execution of a signal
+     handler.  */
+  if (any_breakpoint_lifted ())
     target_pass_signals (0, NULL);
   else
     target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass);
@@ -2310,7 +2317,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
       struct regcache *regcache = get_current_regcache ();
 
       set_step_over_info (get_regcache_aspace (regcache),
-			  regcache_read_pc (regcache));
+			  regcache_read_pc (regcache), 0);
     }
   else
     clear_step_over_info ();
@@ -2350,9 +2357,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
      correctly when the inferior is stopped.  */
   tp->prev_pc = regcache_read_pc (get_current_regcache ());
 
-  /* Reset to normal state.  */
-  init_infwait_state ();
-
   /* Resume inferior.  */
   resume (tp->control.trap_expected || step || bpstat_should_step (),
 	  tp->suspend.stop_signal);
@@ -2417,13 +2421,9 @@ init_wait_for_inferior (void)
   target_last_wait_ptid = minus_one_ptid;
 
   previous_inferior_ptid = inferior_ptid;
-  init_infwait_state ();
 
   /* Discard any skipped inlined frames.  */
   clear_inline_frame_state (minus_one_ptid);
-
-  singlestep_ptid = null_ptid;
-  singlestep_pc = 0;
 }
 
 \f
@@ -2438,9 +2438,6 @@ enum infwait_states
   infwait_nonstep_watch_state
 };
 
-/* The PTID we'll do a target_wait on.*/
-ptid_t waiton_ptid;
-
 /* Current inferior wait state.  */
 static enum infwait_states infwait_state;
 
@@ -2460,11 +2457,6 @@ struct execution_control_state
   const char *stop_func_name;
   int wait_some_more;
 
-  /* We were in infwait_step_watch_state or
-     infwait_nonstep_watch_state state, and the thread reported an
-     event.  */
-  int stepped_after_stopped_by_watchpoint;
-
   /* True if the event thread hit the single-step breakpoint of
      another thread.  Thus the event doesn't cause a stop, the thread
      needs to be single-stepped past the single-step breakpoint before
@@ -2609,6 +2601,7 @@ delete_step_resume_breakpoint_callback (struct thread_info *info, void *data)
 
   delete_step_resume_breakpoint (info);
   delete_exception_resume_breakpoint (info);
+  delete_single_step_breakpoints (info);
   return 0;
 }
 
@@ -2634,6 +2627,7 @@ delete_step_thread_step_resume_breakpoint (void)
 
       delete_step_resume_breakpoint (tp);
       delete_exception_resume_breakpoint (tp);
+      delete_single_step_breakpoints (tp);
     }
   else
     /* In all-stop mode, delete all step-resume and longjmp-resume
@@ -2641,6 +2635,39 @@ delete_step_thread_step_resume_breakpoint (void)
     iterate_over_threads (delete_step_resume_breakpoint_callback, NULL);
 }
 
+static void
+delete_stopped_threads_single_step_breakpoints (void)
+{
+  if (!target_has_execution
+      || ptid_equal (inferior_ptid, null_ptid))
+    {
+      /* If the inferior has exited, we have already deleted the
+	 single-step breakpoints out of GDB's lists.  */
+      return;
+    }
+
+  if (non_stop)
+    {
+      /* If in non-stop mode, only delete the step-resume or
+	 longjmp-resume breakpoint of the thread that just stopped
+	 stepping.  */
+      struct thread_info *tp = inferior_thread ();
+
+      delete_single_step_breakpoints (tp);
+    }
+  else
+    {
+      struct thread_info *tp;
+
+      ALL_NON_EXITED_THREADS (tp)
+        {
+	  /* In all-stop mode, delete single-step breakpoints of any
+	     thread that had them.  */
+	  delete_single_step_breakpoints (tp);
+	}
+    }
+}
+
 /* A cleanup wrapper.  */
 
 static void
@@ -2789,6 +2816,7 @@ wait_for_inferior (void)
       struct execution_control_state ecss;
       struct execution_control_state *ecs = &ecss;
       struct cleanup *old_chain;
+      ptid_t waiton_ptid = minus_one_ptid;
 
       memset (ecs, 0, sizeof (*ecs));
 
@@ -2844,6 +2872,7 @@ fetch_inferior_event (void *client_data)
   struct cleanup *ts_old_chain;
   int was_sync = sync_execution;
   int cmd_done = 0;
+  ptid_t waiton_ptid = minus_one_ptid;
 
   memset (ecs, 0, sizeof (*ecs));
 
@@ -2961,6 +2990,7 @@ void
 init_thread_stepping_state (struct thread_info *tss)
 {
   tss->stepping_over_breakpoint = 0;
+  tss->stepping_over_watchpoint = 0;
   tss->step_after_step_resume_breakpoint = 0;
 }
 
@@ -3120,7 +3150,7 @@ adjust_pc_after_break (struct execution_control_state *ecs)
 	 software breakpoint.  In this case (prev_pc == breakpoint_pc),
 	 we also need to back up to the breakpoint address.  */
 
-      if (singlestep_breakpoints_inserted_p
+      if (thread_has_single_step_breakpoints_set (ecs->event_thread)
 	  || !ptid_equal (ecs->ptid, inferior_ptid)
 	  || !currently_stepping (ecs->event_thread)
 	  || ecs->event_thread->prev_pc == breakpoint_pc)
@@ -3130,13 +3160,6 @@ adjust_pc_after_break (struct execution_control_state *ecs)
     }
 }
 
-static void
-init_infwait_state (void)
-{
-  waiton_ptid = pid_to_ptid (-1);
-  infwait_state = infwait_normal_state;
-}
-
 static int
 stepped_in_from (struct frame_info *frame, struct frame_id step_frame_id)
 {
@@ -3357,40 +3380,6 @@ handle_inferior_event (struct execution_control_state *ecs)
 	   && ecs->ws.kind != TARGET_WAITKIND_EXITED)
     set_executing (ecs->ptid, 0);
 
-  switch (infwait_state)
-    {
-    case infwait_normal_state:
-      if (debug_infrun)
-        fprintf_unfiltered (gdb_stdlog, "infrun: infwait_normal_state\n");
-      break;
-
-    case infwait_step_watch_state:
-      if (debug_infrun)
-        fprintf_unfiltered (gdb_stdlog,
-			    "infrun: infwait_step_watch_state\n");
-
-      ecs->stepped_after_stopped_by_watchpoint = 1;
-      break;
-
-    case infwait_nonstep_watch_state:
-      if (debug_infrun)
-        fprintf_unfiltered (gdb_stdlog,
-			    "infrun: infwait_nonstep_watch_state\n");
-      insert_breakpoints ();
-
-      /* FIXME-maybe: is this cleaner than setting a flag?  Does it
-         handle things like signals arriving and other things happening
-         in combination correctly?  */
-      ecs->stepped_after_stopped_by_watchpoint = 1;
-      break;
-
-    default:
-      internal_error (__FILE__, __LINE__, _("bad switch"));
-    }
-
-  infwait_state = infwait_normal_state;
-  waiton_ptid = pid_to_ptid (-1);
-
   switch (ecs->ws.kind)
     {
     case TARGET_WAITKIND_LOADED:
@@ -3548,8 +3537,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 
       gdb_flush (gdb_stdout);
       target_mourn_inferior ();
-      singlestep_breakpoints_inserted_p = 0;
-      cancel_single_step_breakpoints ();
       stop_print_frame = 0;
       stop_waiting (ecs);
       return;
@@ -3642,12 +3629,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  detach_breakpoints (ecs->ws.value.related_pid);
 	}
 
-      if (singlestep_breakpoints_inserted_p)
-	{
-	  /* Pull the single step breakpoints out of the target.  */
-	  remove_single_step_breakpoints ();
-	  singlestep_breakpoints_inserted_p = 0;
-	}
+      delete_stopped_threads_single_step_breakpoints ();
 
       /* In case the event is caught by a catchpoint, remember that
 	 the event is to be followed at the next resume of the thread,
@@ -3734,9 +3716,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       if (!ptid_equal (ecs->ptid, inferior_ptid))
 	context_switch (ecs->ptid);
 
-      singlestep_breakpoints_inserted_p = 0;
-      cancel_single_step_breakpoints ();
-
       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
 
       /* Do whatever is necessary to the parent branch of the vfork.  */
@@ -3802,14 +3781,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_NO_HISTORY\n");
       /* Reverse execution: target ran out of history info.  */
 
-      /* Pull the single step breakpoints out of the target.  */
-      if (singlestep_breakpoints_inserted_p)
-	{
-	  if (!ptid_equal (ecs->ptid, inferior_ptid))
-	    context_switch (ecs->ptid);
-	  remove_single_step_breakpoints ();
-	  singlestep_breakpoints_inserted_p = 0;
-	}
+      delete_stopped_threads_single_step_breakpoints ();
       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
       observer_notify_no_history ();
       stop_waiting (ecs);
@@ -3948,43 +3920,59 @@ handle_signal_stop (struct execution_control_state *ecs)
   gdbarch = get_frame_arch (frame);
 
   /* Pull the single step breakpoints out of the target.  */
-  if (singlestep_breakpoints_inserted_p)
+  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+      && gdbarch_software_single_step_p (gdbarch))
     {
+      struct regcache *regcache;
+      struct address_space *aspace;
+      CORE_ADDR pc;
+
+      regcache = get_thread_regcache (ecs->ptid);
+      aspace = get_regcache_aspace (regcache);
+      pc = regcache_read_pc (regcache);
+
       /* However, before doing so, if this single-step breakpoint was
 	 actually for another thread, set this thread up for moving
 	 past it.  */
-      if (!ptid_equal (ecs->ptid, singlestep_ptid)
-	  && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
+      if (!thread_has_single_step_breakpoint_here (ecs->event_thread,
+						   aspace, pc))
 	{
-	  struct regcache *regcache;
-	  struct address_space *aspace;
-	  CORE_ADDR pc;
-
-	  regcache = get_thread_regcache (ecs->ptid);
-	  aspace = get_regcache_aspace (regcache);
-	  pc = regcache_read_pc (regcache);
 	  if (single_step_breakpoint_inserted_here_p (aspace, pc))
 	    {
 	      if (debug_infrun)
 		{
 		  fprintf_unfiltered (gdb_stdlog,
-				      "infrun: [%s] hit step over single-step"
-				      " breakpoint of [%s]\n",
-				      target_pid_to_str (ecs->ptid),
-				      target_pid_to_str (singlestep_ptid));
+				      "infrun: [%s] hit another thread's "
+				      "single-step breakpoint\n",
+				      target_pid_to_str (ecs->ptid));
 		}
 	      ecs->hit_singlestep_breakpoint = 1;
 	    }
 	}
+      else
+	{
+	  if (debug_infrun)
+	    {
+	      fprintf_unfiltered (gdb_stdlog,
+				  "infrun: [%s] hit its "
+				  "single-step breakpoint\n",
+				  target_pid_to_str (ecs->ptid));
+	    }
+	}
 
-      remove_single_step_breakpoints ();
-      singlestep_breakpoints_inserted_p = 0;
+      delete_stopped_threads_single_step_breakpoints ();
     }
 
-  if (ecs->stepped_after_stopped_by_watchpoint)
-    stopped_by_watchpoint = 0;
+  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+      && ecs->event_thread->control.trap_expected
+      && ecs->event_thread->stepping_over_watchpoint)
+    {
+      stopped_by_watchpoint = 0;
+    }
   else
-    stopped_by_watchpoint = watchpoints_triggered (&ecs->ws);
+    {
+      stopped_by_watchpoint = watchpoints_triggered (&ecs->ws);
+    }
 
   /* If necessary, step over this watchpoint.  We'll be back to display
      it in a moment.  */
@@ -4004,36 +3992,28 @@ handle_signal_stop (struct execution_control_state *ecs)
          watchpoint expression.  We do this by single-stepping the
 	 target.
 
-	 It may not be necessary to disable the watchpoint to stop over
+	 It may not be necessary to disable the watchpoint to step over
 	 it.  For example, the PA can (with some kernel cooperation)
 	 single step over a watchpoint without disabling the watchpoint.
 
 	 It is far more common to need to disable a watchpoint to step
 	 the inferior over it.  If we have non-steppable watchpoints,
 	 we must disable the current watchpoint; it's simplest to
-	 disable all watchpoints and breakpoints.  */
-      int hw_step = 1;
-
-      if (!target_have_steppable_watchpoint)
-	{
-	  remove_breakpoints ();
-	  /* See comment in resume why we need to stop bypassing signals
-	     while breakpoints have been removed.  */
-	  target_pass_signals (0, NULL);
-	}
-	/* Single step */
-      hw_step = maybe_software_singlestep (gdbarch, stop_pc);
-      target_resume (ecs->ptid, hw_step, GDB_SIGNAL_0);
-      waiton_ptid = ecs->ptid;
-      if (target_have_steppable_watchpoint)
-	infwait_state = infwait_step_watch_state;
-      else
-	infwait_state = infwait_nonstep_watch_state;
-      prepare_to_wait (ecs);
+	 disable all watchpoints.
+
+	 Any breakpoint at PC must also be stepped over -- if there's
+	 one, it will have already triggered before the watchpoint
+	 triggered, and we either already reported it to the user, or
+	 it didn't cause a stop and we called keep_going.  In either
+	 case, if there was a breakpoint at PC, we must be trying to
+	 step past it.  */
+      ecs->event_thread->stepping_over_watchpoint = 1;
+      keep_going (ecs);
       return;
     }
 
   ecs->event_thread->stepping_over_breakpoint = 0;
+  ecs->event_thread->stepping_over_watchpoint = 0;
   bpstat_clear (&ecs->event_thread->control.stop_bpstat);
   ecs->event_thread->control.stop_step = 0;
   stop_print_frame = 1;
@@ -5311,15 +5291,21 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	    {
 	      if (debug_infrun)
 		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: expected thread advanced also\n");
+				    "infrun: expected thread advanced also: prev_pc=%s, stop_pc=%s\n",
+				    paddress (gdbarch, tp->prev_pc),
+				    paddress (gdbarch, stop_pc));
+
+	      /* Clear this before trying to insert the sss
+		 breakpoint, in case we were previously trying to step
+		 over this location in another thread, otherwise the
+		 breakpoint ends up _not_ installed.  It's what
+		 keep_going would do too, if we called it.  */
+	      clear_step_over_info ();
 
 	      insert_single_step_breakpoint (get_frame_arch (frame),
 					     get_frame_address_space (frame),
 					     stop_pc);
-	      singlestep_breakpoints_inserted_p = 1;
 	      ecs->event_thread->control.trap_expected = 1;
-	      singlestep_ptid = inferior_ptid;
-	      singlestep_pc = stop_pc;
 
 	      resume (0, GDB_SIGNAL_0);
 	      prepare_to_wait (ecs);
@@ -5769,6 +5755,8 @@ keep_going (struct execution_control_state *ecs)
     {
       volatile struct gdb_exception e;
       struct regcache *regcache = get_current_regcache ();
+      int remove_bp;
+      int remove_wps;
 
       /* Either the trap was not expected, but we are continuing
 	 anyway (if we got a signal, the user asked it be passed to
@@ -5788,13 +5776,19 @@ keep_going (struct execution_control_state *ecs)
 	 (watchpoints, etc.) but the one we're stepping over, step one
 	 instruction, and then re-insert the breakpoint when that step
 	 is finished.  */
-      if ((ecs->hit_singlestep_breakpoint
-	   || thread_still_needs_step_over (ecs->event_thread))
-	  && !use_displaced_stepping (get_regcache_arch (regcache)))
+
+      remove_bp = (ecs->hit_singlestep_breakpoint
+		   || thread_still_needs_step_over (ecs->event_thread));
+      remove_wps = (ecs->event_thread->stepping_over_watchpoint
+		    && !target_have_steppable_watchpoint);
+
+      if (remove_bp && !use_displaced_stepping (get_regcache_arch (regcache)))
 	{
 	  set_step_over_info (get_regcache_aspace (regcache),
-			      regcache_read_pc (regcache));
+			      regcache_read_pc (regcache), remove_wps);
 	}
+      else if (remove_wps)
+	set_step_over_info (NULL, 0, remove_wps);
       else
 	clear_step_over_info ();
 
@@ -5810,9 +5804,7 @@ keep_going (struct execution_control_state *ecs)
 	  return;
 	}
 
-      ecs->event_thread->control.trap_expected
-	= (ecs->event_thread->stepping_over_breakpoint
-	   || ecs->hit_singlestep_breakpoint);
+      ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
 
       /* Do not deliver GDB_SIGNAL_TRAP (except when the user
 	 explicitly specifies that such a signal should be delivered
diff --git a/gdb/infrun.h b/gdb/infrun.h
index cc9cb33..13127ed 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -122,6 +122,10 @@ extern void follow_inferior_reset_breakpoints (void);
 extern int stepping_past_instruction_at (struct address_space *aspace,
 					 CORE_ADDR address);
 
+/* Returns true if we're trying to step past an instruction that
+   triggers a non-steppable watchpoint.  */
+extern int stepping_past_nonsteppable_watchpoint (void);
+
 extern void set_step_info (struct frame_info *frame,
 			   struct symtab_and_line sal);
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 119361f..a2257ed 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -961,7 +961,7 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
           else
             {
               /* This arch support soft sigle step.  */
-              if (single_step_breakpoints_inserted ())
+              if (thread_has_single_step_breakpoints_set (inferior_thread ()))
                 {
                   /* This is a soft single step.  */
                   record_full_resume_step = 1;
@@ -1085,6 +1085,8 @@ record_full_wait_1 (struct target_ops *ops,
 
 	  while (1)
 	    {
+	      struct thread_info *tp;
+
 	      ret = ops->beneath->to_wait (ops->beneath, ptid, status, options);
 	      if (status->kind == TARGET_WAITKIND_IGNORE)
 		{
@@ -1095,8 +1097,8 @@ record_full_wait_1 (struct target_ops *ops,
 		  return ret;
 		}
 
-              if (single_step_breakpoints_inserted ())
-                remove_single_step_breakpoints ();
+	      ALL_NON_EXITED_THREADS (tp)
+                delete_single_step_breakpoints (tp);
 
 	      if (record_full_resume_step)
 		return ret;
diff --git a/gdb/thread.c b/gdb/thread.c
index 65890e1..1cdbaf4 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -85,26 +85,68 @@ inferior_thread (void)
   return tp;
 }
 
-void
-delete_step_resume_breakpoint (struct thread_info *tp)
+/* Delete the breakpoint pointed at by BP_P, if there's one.  */
+
+static void
+delete_thread_breakpoint (struct breakpoint **bp_p)
 {
-  if (tp && tp->control.step_resume_breakpoint)
+  if (*bp_p != NULL)
     {
-      delete_breakpoint (tp->control.step_resume_breakpoint);
-      tp->control.step_resume_breakpoint = NULL;
+      delete_breakpoint (*bp_p);
+      *bp_p = NULL;
     }
 }
 
 void
+delete_step_resume_breakpoint (struct thread_info *tp)
+{
+  if (tp != NULL)
+    delete_thread_breakpoint (&tp->control.step_resume_breakpoint);
+}
+
+void
 delete_exception_resume_breakpoint (struct thread_info *tp)
 {
-  if (tp && tp->control.exception_resume_breakpoint)
+  if (tp != NULL)
+    delete_thread_breakpoint (&tp->control.exception_resume_breakpoint);
+}
+
+void
+delete_single_step_breakpoints (struct thread_info *tp)
+{
+  if (tp != NULL)
+    delete_thread_breakpoint (&tp->control.single_step_breakpoints);
+}
+
+/* Delete the breakpoint pointed at by BP_P at the next stop, if
+   there's one.  */
+
+static void
+delete_at_next_stop (struct breakpoint **bp)
+{
+  if (*bp != NULL)
     {
-      delete_breakpoint (tp->control.exception_resume_breakpoint);
-      tp->control.exception_resume_breakpoint = NULL;
+      (*bp)->disposition = disp_del_at_next_stop;
+      *bp = NULL;
     }
 }
 
+int
+thread_has_single_step_breakpoints_set (struct thread_info *tp)
+{
+  return tp->control.single_step_breakpoints != NULL;
+}
+
+int
+thread_has_single_step_breakpoint_here (struct thread_info *tp,
+					struct address_space *aspace,
+					CORE_ADDR addr)
+{
+  return (tp->control.single_step_breakpoints != NULL
+	  && breakpoint_is_inserted_here (tp->control.single_step_breakpoints,
+					  aspace, addr));
+}
+
 static void
 clear_thread_inferior_resources (struct thread_info *tp)
 {
@@ -112,18 +154,9 @@ clear_thread_inferior_resources (struct thread_info *tp)
      but not any user-specified thread-specific breakpoints.  We can not
      delete the breakpoint straight-off, because the inferior might not
      be stopped at the moment.  */
-  if (tp->control.step_resume_breakpoint)
-    {
-      tp->control.step_resume_breakpoint->disposition = disp_del_at_next_stop;
-      tp->control.step_resume_breakpoint = NULL;
-    }
-
-  if (tp->control.exception_resume_breakpoint)
-    {
-      tp->control.exception_resume_breakpoint->disposition
-	= disp_del_at_next_stop;
-      tp->control.exception_resume_breakpoint = NULL;
-    }
+  delete_at_next_stop (&tp->control.step_resume_breakpoint);
+  delete_at_next_stop (&tp->control.exception_resume_breakpoint);
+  delete_at_next_stop (&tp->control.single_step_breakpoints);
 
   delete_longjmp_breakpoint_at_next_stop (tp->num);
 
-- 
1.9.3

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Maciej W. Rozycki wrote:

> On Wed, 10 Sep 2014, Ulrich Weigand wrote:
> > Once OSF/1 and IRIX are gone, I hope all of the ECOFF/mdebug debug
> > format support can go as well (mipsread.c, mdebugread.c etc.) ...
> 
>  Some of that stuff will best stay, to support Procedure Descriptor 
> Records used on MIPS ELF targets, including but not limited to Linux.  
> 
>  These records are the only way to get backtracing, and consequently any 
> reasonable control of the debuggee, to work from places that have debug 
> information stripped, such as often when you interrupt your debuggee while 
> waiting in a syscall (libc.so will often have no debug information 
> included, as usually won't other system-installed shared libraries).  
> Without that debugging is from my experience severely impeded -- you end 
> up in the middle of nowhere and virtually all you can do is `continue' or 
> `stepi', that'll in many cases merely put you back in the sleeping 
> syscall.
> 
>  All MIPS ELF binaries produced by the GNU toolchain carry these PDR 
> records along unless their exclusion has been explicitly requested from 
> GAS (which is not the default and in most cases undesirable, these records 
> are very lightweight and occupy little space).
> 
>  I have already identified `mdebugread.h' being the only piece required 
> though -- in addition to `mips-mdebug-tdep.h' and `mips-mdebug-tdep.c' 
> that were removed from our tree as a result of an unfortunate coincidence 
> and have been maintained outside it for years now; they need some 
> improvements at the time they are brought back too.  Maybe `mdebugread.h' 
> can be stripped down a bit and actually folded into `mips-mdebug-tdep.h' 
> eventually.

This confuses me a bit, which is probably because I don't see what's in
mips-mdebug-tdep.c ...

Looking at alpha-mdebug-tdep.c, which I had hoped would be mostly equivalent,
this gets the PDR records from the magic MDEBUG_EFI_SYMBOL_NAME symbol, which
is created by mdebugread.c while parsing the .mdebug section in an ELF file
or while parsing an ECOFF file.  So if we remove mdebugread.c, nobody will
ever set MDEBUG_EFI_SYMBOL_NAME, which means alpha-mdebug-tdep.c is a no-op.

Is this handled differently on mips?   [ In general, it would be really
good if mips-mdebug-tdep.c is brought back into the tree if it is actually
being used in practice.  Having people maintain stuff out-of-tree long-term
makes it hard to see if common code features are no longer used ... ]

Also, do you happen to know if on other (non- OSF/1) Alpha platforms the
.mdebug debug format is ever used?

Bye,
Ulrich

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

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Maciej W. Rozycki]

On Wed, 10 Sep 2014, Ulrich Weigand wrote:

> >  I have already identified `mdebugread.h' being the only piece required 
> > though -- in addition to `mips-mdebug-tdep.h' and `mips-mdebug-tdep.c' 
> > that were removed from our tree as a result of an unfortunate coincidence 
> > and have been maintained outside it for years now; they need some 
> > improvements at the time they are brought back too.  Maybe `mdebugread.h' 
> > can be stripped down a bit and actually folded into `mips-mdebug-tdep.h' 
> > eventually.
> 
> This confuses me a bit, which is probably because I don't see what's in
> mips-mdebug-tdep.c ...

 For that you'd have to do some archaeology, which is very easy with GIT:

$ git log -1 -- gdb/mips-mdebug-tdep.c
commit cb2a4ac5dae478fcd9d6e772530c3aba0576fc7a
Author: Daniel Jacobowitz <drow@false.org>
Date:   Fri Apr 13 14:25:12 2007 +0000

    Delete files for last commit.
$ git show cb2a4ac5dae478fcd9d6e772530c3aba0576fc7a
[...]

> Looking at alpha-mdebug-tdep.c, which I had hoped would be mostly equivalent,
> this gets the PDR records from the magic MDEBUG_EFI_SYMBOL_NAME symbol, which
> is created by mdebugread.c while parsing the .mdebug section in an ELF file
> or while parsing an ECOFF file.  So if we remove mdebugread.c, nobody will
> ever set MDEBUG_EFI_SYMBOL_NAME, which means alpha-mdebug-tdep.c is a no-op.

 Now that you mention it there is this statement:

  sym = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN, 0, NULL);

in `mips-mdebug-tdep.c' (in `non_heuristic_proc_desc') indeed, however 
it's just a fallback in the case where no PDR record has been found.  
This part can be removed if IRIX support was dropped, we don't expect to 
have a `.mdebug' section on non-IRIX MIPS ELF targets, so that would 
become dead code.  The structure of PDR itself is documented in [1] and is 
only a subset of what a `.mdebug' section would include.

 I can see `alpha-mdebug-tdep.c' doesn't have anything like that, it's 
quite different overall.

> Is this handled differently on mips?   [ In general, it would be really
> good if mips-mdebug-tdep.c is brought back into the tree if it is actually
> being used in practice.  Having people maintain stuff out-of-tree long-term
> makes it hard to see if common code features are no longer used ... ]

 That's been long scheduled, but regrettably I have never been able to 
convince the right people to give it any priority.  It's down the n-th 
patch on the list of oustanding MIPS patches I have.

> Also, do you happen to know if on other (non- OSF/1) Alpha platforms the
> .mdebug debug format is ever used?

 On Alpha/Linux you'll get it with `-gstabs', although that'll be along 
some DWARF information for some reason, at least in the somewhat dated 
version of Alpha GCC I have, e.g.:

$ cat main.c
int
main (void)
{
  return 0;
}
$ alpha-linux-gcc -O2 -gstabs -o alpha-linux-main main.c
$ alpha-linux-readelf -S alpha-linux-main
There are 31 section headers, starting at offset 0x2c750:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .interp           PROGBITS         0000000120000200  00000200
       0000000000000013  0000000000000000   A       0     0     1
  [ 2] .note.ABI-tag     NOTE             0000000120000214  00000214
       0000000000000020  0000000000000000   A       0     0     4
  [ 3] .hash             HASH             0000000120000238  00000238
       0000000000000040  0000000000000008   A       4     0     8
  [ 4] .dynsym           DYNSYM           0000000120000278  00000278
       0000000000000048  0000000000000018   A       5     1     8
  [ 5] .dynstr           STRTAB           00000001200002c0  000002c0
       0000000000000038  0000000000000000   A       0     0     1
  [ 6] .gnu.version      VERSYM           00000001200002f8  000002f8
       0000000000000006  0000000000000002   A       4     0     2
  [ 7] .gnu.version_r    VERNEED          0000000120000300  00000300
       0000000000000020  0000000000000000   A       5     1     8
  [ 8] .rela.plt         RELA             0000000120000320  00000320
       0000000000000018  0000000000000018   A       4    18     8
  [ 9] .init             PROGBITS         0000000120000338  00000338
       0000000000000068  0000000000000000  AX       0     0     8
  [10] .text             PROGBITS         00000001200003a0  000003a0
       0000000000000250  0000000000000000  AX       0     0     16
  [11] .fini             PROGBITS         00000001200005f0  000005f0
       0000000000000040  0000000000000000  AX       0     0     8
  [12] .eh_frame_hdr     PROGBITS         0000000120000630  00000630
       0000000000000024  0000000000000000   A       0     0     4
  [13] .eh_frame         PROGBITS         0000000120000658  00000658
       000000000000008c  0000000000000000   A       0     0     8
  [14] .ctors            PROGBITS         00000001200106e8  000006e8
       0000000000000010  0000000000000000  WA       0     0     8
  [15] .dtors            PROGBITS         00000001200106f8  000006f8
       0000000000000010  0000000000000000  WA       0     0     8
  [16] .jcr              PROGBITS         0000000120010708  00000708
       0000000000000008  0000000000000000  WA       0     0     8
  [17] .dynamic          DYNAMIC          0000000120010710  00000710
       0000000000000190  0000000000000010  WA       5     0     8
  [18] .plt              PROGBITS         00000001200108a0  000008a0
       000000000000002c  0000000000000000 WAX       0     0     16
  [19] .got              PROGBITS         00000001200108d0  000008d0
       0000000000000020  0000000000000000  WA       0     0     8
  [20] .sdata            PROGBITS         00000001200108f0  000008f0
       0000000000000018  0000000000000000 WAp       0     0     8
  [21] .sbss             NOBITS           0000000120010908  00000908
       0000000000000001  0000000000000000 WAp       0     0     1
  [22] .comment          PROGBITS         0000000000000000  00000908
       000000000000007d  0000000000000000           0     0     1
  [23] .debug_aranges    PROGBITS         0000000000000000  00000990
       0000000000000080  0000000000000000           0     0     16
  [24] .debug_info       PROGBITS         0000000000000000  00000a10
       00000000000000fe  0000000000000000           0     0     1
  [25] .debug_abbrev     PROGBITS         0000000000000000  00000b0e
       0000000000000020  0000000000000000           0     0     1
  [26] .debug_line       PROGBITS         0000000000000000  00000b2e
       0000000000000108  0000000000000000           0     0     1
  [27] .mdebug           LOPROC+1         0000000000000000  00000c38
       000000000002ba08  0000000000000001           0     0     8
  [28] .shstrtab         STRTAB           0000000000000000  0002c640
       000000000000010b  0000000000000000           0     0     1
  [29] .symtab           SYMTAB           0000000000000000  0002cf10
       0000000000000630  0000000000000018          30    49     8
  [30] .strtab           STRTAB           0000000000000000  0002d540
       0000000000000269  0000000000000000           0     0     1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)
$

It all gets stripped together though.  For MIPS you don't need any debug 
information to get PDR:

$ mips-linux-gcc -O2 -s -o mips-linux-main main.c
$ mips-linux-readelf -l mips-linux-main
There are 27 section headers, starting at offset 0xa7c:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .interp           PROGBITS        00400134 000134 00000d 00   A  0   0  1
  [ 2] .note.ABI-tag     NOTE            00400144 000144 000020 00   A  0   0  4
  [ 3] .reginfo          MIPS_REGINFO    00400164 000164 000018 18   A  0   0  4
  [ 4] .dynamic          DYNAMIC         0040017c 00017c 0000d8 08   A  7   0  4
  [ 5] .hash             HASH            00400254 000254 000040 04   A  6   0  4
  [ 6] .dynsym           DYNSYM          00400294 000294 0000b0 10   A  7   1  4
  [ 7] .dynstr           STRTAB          00400344 000344 000094 00   A  0   0  1
  [ 8] .gnu.version      VERSYM          004003d8 0003d8 000016 02   A  6   0  2
  [ 9] .gnu.version_r    VERNEED         004003f0 0003f0 000020 00   A  7   1  4
  [10] .init             PROGBITS        00400410 000410 0000a4 00  AX  0   0  4
  [11] .text             PROGBITS        004004c0 0004c0 0002e0 00  AX  0   0 16
  [12] .MIPS.stubs       PROGBITS        004007a0 0007a0 000020 00  AX  0   0  4
  [13] .fini             PROGBITS        004007c0 0007c0 000058 00  AX  0   0  4
  [14] .rodata           PROGBITS        00400818 000818 000004 04  AM  0   0  4
  [15] .eh_frame         PROGBITS        0040081c 00081c 000004 00   A  0   0  4
  [16] .ctors            PROGBITS        00410820 000820 000008 00  WA  0   0  4
  [17] .dtors            PROGBITS        00410828 000828 000008 00  WA  0   0  4
  [18] .jcr              PROGBITS        00410830 000830 000004 00  WA  0   0  4
  [19] .data             PROGBITS        00410840 000840 000030 00  WA  0   0 16
  [20] .rld_map          PROGBITS        00410870 000870 000004 00  WA  0   0  4
  [21] .got              PROGBITS        00410880 000880 000040 04 WAp  0   0 16
  [22] .bss              NOBITS          004108c0 0008c0 000010 00  WA  0   0 16
  [23] .pdr              PROGBITS        00000000 0008c0 000060 00      0   0  4
  [24] .comment          PROGBITS        00000000 000920 00007d 00      0   0  1
  [25] .mdebug.abi32     PROGBITS        0000007d 00099d 000000 00      0   0  1
  [26] .shstrtab         STRTAB          00000000 00099d 0000dd 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)
$

-- see the `.pdr' and `.mdebug.abi32' sections.  So its use is quite 
different to Alpha's `.mdebug' section.

 I don't know how Alpha performs under GDB in stripped code, I think it 
just does not at all, just like MIPS without these PDR records (they can 
be forcefully stripped with `objcopy -R .pdr' or not produced in the first 
place with GAS's `-mno-pdr' option).

 References:

[1] ftp://ftp.sgi.com/sgi/dev/davea/Mdebug.ps

  Maciej

Re: eliminate deprecated_insert_raw_breakpoint. what's left.

[Ulrich Weigand]

Maciej W. Rozycki wrote:

>  Now that you mention it there is this statement:
> 
>   sym = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN, 0, NULL);
> 
> in `mips-mdebug-tdep.c' (in `non_heuristic_proc_desc') indeed, however 
> it's just a fallback in the case where no PDR record has been found.  
> This part can be removed if IRIX support was dropped, we don't expect to 
> have a `.mdebug' section on non-IRIX MIPS ELF targets, so that would 
> become dead code.  The structure of PDR itself is documented in [1] and is 
> only a subset of what a `.mdebug' section would include.

OK, I see.  Looking at gas sources, I can now see that it will create a
.pdr section instead of .mdebug (unless ECOFF debugging is enabled).

> > Also, do you happen to know if on other (non- OSF/1) Alpha platforms the
> > .mdebug debug format is ever used?
> 
>  On Alpha/Linux you'll get it with `-gstabs', although that'll be along 
> some DWARF information for some reason, at least in the somewhat dated 
> version of Alpha GCC I have, e.g.:

Thanks for the info!  Looking through GCC and GAS sources, this is triggered
by GCC passing the -mdebug flag to GAS when it gets -gstabs.  In fact, there
is also a code path on Mips to pass -mdebug to GAS; but this needs the -gcoff
GCC option ...

So in any case, it seems that once IRIX and OSF are gone, we no longer need
to support the ECOFF file format, but there are still ways to generate the
ECOFF *debug* format (even though non-default) on two Linux platforms.  Well,
I guess there's currently no need to remove GDB support for that  ...

Bye,
Ulrich

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