From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16148 invoked by alias); 27 Nov 2002 20:55:54 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 16141 invoked from network); 27 Nov 2002 20:55:52 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 27 Nov 2002 20:55:52 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 2F7F33F30; Wed, 27 Nov 2002 15:55:32 -0500 (EST) Message-ID: <3DE53144.3020502@redhat.com> Date: Wed, 27 Nov 2002 12:55:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Michael Snyder , Mark Kettenis Cc: gdb-patches@sources.redhat.com, Daniel Jacobowitz Subject: Re: [Fwd: Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)]] References: <3DE3F135.6030605@redhat.com> Content-Type: multipart/mixed; boundary="------------090809040308030407020108" X-SW-Source: 2002-11/txt/msg00691.txt.bz2 This is a multi-part message in MIME format. --------------090809040308030407020108 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 538 Michael (I suspect Mark is in bed by now), Is there any chance that the attached could be reviewed over the next few hours? Daniel (who is off this week -> us long weekend) noted that the #ifdef vis: +#ifdef __linux__ + fetch_pid = TIDGET (inferior_ptid); + if (fetch_pid == 0) + fetch_pid = PIDGET (inferior_ptid); +#else + fetch_pid = PIDGET (inferior_ptid); +#endif isn't needed. The code should be unconditional. I can fix/commit that provided the patch is ok. It is effectively the last remaining fix for 5.3. Andrew --------------090809040308030407020108 Content-Type: message/rfc822; name="mailbox-message://ac131313@movemail/fsf/gdb/patches#11953735" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="mailbox-message://ac131313@movemail/fsf/gdb/patches#11953735" Content-length: 14027 X-Mozilla-Status2: 12000000 Return-Path: Delivered-To: ac131313@localhost.redhat.com Received: from localhost (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 9079F3FE4 for ; Tue, 26 Nov 2002 17:11:50 -0500 (EST) Envelope-to: cagney@gnu.org Delivery-date: Tue, 26 Nov 2002 17:11:01 -0500 Received: from fencepost.gnu.org by localhost with IMAP (fetchmail-5.9.13) for ac131313@localhost (single-drop); Tue, 26 Nov 2002 17:11:50 -0500 (EST) Received: from monty-python.gnu.org ([199.232.76.173]) by fencepost.gnu.org with esmtp (Exim 4.10) id 18Gnv3-00061h-00 for cagney@gnu.org; Tue, 26 Nov 2002 17:11:01 -0500 Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.10) id 18Gnur-0007Di-00 for cagney@gnu.org; Tue, 26 Nov 2002 17:10:52 -0500 Received: from sources.redhat.com ([209.249.29.67]) by monty-python.gnu.org with smtp (Exim 4.10) id 18Gnur-0007B6-00 for cagney@gnu.org; Tue, 26 Nov 2002 17:10:49 -0500 Received: (qmail 21141 invoked by alias); 26 Nov 2002 22:10:07 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Delivered-To: mailing list gdb-patches@sources.redhat.com Received: (qmail 20994 invoked from network); 26 Nov 2002 22:10:03 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 26 Nov 2002 22:10:03 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id D27A33F30; Tue, 26 Nov 2002 17:09:57 -0500 (EST) Message-ID: <3DE3F135.6030605@redhat.com> Date: Tue, 26 Nov 2002 17:09:57 -0500 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Michael Snyder , Mark Kettenis Cc: gdb-patches@sources.redhat.com, Daniel Jacobowitz Subject: [Fwd: Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)]] Content-Type: multipart/mixed; boundary="------------000302090405000700030207" X-Spam-Status: No, hits=-2.0 required=5.0 tests=FROM_ENDS_IN_NUMS,FWD_MSG,MAILTO_TO_SPAM_ADDR, PATCH_UNIFIED_DIFF,QUOTED_EMAIL_TEXT,SPAM_PHRASE_00_01, USER_AGENT,USER_AGENT_MOZILLA_UA,X_ACCEPT_LANG version=2.41 X-Spam-Level: This is a multi-part message in MIME format. --------------000302090405000700030207 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 368 For the thread maintainers (this was unfortunatly hidden in a gdb@ discussion). ####### ##### ##### # # # # # # # # ###### ##### ### # ### # # # # ### # # ##### ### ##### # DanielJ's off line for a week so someone will also need to commit. Andrew --------------000302090405000700030207 Content-Type: message/rfc822; name="Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)]" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)]" Content-length: 10300 X-Mozilla-Status2: 00000000 Return-Path: Delivered-To: ac131313@localhost.redhat.com Received: from localhost (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 80B943E4B for ; Thu, 21 Nov 2002 19:15:47 -0500 (EST) X-Sieve: cmu-sieve 2.0 Received: from potter.sfbay.redhat.com by localhost with IMAP (fetchmail-5.9.13) for ac131313@localhost (single-drop); Thu, 21 Nov 2002 19:15:47 -0500 (EST) Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by potter.sfbay.redhat.com (8.11.6/8.11.6) with ESMTP id gAM0EoM15521 for ; Thu, 21 Nov 2002 16:14:50 -0800 Received: from mx1.redhat.com (mx1.redhat.com [172.16.48.31]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with SMTP id gAM0EnD00620 for ; Thu, 21 Nov 2002 19:14:49 -0500 Received: from crack.them.org (mail@crack.them.org [65.125.64.184]) by mx1.redhat.com (8.11.6/8.11.6) with SMTP id gALNonP14552 for ; Thu, 21 Nov 2002 18:50:49 -0500 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18F3LX-0008Qe-00; Thu, 21 Nov 2002 20:15:07 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18F1T5-0002uy-00; Thu, 21 Nov 2002 19:14:47 -0500 Date: Thu, 21 Nov 2002 19:14:47 -0500 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb@sources.redhat.com Subject: Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)] Message-ID: <20021122001447.GA7884@nevyn.them.org> Mail-Followup-To: Andrew Cagney , gdb@sources.redhat.com References: <3DDD6150.10407@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3DDD6150.10407@redhat.com> User-Agent: Mutt/1.5.1i Content-length: 8385 On Thu, Nov 21, 2002 at 05:42:24PM -0500, Andrew Cagney wrote: > FYI, > > Too many memory reads/writes was one reason for a ptrace'd threaded > shlib program running slow, I suspect this is the other. Maybe, maybe not... definitely needs to go though! Thanks for such a thorough investigation, it gave me a good idea. > Briefly, the GNU/Linux thread code is giving regcache.c conflicting > stories over which inferior ptid should be in the register cache. As a > consequence, every single register fetch leads to a regcache flush and > re-fetch. Outch! > > > Briefly, core GDB tries to fetch a register. This eventually leads to > the call: > > regcache_raw_read(REGNUM) > > registers_tpid != inferior_tpid > (gdb) print registers_ptid > $6 = {pid = 31263, lwp = 0, tid = 0} > (gdb) print inferior_ptid > $7 = {pid = 31263, lwp = 31263, tid = 0} > -> flush regcache > -> registers_tpid = inferior_tpid > -- at this point regnum is invalid > target_fetch_registers (regnum) > > Since the inferior doesn't match the target, the cache is flushed, > inferior_ptid is updated, and the register is fetched. The fetch flows > on down into the depths of the target and the call: > > Seen the problem yet? Yup. Saw something else very interesting, too. > The long term fix is to have per-thread register caches, that is > progressing. > > I don't know about a short term fix though. I was working on a short-term fix and discovered it was almost entirely in place already. Look at a couple of random fetch_inferior_registers implementations; every one that a GNU/Linux platform uses already will fetch the LWP's registers if the LWP is non-zero. So why not give that to 'em? Leave the inferior_ptid as it is, and make fetch_inferior_registers honor the LWP id. I fixed sparc-nat.c (the fix is a little gross but holds to the style of the file); I fixed up lin-lwp; I timed a couple of tests. Look at what I got. Currently, on my desktop, to run linux-dp.exp and schedlock.exp: runtest linux-dp.exp schedlock.exp 101.06s user 50.80s system 130% cpu 1:56.27 total With change: runtest linux-dp.exp schedlock.exp 92.13s user 48.46s system 131% cpu 1:47.30 total Wait, I'm being foolish. Schedlock has some CPU-intensive loops in it so when you're trying to get CPU information for GDB it's not a good choice. Trying again with linux-dp.exp and print-threads.exp, currently: runtest linux-dp.exp print-threads.exp 17.21s user 48.22s system 82% cpu 1:19.56 total With change: runtest linux-dp.exp print-threads.exp 16.67s user 45.35s system 82% cpu 1:15.27 total Linux-dp also has some spinning and sending of SIGINTs, so again the work done isn't entirely deterministic - but that's still an interesting difference to me. And we're definitely finding the correct registers for each thread, at least on i386-linux; I'm pretty confident about other targets, too. And: it appears to fix the PR. We do so terrifyingly much reading from the inferior that it's kind of hard to tell; but normally it would have crashed by now. And I can "info threads" and print out $eip without it looping again. So, thoughts on the attached patch? -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2002-11-21 Daniel Jacobowitz Fix PR gdb/725 * lin-lwp.c (lin_lwp_fetch_registers): Remove. (lin_lwp_store_registers): Remove. (init_lin_lwp_ops): Use fetch_inferior_registers and store_inferior_registers directly. * sparc-nat.c (fetch_inferior_registers): Honor LWP ID. (store_inferior_registers): Likewise. Index: lin-lwp.c =================================================================== RCS file: /cvs/src/src/gdb/lin-lwp.c,v retrieving revision 1.36 diff -u -p -r1.36 lin-lwp.c --- lin-lwp.c 31 Oct 2002 21:00:08 -0000 1.36 +++ lin-lwp.c 22 Nov 2002 00:12:14 -0000 @@ -1343,32 +1343,6 @@ lin_lwp_mourn_inferior (void) child_ops.to_mourn_inferior (); } -static void -lin_lwp_fetch_registers (int regno) -{ - struct cleanup *old_chain = save_inferior_ptid (); - - if (is_lwp (inferior_ptid)) - inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid)); - - fetch_inferior_registers (regno); - - do_cleanups (old_chain); -} - -static void -lin_lwp_store_registers (int regno) -{ - struct cleanup *old_chain = save_inferior_ptid (); - - if (is_lwp (inferior_ptid)) - inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid)); - - store_inferior_registers (regno); - - do_cleanups (old_chain); -} - static int lin_lwp_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write, struct mem_attrib *attrib, @@ -1426,8 +1400,10 @@ init_lin_lwp_ops (void) lin_lwp_ops.to_detach = lin_lwp_detach; lin_lwp_ops.to_resume = lin_lwp_resume; lin_lwp_ops.to_wait = lin_lwp_wait; - lin_lwp_ops.to_fetch_registers = lin_lwp_fetch_registers; - lin_lwp_ops.to_store_registers = lin_lwp_store_registers; + /* fetch_inferior_registers and store_inferior_registers will + honor the LWP id, so we can use them directly. */ + lin_lwp_ops.to_fetch_registers = fetch_inferior_registers; + lin_lwp_ops.to_store_registers = store_inferior_registers; lin_lwp_ops.to_xfer_memory = lin_lwp_xfer_memory; lin_lwp_ops.to_kill = lin_lwp_kill; lin_lwp_ops.to_create_inferior = lin_lwp_create_inferior; Index: sparc-nat.c =================================================================== RCS file: /cvs/src/src/gdb/sparc-nat.c,v retrieving revision 1.15 diff -u -p -r1.15 sparc-nat.c --- sparc-nat.c 14 Nov 2002 20:37:29 -0000 1.15 +++ sparc-nat.c 22 Nov 2002 00:12:14 -0000 @@ -1,5 +1,6 @@ /* Functions specific to running gdb native on a SPARC running SunOS4. - Copyright 1989, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000, 2001 + Copyright 1989, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000, 2001, + 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -58,6 +59,15 @@ fetch_inferior_registers (int regno) struct regs inferior_registers; struct fp_status inferior_fp_registers; int i; + int fetch_pid; + +#ifdef __linux__ + fetch_pid = TIDGET (inferior_ptid); + if (fetch_pid == 0) + fetch_pid = PIDGET (inferior_ptid); +#else + fetch_pid = PIDGET (inferior_ptid); +#endif /* We should never be called with deferred stores, because a prerequisite for writing regs is to have fetched them all (PREPARE_TO_STORE), sigh. */ @@ -75,7 +85,7 @@ fetch_inferior_registers (int regno) || regno >= Y_REGNUM || (!deprecated_register_valid[SP_REGNUM] && regno < I7_REGNUM)) { - if (0 != ptrace (PTRACE_GETREGS, PIDGET (inferior_ptid), + if (0 != ptrace (PTRACE_GETREGS, fetch_pid, (PTRACE_ARG3_TYPE) & inferior_registers, 0)) perror ("ptrace_getregs"); @@ -108,7 +118,7 @@ fetch_inferior_registers (int regno) regno == FPS_REGNUM || (regno >= FP0_REGNUM && regno <= FP0_REGNUM + 31)) { - if (0 != ptrace (PTRACE_GETFPREGS, PIDGET (inferior_ptid), + if (0 != ptrace (PTRACE_GETFPREGS, fetch_pid, (PTRACE_ARG3_TYPE) & inferior_fp_registers, 0)) perror ("ptrace_getfpregs"); @@ -153,6 +163,15 @@ store_inferior_registers (int regno) struct regs inferior_registers; struct fp_status inferior_fp_registers; int wanna_store = INT_REGS + STACK_REGS + FP_REGS; + int store_pid; + +#ifdef __linux__ + store_pid = TIDGET (inferior_ptid); + if (store_pid == 0) + store_pid = PIDGET (inferior_ptid); +#else + store_pid = PIDGET (inferior_ptid); +#endif /* First decide which pieces of machine-state we need to modify. Default for regno == -1 case is all pieces. */ @@ -236,7 +255,7 @@ store_inferior_registers (int regno) inferior_registers.r_y = *(int *) &deprecated_registers[REGISTER_BYTE (Y_REGNUM)]; - if (0 != ptrace (PTRACE_SETREGS, PIDGET (inferior_ptid), + if (0 != ptrace (PTRACE_SETREGS, store_pid, (PTRACE_ARG3_TYPE) & inferior_registers, 0)) perror ("ptrace_setregs"); } @@ -252,7 +271,7 @@ store_inferior_registers (int regno) &deprecated_registers[REGISTER_BYTE (FPS_REGNUM)], sizeof (FPU_FSR_TYPE)); if (0 != - ptrace (PTRACE_SETFPREGS, PIDGET (inferior_ptid), + ptrace (PTRACE_SETFPREGS, store_pid, (PTRACE_ARG3_TYPE) & inferior_fp_registers, 0)) perror ("ptrace_setfpregs"); } --------------000302090405000700030207-- --------------090809040308030407020108--