From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23050 invoked by alias); 5 Dec 2008 19:07:14 -0000 Received: (qmail 23040 invoked by uid 22791); 5 Dec 2008 19:07:12 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 05 Dec 2008 19:06:19 +0000 Received: (qmail 28098 invoked from network); 5 Dec 2008 19:06:16 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Dec 2008 19:06:16 -0000 From: Pedro Alves To: "Ulrich Weigand" Subject: Re: Get rid of stop_pc (was: [RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement) Date: Fri, 05 Dec 2008 19:07:00 -0000 User-Agent: KMail/1.9.10 Cc: gdb-patches@sourceware.org, Doug Evans References: <200812051842.mB5IgjF3021686@d12av02.megacenter.de.ibm.com> In-Reply-To: <200812051842.mB5IgjF3021686@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200812051906.14828.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-12/txt/msg00100.txt.bz2 On Friday 05 December 2008 18:42:45, Ulrich Weigand wrote: > Pedro Alves wrote: > > > > > > > > > > > (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. > > @@ -3705,6 +3706,7 @@ handle_step_into_function (struct execut > > { > > struct symtab *s; > > struct symtab_and_line stop_func_sal, sr_sal; > > + CORE_ADDR stop_pc = read_pc (); > > > > s = find_pc_symtab (stop_pc); > > if (s && s->language != language_asm) > > @@ -3781,6 +3783,7 @@ handle_step_into_function_backward (stru > > { > > struct symtab *s; > > struct symtab_and_line stop_func_sal, sr_sal; > > + CORE_ADDR stop_pc = read_pc (); > > > > s = find_pc_symtab (stop_pc); > > if (s && s->language != language_asm) > > These could probably receive the stop_pc from handle_inferior_event > instead of recomputing it. Right. It would hit the cache, but, then again, if/when we have a stop_pc per-thread, we'd use that. > > > @@ -4283,7 +4286,7 @@ Further execution is probably impossible > > if (tp->stop_step > > && frame_id_eq (tp->step_frame_id, > > get_frame_id (get_current_frame ())) > > - && step_start_function == find_pc_function (stop_pc)) > > + && step_start_function == find_pc_function (read_pc ())) > > source_flag = SRC_LINE; /* finished step, just print source line */ > > else > > source_flag = SRC_AND_LOC; /* print location and source line */ > > As Andrew's comment notes, the function comparison should be redundant > these days as it is already implied in the frame-ID comparison. > Oh, that's what that comment means? I always had trouble parsing the English in it. Makes sense. > > @@ -1149,7 +1149,7 @@ signal_command (char *signum_exp, int fr > > FIXME: Neither should "signal foo" but when I tried passing > > (CORE_ADDR)-1 unconditionally I got a testsuite failure which I haven't > > tried to track down yet. */ > > - proceed (oursig == TARGET_SIGNAL_0 ? (CORE_ADDR) -1 : stop_pc, oursig, 0); > > + proceed (oursig == TARGET_SIGNAL_0 ? (CORE_ADDR) -1 : read_pc (), oursig, 0); > > } > > > > /* Proceed until we reach a different source line with pc greater than > > Dan wanted to get rid of this use of stop_pc anyway, see: > http://sourceware.org/ml/gdb-patches/2008-08/msg00651.html Yep. I think his patch makes sense: http://sourceware.org/ml/gdb-patches/2008-11/msg00439.html > > > @@ -1585,8 +1585,7 @@ program_info (char *args, int from_tty) > > stat = bpstat_num (&bs, &num); > > > > target_files_info (); > > - printf_filtered (_("Program stopped at %s.\n"), > > - hex_string ((unsigned long) stop_pc)); > > + printf_filtered (_("Program stopped at %s.\n"), paddr_nz (read_pc ())); > > if (tp->stop_step) > > printf_filtered (_("It stopped after being stepped.\n")); > > else if (stat != 0) > > If we keep a tp->stop_pc, this place should also make use of it; > otherwise the message isn't really valid (and not very useful: > if it always just prints $pc, it would be redundant with the > other commands to do so ...). Right you are. I'll fix the step_1/step_once bit for now. -- Pedro Alves