Re: [RFA] Don't ignore consecutive breakpoints.

[Michael Snyder <msnyder@specifix.com>]

On Thu, 2007-11-29 at 14:27 +0300, Vladimir Prus wrote:
> On Monday 26 November 2007 21:39:21 Michael Snyder wrote:
> > On Fri, 2007-11-23 at 23:10 +0300, Vladimir Prus wrote:
> > > Suppose we have two breakpoints at two consecutive
> > > addresses, and we do "step" while stopped on the
> > > first breakpoint. GDB testsuite has a test (consecutive.exp)
> > > that the second breakpoint will be hit a reported, and the
> > 
> > Yeah, I was the author of that test, back in 2001.
> > Several years and several employers ago, but I think 
> > I am able to remember a little about the context.
> > 
> > > test passes, but the code directly contradicts, saying:
> > > 
> > >       /* Don't even think about breakpoints if just proceeded over a
> > >          breakpoint.  */
> > >       if (stop_signal == TARGET_SIGNAL_TRAP && trap_expected)
> > > 	{
> > >           if (debug_infrun)
> > > 	    fprintf_unfiltered (gdb_stdlog, "infrun: trap expected\n");
> > > 	  bpstat_clear (&stop_bpstat);
> > > 	}
> > > 
> > > what's happening is that we indeed ignore the breakpoint, and try
> > > to step further. However ecs->another_trap is not set, so we step
> > > with breakpoints inserted, and immediately hit the now-inserted
> > > breakpoint. Therefore, I propose to remove that code.
> > > 
> > > On x86, the below patch causes a single test outcome change:
> > > 
> > > -KFAIL: gdb.base/watchpoint.exp: next after watch x (PRMS: gdb/38)
> > > +PASS: gdb.base/watchpoint.exp: next after watch x
> > 
> > Yeah, the problem is that you have only tested x86 architecture, 
> > and what I think I recall is that this test was for software
> > single-step.
> > 
> > You have to be aware that you have just single-stepped, so that
> > you interpret the trap instruction under the PC as related to 
> > stepping.  If you have two consecutive BP-related traps, and you
> > try to single step over one of them, you may miss the second one
> > because you believe it to be only a single-stepping trap.
> > 
> > Can you test your patch on an architecture that uses software SS?
> 
> I've tested on arm-linux/qemu, which uses software single-step,
> and got no regressions.
> 
> Looking again at the patch, the code fragment I'm changing has
> two side-effects:
> 
> - Setting ecs->random_signal
> - Setting stop_bpstat
> 
> My patch has no effect on the way ecs->random_signal is set.
> However, in the case when we've just single-stepped over
> breakpoint, the original code will clear stop_bpstat, and in
> my patch, it would be set. We will immediately report report
> the hit of the consecutive breakpoint. Since we don't set
> ecs->another_trap, the trap_expected variable will be reset
> to 0 when we resume.
> 
> So, is the patch OK?

Thanks for the testing and analysis.
I have no further objection.

Michael