public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
Cc: gdb-patches@sourceware.org,  "'Joel Brobecker'" <brobecker@adacore.com>
Subject: Re: [RFA- v2] Testcase for bug report 11531 and fix for Solaris
Date: Sun, 25 Apr 2010 20:10:00 -0000	[thread overview]
Message-ID: <201004252110.20098.pedro@codesourcery.com> (raw)
In-Reply-To: <002101cae3c0$a56276c0$f0276440$@muller@ics-cnrs.unistra.fr>

On Saturday 24 April 2010 16:13:06, Pierre Muller wrote:
>     Work around the problem by removing hardware watchpoints if a step is
>     requested, GDB will check for a hardware watchpoint trigger after the
>     step anyway.  */
> -#define CANNOT_STEP_HW_WATCHPOINTS
> +/* The code related to this macro does not work
> +   anymore. Thus we remove this macro definition.  */
> +/* #define CANNOT_STEP_HW_WATCHPOINTS */

The new comment isn't correct.  I'd like to clarify this, at least
for the archives.

It's not that the code does not work anymore.  The _main_ issue the
workaround handles, is the fact that on older Solaris kernels, when
stepping around a page that has a watchpoint installed, the kernel
would forget the step, and hence, the inferior would run free.  The
workaround should still be preventing that as is.

What no longer works since the workaround was written, is that GDB
used to check if there was any watchpoint that triggered after
all single-steps, regardless of whether the target indicating common
code a watchpoint triggered or not, hence PR 11531.

Obviously, having the inferior randomly running free when the user
does "step" or "next" is worse than missing a watchpoint when
stepping.

If we still wanted to support Solaris <= 2.7, we'd
still want the workaround in some form.  So, it's not that 
it "doesn't work anymore" -- it's that the current workaround
implementation is incomplete because it breaks something else. 
For example, we'd need to somehow make GDB core check for
watchpoints after every single-step on this target.  And, we
would indeed want to make this conditional on the currently
running kernel version, because although it should be relatively
simple to workaround PR11531 (regular watchpoints), by always
checking for watched value changes after all single-steps,
read and write watchpoints, would be harder to unbreak.
(yes, procfs.c doesn't report the stopped data address today,
so read and write watchpoints don't work anyway, but it could
report it, as Solaris' procfs does support that, IIRC).

As nobody said they're still interested in those older Solaris'
kernel, let's indeed just go the simple route of removing the whole
workaround, and not leave that code commented out (as Joel suggested),
but please let's keep finally removing the nm file and the
associated glue for a separate patch.

-- 
Pedro Alves

  parent reply	other threads:[~2010-04-25 20:10 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-23 16:41 [RFA] Testcase for bug report 11531 Pierre Muller
2010-04-23 17:29 ` Joel Brobecker
2010-04-23 18:16   ` Pedro Alves
2010-04-23 18:25     ` Joel Brobecker
2010-04-24 15:13     ` [RFA- v2] Testcase for bug report 11531 and fix for Solaris Pierre Muller
2010-04-25 13:20       ` Joel Brobecker
2010-04-26 11:24         ` [RFA- v3] " Pierre Muller
2010-04-26 16:49           ` Joel Brobecker
2010-04-26 20:50             ` Pierre Muller
2010-04-25 20:10       ` Pedro Alves [this message]
2010-04-26 10:55         ` [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531) Pierre Muller
2010-04-26 11:26           ` Pedro Alves
2010-04-26 11:50             ` Pierre Muller
2010-04-26 11:56               ` Pedro Alves
2010-04-26 12:03                 ` Pierre Muller
2010-04-26 11:29           ` Mark Kettenis
2010-04-26 11:52             ` Pierre Muller

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=201004252110.20098.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pierre.muller@ics-cnrs.unistra.fr \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).