Re: [rfc] Do not call read_pc in startup_inferior

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Pedro Alves wrote:
> On Thursday 30 April 2009 15:25:17, Ulrich Weigand wrote:
> > I see.  It seems "stop_pc" isn't really the proper mechanism for this.
> > (See also the problems with skipping breakpoints in multi-threaded 
> > applications that Doug Evans has been working on ...)
> 
> Yeah, I'm also interested in this for multi-process as well.
> 
> > What would you think about the following replacement for stop_pc:
> 
> Before going further, I don't think you should hold on your current
> patch?

Yes, I don't think my current patch makes anything worse here.
I'll check it in shortly.

> > - Per thread, maintain a "last stop reason" state that identifies
> >   *where* and *why* this thread stopped.
> > - We skip a breakpoint whenever we're about to restart a thread and
> >   its "last stop reason" is "breakpoint" with an address equal to
> >   the restart address (and there's still a breakpoint at this location).
> 
> This logic with per-thread "last stop reason is breakpoint"
> changes the current behaviour, in the wrong direction, I believe.
> I don't think "breakpoint" or not is the correct check.  Consider:
> 
>  1. (gdb) step
>  2. (gdb) step
>  3. (gdb) break
>  4. (gdb) continue
> 
> At 3. the current thread was not stopped at a breakpoint, yet,
> in 4., `proceed' is expected to skip over the breakpoint you've
> just set.

Hmmm, I was looking for consistency with the case where you
change PC (either explicitly, or by calling an inferior function
or triggering an inferior signal handler) to a location that
has a breakpoint set: in this case, GDB does stop immediately
and reports the breakpoint hit.

However, from a user interface perspective, the two cases are
probably different enough to deserve different handling: you
may want to step through the signal handler or inferior function,
so you want the breakpoint to hit.  On the other hand, in the
sequence you mention, having the "continue" stop immediately
doesn't seem to serve any useful purpose.

> If we had a per-thread stop_pc, we'd be able to make this
> behave identically to the example below.

OK.  Let's just go for a per-thread stop_pc and keep the rest
of the logic unchanged ...

> > - In "info program", display the last stop reason of the current
> >   thread (or maybe all threads?). 
> 
> I think the intent of "info program" is to show the last stop reason,
> no matter what thread you have selected.  I think "info threads" would
> be a good place to show last stop reason of all threads, especially 
> if you think of non-stop mode.  Most IDE's I know that show last
> stop reason, put it next to the description of each thread, in the
> thread tree/list.

Makes sense.  This means we'd have to remember the "thread we last
stopped at" so we can show its stop_pc.  I don't think we actually
have this information right now?

> > Hmm, if we have two breakpoints at the same address, one that is
> > supposed to silently restart, and one that is supposed to be
> > reported to the user, shouldn't the second one always "win"?
> 
> Close, but not quite.  Yes, we should stop for the second one, but,
> we should still handle the BPSTAT_WHAT_CHECK_SHLIBS shlibs checking.
> Currently, shlib event gets picked over a user breakpoint, and infrun.c
> immediatelly resumes the target after hanling BPSTAT_WHAT_CHECK_SHLIBS.
> The root problem here is that the shlib event breakpoint handling
> should *not* be mutually exclusive with other breakpoints handling,
> but, it is currently.  Funny enough, Doug has just tripped on
> this: http://sourceware.org/ml/gdb-patches/2009-04/msg00801.html
> In his case, when stopping for a shlib event, and if the watchpoint's
> value has changed simultaneously,
> bpstat_stop_status->bpstat_check_watchpoint->watchpoint_check
> updates the watchpoint value, but, GDB resumes the inferior immediately,
> even though there was a watchpoint hit to report...  Exactly the same
> problem.

Indeed.  That's another odd corner that I've never quite understood:
at the point where we look at breakpoints, the "bpstat" structure is
already able to represent that multiple causes of actions have occurred
at the same time.  But this detailed status is then reduced via bpstat_what
to a single action code ...   Maybe handle_inferior_event should
really make full use of all the information present in the bpstat.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com