From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16306 invoked by alias); 11 Jun 2004 22:46:12 -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 16187 invoked from network); 11 Jun 2004 22:46:02 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 11 Jun 2004 22:46:02 -0000 Received: from drow by nevyn.them.org with local (Exim 4.34 #1 (Debian)) id 1BYumd-0002cV-Pp; Fri, 11 Jun 2004 18:45:59 -0400 Date: Fri, 11 Jun 2004 22:46:00 -0000 From: Daniel Jacobowitz To: Mark Kettenis Cc: jjohnstn@redhat.com, gdb-patches@sources.redhat.com Subject: Re: [RFC]: x86 threaded watchpoint support [1/3] Message-ID: <20040611224559.GA10014@nevyn.them.org> Mail-Followup-To: Mark Kettenis , jjohnstn@redhat.com, gdb-patches@sources.redhat.com References: <40CA20B0.4060106@redhat.com> <200406112200.i5BM0JBA000385@elgar.kettenis.dyndns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200406112200.i5BM0JBA000385@elgar.kettenis.dyndns.org> User-Agent: Mutt/1.5.5.1+cvs20040105i X-SW-Source: 2004-06/txt/msg00282.txt.bz2 On Sat, Jun 12, 2004 at 12:00:19AM +0200, Mark Kettenis wrote: > Date: Fri, 11 Jun 2004 17:14:24 -0400 > From: Jeff Johnston > > The following patch gets threaded watchpoint support working for > the x86. On x86 linux, the dr_status register is thread-specific. > This means that the current method which uses the PID to call > PTRACE is wrong. I have changed this to use the current lwp for > the inferior_ptid. Corresponding to this, the > i386_stopped_data_address function switches the inferior_ptid to > the trap_ptid. Thus, we can see if we really stopped for a > watchpoint or hardware breakpoint. > > I'm not surprised that the current stuff is wrong. However, have you > verified that the dr_status register is thread-specific for *all* > versions of GNU/Linux that we support and not just the RedHat kernel > that you're working with? I'm pretty sure - GNU/Linux has never had a concept of "process registers", since threads have evolved from processes rather than the other way around. > Because the thread-db.c code changes the trap_ptid into a > high-level ptid (pid + tid), I had to add a new target vector > interface which gives back the lwp for a given ptid. This is used > by the low level dr get routine. > > Really? Doesn't TIDGET work for you? TIDGET at this point is the thread ID, i.e. internal state from NPTL or LinuxThreads. It's not the LWP id that we need in order to all ptrace. It sounds like this new vector is really the death blow to thread-db.c. Maybe I should see how much of it we can throw away. Copious free time and all that. > Index: lin-lwp.c > =================================================================== > RCS file: /cvs/src/src/gdb/lin-lwp.c,v > retrieving revision 1.55 > diff -u -p -r1.55 lin-lwp.c > --- lin-lwp.c 25 May 2004 14:58:28 -0000 1.55 > +++ lin-lwp.c 11 Jun 2004 20:55:28 -0000 > @@ -1200,21 +1200,29 @@ child_wait (ptid_t ptid, struct target_w > } > > /* Handle GNU/Linux's extended waitstatus for trace events. */ > - if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP > - && status >> 16 != 0) > + if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP) > { > - linux_handle_extended_wait (pid, status, ourstatus); > + /* Set trap_ptid like lin_lwp_wait does. This is needed > + for watchpoint support. For example, the x86 linux > + watchpoints need to know what thread an event occurred > + on so as to read the correct thread-specific status. */ > + trap_ptid = pid_to_ptid (pid); > > - /* If we see a clone event, detach the child, and don't > - report the event. It would be nice to offer some way to > - switch into a non-thread-db based threaded mode at this > - point. */ > - if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS) > + if (status >> 16 != 0) > > What's this shift supposed to do? This one's my fault so I'll answer for him: it was already there, up a couple lines in the diff. It's for the extended waitstatus support. I must say that I'm not thrilled by having trap_ptid leak out into yet more files. It's an extremely underspecified interface. -- Daniel Jacobowitz