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

[Michael Snyder <msnyder at cygnus dot com>]

Daniel Jacobowitz wrote:
> 
> resume () in infrun.c has this block:
> 
>   if (SOFTWARE_SINGLE_STEP_P () && step)
>     {
>       /* Do it the hard way, w/temp breakpoints */
>       SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ );
>       /* ...and don't ask hardware to do it.  */
>       step = 0;
> 
> Then, further down, if (use_thread_step_needed && thread_step_needed)
> and there's already a breakpoint at the PC, is this:
> 
>               if (!step)
>                 {
>                   warning ("Internal error, changing continue to step.");
> 
> That blows up, because step will always be zero here if
> SOFTWARE_SINGLE_STEP_P ().  Is this patch OK?  It seems to work in my tests
> here.

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()))