Set the stepping range from the function at PC, not at stop_pc.

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

Following up on:
 http://sourceware.org/ml/gdb-patches/2008-12/msg00100.html

Here goes.  This addresses this valid use case:

< stopped at 0x1234, thread 1, (stop_pc == 0x1234) >
 (gdb) set $pc = 0xf00
 (gdb) step

GDB at this point would set the stepping range based on the
function at 0x1234, but execution was about to resume at 0xf00.

GDB should instead be setting the stepping range from the
function at 0xf00.

I think it's obvious after the discussion, but I'd still like to
have your approval.  OK?  Any objections?

On Friday 05 December 2008 19:06:14, Pedro Alves wrote:
> On Friday 05 December 2008 18:42:45, Ulrich Weigand wrote:
> > Pedro Alves wrote:
> > 
> > > > > > <stopped at 0x1234, thread 1>
> > > > > >  (gdb) set $pc = 0xf00
> > > > > >  (gdb) call func()
> > > > > 
> > > > > Huh.  But that case is in fact *broken*, because GDB will use stop_pc
> > > > > incorrectly: for example, the check whether we are about to continue
> > > > > at a breakpoint will look at stop_pc, but then continue at $pc.  
> > > > 
> > > > This one I believe was the original intention.  The rationale being
> > > > that you'd not want to hit a breakpoint again at stop_pc (0x1234),
> > > > because there's where you stopped; but, you'd want to hit a a breakpoint
> > > > at 0xf00, sort of like jump *$pc hits a breakpoint at $pc.
> > > > 
> > > > Note, I'm not saying I agree with this.  I did say that probably nobody
> > > > would notice if we got rid of stop_pc.
> > 
> > OK, I see.  This is a valid use case, and it may make sense to keep it.
> > However, as you point out, to make this really work as intended, we'd
> > have make stop_pc a per-thread variable.
> > 
> > And even in that case, the uses of stop_pc in step_1 and step_once seem
> > invalid to me.
> > 
> 
> 100% Agreed.  I'll take care of it.

-- 
Pedro Alves

[-- Attachment #2: step_func_pc.diff --]
[-- Type: text/x-diff, Size: 1678 bytes --]

2008-12-05  Pedro Alves  <pedro@codesourcery.com>

	* infcmd.c (step_1, step_once): Look up the stepping range based
	on the current PC, not on stop_pc.

---
 gdb/infcmd.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-12-05 19:20:20.000000000 +0000
+++ src/gdb/infcmd.c	2008-12-05 19:27:49.000000000 +0000
@@ -812,12 +812,15 @@ step_1 (int skip_subroutines, int single
 
 	  if (!single_inst)
 	    {
-	      find_pc_line_pc_range (stop_pc,
+	      CORE_ADDR pc;
+
+	      pc = read_pc ();
+	      find_pc_line_pc_range (pc,
 				     &tp->step_range_start, &tp->step_range_end);
 	      if (tp->step_range_end == 0)
 		{
 		  char *name;
-		  if (find_pc_partial_function (stop_pc, &name,
+		  if (find_pc_partial_function (pc, &name,
 						&tp->step_range_start,
 						&tp->step_range_end) == 0)
 		    error (_("Cannot find bounds of current function"));
@@ -932,7 +935,10 @@ step_once (int skip_subroutines, int sin
 
       if (!single_inst)
 	{
-	  find_pc_line_pc_range (stop_pc,
+	  CORE_ADDR pc;
+
+	  pc = read_pc ();
+	  find_pc_line_pc_range (pc,
 				 &tp->step_range_start, &tp->step_range_end);
 
 	  /* If we have no line info, switch to stepi mode.  */
@@ -943,7 +949,7 @@ step_once (int skip_subroutines, int sin
 	  else if (tp->step_range_end == 0)
 	    {
 	      char *name;
-	      if (find_pc_partial_function (stop_pc, &name,
+	      if (find_pc_partial_function (pc, &name,
 					    &tp->step_range_start,
 					    &tp->step_range_end) == 0)
 		error (_("Cannot find bounds of current function"));