Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Eli Zaretskii" <eliz@gnu.org>
To: Jeff Johnston <jjohnstn@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFC]: x86 threaded watchpoint support [1/3]
Date: Sat, 12 Jun 2004 09:35:00 -0000	[thread overview]
Message-ID: <8011-Sat12Jun2004123150+0300-eliz@gnu.org> (raw)
In-Reply-To: <40CA20B0.4060106@redhat.com> (message from Jeff Johnston on Fri, 11 Jun 2004 17:14:24 -0400)

> Date: Fri, 11 Jun 2004 17:14:24 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> 
>  /* 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.)


      parent reply	other threads:[~2004-06-12  9:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-06-11 21:14 Jeff Johnston
2004-06-11 22:00 ` Mark Kettenis
2004-06-11 22:46   ` Daniel Jacobowitz
2004-06-12 17:12     ` Mark Kettenis
2004-06-14 18:28     ` Jeff Johnston
2004-06-12  9:35 ` Eli Zaretskii [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8011-Sat12Jun2004123150+0300-eliz@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=jjohnstn@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox