[RFA/PATCH] breakpoint.c: fix until command

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> On Thu, Jan 02, 2003 at 05:44:42PM -0800, Michael Snyder wrote:
> > Elena Zannoni wrote:
> > >
> > > Elena Zannoni writes:
> > > >  > Nevertheless, that is and has always been the intent.
> > >  >  > If you're in factorial(5), and you say "until 100",
> > >  >  > you don't stop until line 100 is hit by factorial(5).
> > >  >
> > >  >
> > >  > I am tracking down this to something that changed between (ahem...)
> > >  > 4.18 and 5.0. The code in breakpoint.c didn't change. Right now,
> > >  > stepping the two gdb's side to side, I can see a difference in
> > >  > get_prev_frame, because of a different value returned by
> > >  > FRAME_CHAIN_VALID. :-( (i have not still stepped past that to see how
> > >  > that could influence the until foo behavior, maybe it doesn't).
> > >  >
> > >  > The behavior you specify above is in 5.0 and not in 4.18, while the
> > >  > 'until foo' works in 4.18 and is broken in 5.0.
> > >  >
> > >  > More digging.
> > >  >
> > >  > Elena
> > >
> > > OK. The reason for which 'until foo' worked at all in 4.18 is totally
> > > fortuitous.  It is because of this patch in breakpoint.c:
> > >
> > > 1998-09-08  Jason Molenda  (jsm@bugshack.cygnus.com)
> > >
> > >         * breakpoint.c (bpstat_stop_status):  Declare a bp match if the
> > >         current fp matches the bp->fp OR if the current fp is less than
> > >         the bp->fp if we're looking at a bp_step_resume breakpoint.
> > >
> > > Index: breakpoint.c
> > > ===================================================================
> > > RCS file: /cvs/cvsfiles/src/gdb/breakpoint.c,v
> > > retrieving revision 1.190
> > > retrieving revision 1.191
> > > diff -u -p -p -r1.190 -r1.191
> > > --- breakpoint.c        1998/07/17 15:29:10     1.190
> > > +++ breakpoint.c        1998/09/09 04:16:57     1.191
> > > @@ -1506,7 +1506,9 @@ bpstat_stop_status (pc, not_a_breakpoint
> > >        else if (DECR_PC_AFTER_BREAK != 0 || must_shift_inst_regs)
> > >         real_breakpoint = 1;
> > >
> > > -      if (b->frame && b->frame != (get_current_frame ())->frame)
> > > +      if (b->frame && b->frame != (get_current_frame ())->frame &&
> > > +          (b->type == bp_step_resume &&
> > > +           (get_current_frame ())->frame INNER_THAN b->frame))
> > >         bs->stop = 0;
> > >        else
> > >         {
> > >
> > > Note that this added condition is always false for a bp_until type
> > > breakpoint.  So, effectively we were invalidating the check of the
> > > current frame vs. bp->frame. And we always stopped.
> > >
> > > However, since we were not checking the frames, the case Michael wants
> > > didn't work.
> > >
> > > The patch above was reverted in 1999:
> > >
> > > 1999-08-13  Jim Kingdon  <http://developer.redhat.com/>
> > >
> > >         * breakpoint.c (bpstat_stop_status): Revert 1998-09-08 change
> > >         to ->frame matching.  The change did not match the ChangeLog
> > >         entry, looked fishy, and caused infinite stepping when running
> > >         "next" from main on sparc w/ RH Linux.  Thanks to Jakub for the
> > >         report.
> > >
> > > the effect was that the frame matching check was re-enabled, and so
> > > 'until foo' stopped working.
> > >
> > > I don't think there is a way to have both behaviors work correctly.  I
> > > thought of checking that the pc which you want to run until is in
> > > the same function as the one of the selected frame, and in that case
> > > enforce the check (by using a non-null frame for the bp_until),
> > > otherwise use the null frame (which disables the check). But what would
> > > be the correct behavior if you say:
> > >
> > > "until bar" where bar is recursive, and you are in "bar" at the
> > > moment?  This doesn't work currently. It seems intuitive that you
> > > would stop the next time you enter "bar". Right now you end up at the
> > > caller of "bar".
> > >
> > > I think it is a matter of deciding which behavior is more useful.
> > >
> > > (note that I tried to revert Jason's patch in stock 4.18 and 'until
> > > foo' stopped working, i.e. it wasn't something else that broke between
> > > 4.18 and 5.0)
> >
> > You raise a good point.  The commands "until <line>" and "until <func>"
> > are inconsistant.  Moreover the docs do not seem to describe this
> > recursion behavior.  Maybe a conversation with a wider audience is
> > in order (the gdb list)?  I'm sure I can't be the only one who
> > remembers that "until" behaved this way, and we shouldn't change
> > the behavior precipitously.
> 
> Am I the only one getting the feeling that we have two useful behaviors
> here; and that we should pick one for "until" but expose the other
> under some other name or with some option?

;-)  That's often the case when someone feels 'intuitively' that 
gdb should behave differently.  We have to look out for feeping
creaturitis, but in this case I'm getting the impression that the
two behaviors are mutually incompatible, and may need to be separated
somehow.

If you say "until <line>", and the line is inside the current function,
you can impose the frame restriction.  If the line (or address) is outside 
the current function, or if you give a function name or something else, 
you can't.  And I don't think we can code that distinction at runtime.