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

[Daniel Jacobowitz <dmj+@andrew.cmu.edu>]

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