Re: remove global stop_bpstat dependency from breakpoints module

[Pedro Alves <pedro at codesourcery dot com>]

A Thursday 05 June 2008 19:03:31, Daniel Jacobowitz wrote:
> On Thu, May 08, 2008 at 02:22:04AM +0100, Pedro Alves wrote:
> > I had missed this on the non-stop series.  I'll need to context switch
> > stop_bpstat in non-stop mode, because there, we'll have simultaneous
> > independant stop events, one per thread, and each should have its own
> > stop_bpstat.  That's a small change to patch 4 in series I posted.
>
> The implementation makes it look like this will work for watchpoints
> too.  If we want to preserve that - which is not the documented
> behavior - then I agree we should context-switch stop_bpstat.  But
> I'm thinking it makes more sense to restrict this to breakpoints,
> and search for a breakpoint at the current PC.

Well, I can't say if we want to preserve that.  I never use
this functionality, so I wouldn't missed it.  The
"ignore" command gets me the same functionality anyway.

Notice that I was proposing context switching it in non-stop
only.  The current implementation sets the ignore count
on the breakpoint that we stopped last.  If we just read current
PC, we may be reading it from the wrong thread:

 Breakpoint 1, at xxx
 thread 2
 c 10

we could use get_last_target_status to get at the
correct PC if we do want to preserve this behaviour in
all-stop.  In non-stop, I'd read from the current thread always.

But, the help doc:

"Continue program being debugged, after signal or breakpoint.
If proceeding from breakpoint, a number N may be used as an argument,
which means to set the ignore count of that breakpoint to N - 1 (so that
the breakpoint won't break until the Nth time it is reached)"

... doesn't make it clear, if "proceeding" was meant to apply
to the last breakpoint, or to the breakpoint under PC.

As I said, I would mind at all to change this to the read
the PC of the current thread.  Just pointing out the current
behaviour.

>
> What do you think?  Also, does that remove the only reason to
> context-switch stop_bpstat?  If so, we don't need this patch at all.
>

Yes.

> > One way to fix it, would be to also loop through all threads to update
> > their version of stop_bpstat, but I'd like better.
>
> If we do need to context-switch stop_bpstat, it seems like checking
> all of those would be simpler than this patch as posted.

Well, simpler yes, but adds even more coupling.  I thought this was
a nice cleanup.  It's mentioned a few times in comments
throughout breakpoints.c.

-- 
Pedro Alves