Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Pedro Alves <pedro@codesourcery.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Don't delete local watchpoints just because a different thread  stopped.
Date: Thu, 19 Nov 2009 17:46:00 -0000	[thread overview]
Message-ID: <20091119174539.GA24336@caradoc.them.org> (raw)
In-Reply-To: <200911190207.02432.pedro@codesourcery.com>

On Thu, Nov 19, 2009 at 02:07:01AM +0000, Pedro Alves wrote:
> One could even come up with a test case where the current
> frame id on this other thread does happen to be in the
> frame chain.  Shouldn't be hard to trigger if the threads
> are running the same function --- GDB can then mistakenly
> try to extract a new current watchpoint value on the wrong
> thread context.

I hope your threads have different stacks :-)  So I don't think this
case happens.

> Index: src/gdb/breakpoint.h
> ===================================================================
> --- src.orig/gdb/breakpoint.h	2009-11-19 01:10:05.000000000 +0000
> +++ src/gdb/breakpoint.h	2009-11-19 01:10:16.000000000 +0000
> @@ -457,6 +457,10 @@ struct breakpoint
>         should be evaluated on the outermost frame.  */
>      struct frame_id watchpoint_frame;
>  
> +    /* Holds the thread which identifies the frame this watchpoint
> +       should be considered in scope for, or -1 if don't care.  */
> +    int watchpoint_thread;
> +
>      /* For hardware watchpoints, the triggered status according to the
>         hardware.  */
>      enum watchpoint_triggered watchpoint_triggered;

Why not just use a ptid?  If the answer has to do with ptid reuse,
then we ought to delete the watchpoint before that comes up - anyway,
at least deserves a comment of what units this is in.

> @@ -1004,6 +1028,12 @@ update_watchpoint (struct breakpoint *b,
>    bpstat bs;
>    struct program_space *frame_pspace;
>  
> +  /* If this is a local watchpoint, we only want to check if the
> +     watchpoint frame is in scope if the current thread is the thread
> +     that was used to create the watchpoint.  */
> +  if (!watchpoint_thread_match (b))
> +    return;
> +
>    /* We don't free locations.  They are stored in bp_location array and
>       update_global_locations will eventually delete them and remove
>       breakpoints if needed.  */

For all-stop, do we want to check whenever the watchpoint's thread is
stopped?

-- 
Daniel Jacobowitz
CodeSourcery


  reply	other threads:[~2009-11-19 17:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-19  2:07 Pedro Alves
2009-11-19 17:46 ` Daniel Jacobowitz [this message]
2009-11-20  0:31   ` Pedro Alves
2009-11-20  1:14     ` Daniel Jacobowitz
2009-11-20  1:33       ` Pedro Alves
2009-11-20  3:47         ` Daniel Jacobowitz

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=20091119174539.GA24336@caradoc.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.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