Re: PATCH: resume + threads + software stepping == boom

Daniel Jacobowitz
2001-06-09T16:05:00Z

On Sat, Jun 09, 2001 at 01:15:00PM -0400, Andrew Cagney wrote:
> Could I suggest:
>
> > On Fri, Jun 08, 2001 at 04:20:30PM -0700, Michael Snyder wrote:
> >
> >> Michael Snyder wrote:
> >
> >> > I like the problem analysis, but not the implementation of the solution.
> >> > If we are going to always set step to zero for SOFTWARE_SINGLE_STEP_P,
> >> > then it does not make sense to set it to one again, even if the code
> >> > will never be reached (in theory). I would rather see it made explicit
> >> > that this code should never be reached if SOFTWARE_SINGLE_STEP_P is true.
> >> > Something like this:
> >> >
> >> > < if (!step)
> >> > ---
> >
> >> > > if (!(step && SOFTWARE_SINGLE_STEP_P()))
> >
> >>
> >> Err, my logic is wrong, but you get the idea... maybe I meant
> >> if (!step && !SOFTWARE_SINGLE_STEP_P())
>
>
> Dumping the warning("internal error: ...") and replacing the entire
> block of code with something like:
>
> gdb_assert (step || SOFTWARE_SINGLE_STEP_P ())
>
> (I know my logic is wrong). That way the problem of ``never reached (in
> theory)'' is eliminated.

My concern is that this is a weaker assertion than was intended. How
about this - it changes the meaning of the assertion slightly since
it's not conditional on should_resume, but I think it'll be correct
none the less:

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.35
diff -u -r1.35 infrun.c
--- infrun.c 2001/06/02 00:36:20 1.35
+++ infrun.c 2001/06/09 23:05:32
@@ -850,6 +850,11 @@
if (breakpoint_here_p (read_pc ()) == permanent_breakpoint_here)
SKIP_PERMANENT_BREAKPOINT ();

+ /* If we stopped on a breakpoint and it was not deleted, we want to continue
+ only this thread, so we should be stepping. */
+ gdb_assert (step || !use_thread_step || !thread_step_needed ||
+ !breakpoint_here_p (read_pc ()));
+
if (SOFTWARE_SINGLE_STEP_P () && step)
{
/* Do it the hard way, w/temp breakpoints */
@@ -927,14 +932,7 @@
}
else
{
- if (!step)
- {
- warning ("Internal error, changing continue to step.");
- remove_breakpoints ();
- breakpoints_inserted = 0;
- trap_expected = 1;
- step = 1;
- }
+ /* Breakpoint not deleted, so step only this thread. */
resume_ptid = inferior_ptid;
}
}

--
Daniel Jacobowitz Debian GNU/Linux Developer
Monta Vista Software Debian Security Team