From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1887 invoked by alias); 14 Mar 2009 12:53:35 -0000 Received: (qmail 1878 invoked by uid 22791); 14 Mar 2009 12:53:34 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS 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; Sat, 14 Mar 2009 12:53:29 +0000 Received: (qmail 4708 invoked from network); 14 Mar 2009 12:53:27 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Mar 2009 12:53:27 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] When debugging infrun, print stop_pc in all possible cases. Date: Sat, 14 Mar 2009 13:56:00 -0000 User-Agent: KMail/1.9.10 Cc: Doug Evans References: <20090226180105.138461C78A7@localhost> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200903141243.42550.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: 2009-03/txt/msg00229.txt.bz2 On Friday 13 March 2009 17:06:13, Doug Evans wrote: > On Thu, Feb 26, 2009 at 11:01 AM, Doug Evans wrote: > > Hi. > > > > I've been debugging a few issues and it's been really helpful to know > > stop_pc in cases that aren't printed today. > > This patch moves the debug printing of stop_pc up. > > > > I'm not submitting it RFA because I'm not entirely happy with it. > > Printing stop_pc has been helpful, I'd like to improve what's there today, > > but it's not clear to me that this patch reasonably handles the cases > > when one can't read pc before the big switch() on ecs->ws.kind. Hmm,: - not ignoring TARGET_WAITKIND_IGNORE. - accessing the target before the set_executing call. It may end up being a problem in the future. - With displaced stepping, we lost the printing of the stop_pc *after* the fixup. - I'm not sure we want to move the watchpoint debug bits as well. > Any comments on this approach? > Would you do it differently? > [E.g. One could add code to print the pc to all the appropriate cases > in the switch(), but that seems clumsy.] Dunno, perhaps, come up with a "set_stop_pc (CORE_ADDR)" function, do the debug printing there, and replace all the appropriate accesses to that global by a call to the new function? This is probably one of the cases where I'd hack the debug output, fix the issue, and then remove the hack and forget about it. :-) > > > > Comments? > > > > 2009-02-26 Doug Evans > > > > * infrun.c (handle_inferior_event): Move the debug printing of stop_pc, > > and possible watchpoint address, to an earlier point so we print it > > in all cases (where it's possible). > > > > Index: infrun.c > > =================================================================== > > RCS file: /cvs/src/src/gdb/infrun.c,v > > retrieving revision 1.360 > > diff -u -p -r1.360 infrun.c > > --- infrun.c 25 Feb 2009 02:14:22 -0000 1.360 > > +++ infrun.c 26 Feb 2009 17:52:28 -0000 > > @@ -2178,6 +2178,46 @@ handle_inferior_event (struct execution_ > > /* Dependent on the current PC value modified by adjust_pc_after_break. */ > > reinit_frame_cache (); > > > > + if (debug_infrun) > > + { > > + /* It's helpful to know the stop pc in all cases when we stop > > + (even if we're eventually going to resume). > > + To keep things simple we print it here before the big switch on > > + ecs->ws.kind. If we leave printing of stop_pc until later, > > + we will miss the cases where we "return;" early. > > + There are a few cases where we can't read pc though. */ > > + > > + switch (ecs->ws.kind) > > + { > > + case TARGET_WAITKIND_EXITED: > > + case TARGET_WAITKIND_SIGNALLED: > > + break; > > + > > + default: > > + { > > + CORE_ADDR debug_stop_pc = > > + regcache_read_pc (get_thread_regcache (ecs->ptid)); > > + > > + fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = 0x%s\n", > > + paddr_nz (debug_stop_pc)); > > + > > + if (STOPPED_BY_WATCHPOINT (&ecs->ws)) > > + { > > + CORE_ADDR addr; > > + > > + if (target_stopped_data_address (¤t_target, &addr)) > > + fprintf_unfiltered (gdb_stdlog, > > + "infrun: stopped by watchpoint, data address = 0x%s\n", > > + paddr_nz (addr)); > > + else > > + fprintf_unfiltered (gdb_stdlog, > > + "infrun: stopped by watchpoint, no data address available\n"); > > + } > > + break; > > + } > > + } > > + } > > + > > if (ecs->ws.kind != TARGET_WAITKIND_IGNORE) > > { > > breakpoint_retire_moribund (); > > @@ -2507,25 +2547,6 @@ targets should add new threads to the th > > > > stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid)); > > > > - if (debug_infrun) > > - { > > - fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = 0x%s\n", > > - paddr_nz (stop_pc)); > > - if (STOPPED_BY_WATCHPOINT (&ecs->ws)) > > - { > > - CORE_ADDR addr; > > - fprintf_unfiltered (gdb_stdlog, "infrun: stopped by watchpoint\n"); > > - > > - if (target_stopped_data_address (¤t_target, &addr)) > > - fprintf_unfiltered (gdb_stdlog, > > - "infrun: stopped data address = 0x%s\n", > > - paddr_nz (addr)); > > - else > > - fprintf_unfiltered (gdb_stdlog, > > - "infrun: (no data address available)\n"); > > - } > > - } > > - > > if (stepping_past_singlestep_breakpoint) > > { > > gdb_assert (singlestep_breakpoints_inserted_p); > > > -- Pedro Alves