From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8502 invoked by alias); 12 Jun 2004 09:35:01 -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 8493 invoked from network); 12 Jun 2004 09:34:59 -0000 Received: from unknown (HELO aragorn.inter.net.il) (192.114.186.23) by sourceware.org with SMTP; 12 Jun 2004 09:34:59 -0000 Received: from zaretski ([80.230.141.54]) by aragorn.inter.net.il (MOS 3.4.6-GR) with ESMTP id DBN06338; Sat, 12 Jun 2004 12:34:49 +0300 (IDT) Date: Sat, 12 Jun 2004 09:35:00 -0000 From: "Eli Zaretskii" To: Jeff Johnston Message-Id: <8011-Sat12Jun2004123150+0300-eliz@gnu.org> CC: gdb-patches@sources.redhat.com In-reply-to: <40CA20B0.4060106@redhat.com> (message from Jeff Johnston on Fri, 11 Jun 2004 17:14:24 -0400) Subject: Re: [RFC]: x86 threaded watchpoint support [1/3] Reply-to: Eli Zaretskii References: <40CA20B0.4060106@redhat.com> X-SW-Source: 2004-06/txt/msg00284.txt.bz2 > Date: Fri, 11 Jun 2004 17:14:24 -0400 > From: Jeff Johnston > > /* Mirror the inferior's DRi registers. We keep the status and > - control registers separated because they don't hold addresses. */ > + control registers separated because they don't hold addresses. > + > + Note that the DR_STATUS register is thread-specific and the > + mirror value should be refreshed as necessary. */ > static CORE_ADDR dr_mirror[DR_NADDR]; If the DR_STATUS register is thread specific, I think the mirroring should be redesigned to support that out of the box. The solution you suggest sounds like working around the existing code, but there's no reason to do that, IMHO. > +extern ptid_t trap_ptid; > +extern ptid_t inferior_ptid; As Mark points out, this will break any x86 target that doesn't define these in its source files. So this solution is not a good one. A better solution would be to define target-specific accessor functions that would return these values. > + ptid_t saved_ptid = inferior_ptid; > > + /* Debug status register is thread-specific. A watchpoint will > + cause a trap to occur. Switch to trap ptid to get relevant > + status for that thread. */ > + inferior_ptid = trap_ptid; > dr_status_mirror = I386_DR_LOW_GET_STATUS (); > + inferior_ptid = saved_ptid; Yikes! Why not redesign I386_DR_LOW_GET_STATUS so that it could accept the required thread id directly? Bottom line: I think we need to redesign the low-level watchpoint support for x86 so that it supports per-thread hardware watchpoints. (I actually raised this issue back when the current code was designed and was told to disregard it. Now it came back to haunt us.)