Re: Get rid of stop_pc (was: [RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement)

[Doug Evans <dje@google.com>]

On Thu, Dec 4, 2008 at 5:15 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 05 December 2008 00:36:56, Pedro Alves wrote:
>> On Friday 05 December 2008 00:18:00, Ulrich Weigand wrote:
>> > Pedro Alves wrote:
>> > > On Thursday 04 December 2008 22:32:12, Doug Evans wrote:
>> > > > In the original code, is there a case when stop_pc != registers.pc?
>> > >
>> > > Here,
>> > >
>> > > <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.
>>
>> > It seems to me just about every current user of stop_pc *really* wants
>> > to look at regcache_read_pc (get_current_regcache ()) ...
>
> Is using read_pc instead OK with you?  It's what I had written already.
>
>> I've been sneaking the idea of getting rid of stop_pc for a while now:
>>  http://sourceware.org/ml/gdb-patches/2008-06/msg00450.html
>>
>> In fact, I have a months old patch here that completelly removes stop_pc.
>> IIRC, there were no visible changes in the testsuite.  Say the word,
>> and I'll brush it up, regtest, submit it.
>
> Here it is, it still applied cleanly.  It's smallish.  Regtested on
> x86-64-unknown-linux-gnu.
>
> My original motivation was to get rid of the ugly checks
> in switch_to_thread, and to try to minimize the extra thread
> switching and register reads in non-stop mode.
>
> I had held posting this when I wrote it, since I was not sure we'd not
> miss stop_pc in some case.
>
> --
> Pedro Alves
>

Nit.

The check for !frame isn't related to removing stop_pc.  Separate
patch or add a changelog entry?

Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-12-05 00:47:41.000000000 +0000
+++ src/gdb/infcmd.c	2008-12-05 00:49:53.000000000 +0000
@@ -924,14 +921,17 @@ step_once (int skip_subroutines, int sin
 	 the longjmp breakpoint was not required.  Use the
 	 INFERIOR_PTID thread instead, which is the same thread when
 	 THREAD is set.  */
-      struct thread_info *tp = inferior_thread ();
+      struct thread_info *tp;
+      CORE_ADDR stop_pc;
+
+      tp = inferior_thread ();
       clear_proceed_status ();

       frame = get_current_frame ();
-      if (!frame)		/* Avoid coredump here.  Why tho? */
-	error (_("No current frame"));
       tp->step_frame_id = get_frame_id (frame);

+      stop_pc = read_pc ();
+
       if (!single_inst)
 	{
 	  find_pc_line_pc_range (stop_pc,