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

Doug Evans
2008-12-05T01:50:05Z

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,