public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Testcase for bug report 11531
@ 2010-04-23 16:41 Pierre Muller
  2010-04-23 17:29 ` Joel Brobecker
  0 siblings, 1 reply; 17+ messages in thread
From: Pierre Muller @ 2010-04-23 16:41 UTC (permalink / raw)
  To: gdb-patches

  Test case for the newly discovered
problem with the old CANNOT_STEP_HW_WATCHPOINTS macro
for recent Solaris.

  This leads to two failures on current x86_64-pc-solaris-2.11
(none of x86_64-pc-linux)
that disappear if I undefined the macro.

  Is it possible to commit this first or
should we first find a way to solve the problem?

  I could add a single line diff that would remove 
the macro definition in config/i386/nm-i386-sol2.h.

  The other question after is what to do with the code
and comments related to CANNOT_STEP_HW_WATCHPOINTS macro
in infrun.c source.

Pierre Muller
Pascal language support maintainer for GDB

2010-04-23  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR breakpoints/11531.
	* testsuite/gdb.base/gdb11531.c: New file.
	* testsuite/gdb.base/gdb11531.exp: New file.

Index: src/gdb/testsuite/gdb.base/gdb11531.c
===================================================================
RCS file: testsuite/gdb.base/gdb11531.c
diff -N testsuite/gdb.base/gdb11531.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.base/gdb11531.c	23 Apr 2010 16:17:25 -0000
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   Contributed by Pierre Muller.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Test for Solaris specific watchpoint problem.  */
+
+struct rec
+{
+ int x;
+ double y;
+ int z;
+};
+
+static struct rec myrec;
+
+int
+main ()
+{
+  myrec.x = 5;
+  myrec.y = 3.4;
+  myrec.z = 56;
+  myrec.x = 78;
+  return myrec.x;
+}
+
Index: src/gdb/testsuite/gdb.base/gdb11531.exp
===================================================================
RCS file: testsuite/gdb.base/gdb11531.exp
diff -N testsuite/gdb.base/gdb11531.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.base/gdb11531.exp	23 Apr 2010 16:17:25 -0000
@@ -0,0 +1,59 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB for Solaris specific watchpoint problem.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "gdb11531"
+
+if { [prepare_for_testing $testfile.exp $testfile $testfile.c {debug}] } {
+    return -1;
+}
+
+
+# Do not use run_to main
+# as this sets a breakpoint at main.
+# If the breakpoint is at the same instruction as the 
+# watchpoint value assignment
+# you can fall into the problem of the stepping over the breakpoint
+# location that can also trigger a watchpoint miss
+# This is not the problem reported here.
+
+gdb_test "start" ".*Temporary breakpoint.*"
+
+gdb_test "watch myrec.x" ".*atchpoint \[0-9\]+: myrec\.x" "Set watchpoint "
+
+gdb_test "next" \
+    "Old value = 0.*New value = 5.*" \
+    "watchpoint variable triggers at next"
+
+gdb_test "continue" \
+    "Old value = .*New value = 78.*" \
+    "watchpoint variable triggers at continue"
+
+gdb_test "continue" ".*Program exited.*" "Continue to program exit"  
+
+gdb_test "start" "" "restart"
+
+gdb_test "next" "Old value = 0.*New value = 5.*" "watchpoint triggers after
second start"
+

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA] Testcase for bug report 11531
  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
  0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2010-04-23 17:29 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches

> +# Do not use run_to main
> +# as this sets a breakpoint at main.
> +# If the breakpoint is at the same instruction as the 
> +# watchpoint value assignment
> +# you can fall into the problem of the stepping over the breakpoint
> +# location that can also trigger a watchpoint miss
> +# This is not the problem reported here.
> +
> +gdb_test "start" ".*Temporary breakpoint.*"

Unfortunately, this does not work when testing with the remote protocol.
Do use "runto", but use "*main" instead of "main", or something like
that. That way, you should always stop before the instruction that
causes the watchpoint to trigger.

> +gdb_test "next" \
> +    "Old value = 0.*New value = 5.*" \
> +    "watchpoint variable triggers at next"
> +
> +gdb_test "continue" \
> +    "Old value = .*New value = 78.*" \
> +    "watchpoint variable triggers at continue"

This is a bit on the paranoid side, but can we use end-of-line
expressions rather than ".*" to match the line break? For instance,
I often use:

    set eol "\[\r\n\]+"
    gdb_test "next" \
        "Old value = 0${eol}New value = 5${eol}" \
        "watchpoint variable triggers at next"

It's unlikely that you'll have any other number than zero if the
leading digit is zero - but conceivably, we could print the new
value as 55 and still pass the test...

I did suggest at one time to provide a routine that takes a list of
regexp strings, and builds a regexp string joined with the typical
new-line regexp.  That way, I could write the test as follow:

    test_gdb_complete "pck" \
                      [multi_line "(p pck\\.ad\[sb\])?" \
                                  "(p pck\\.ad\[sb\])?" \
                                  "p pck.external_identical_one" \
                                  "p pck.inner.inside_variable" \
                                  "p pck.local_identical_one" \
                                  "p pck.local_identical_two" \
                                  "p pck.my_global_variable" \
                                  "p pck.proc" ]


Daniel made another suggestion, which I needed time to investigate
and yet never did :-(.

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA] Testcase for bug report 11531
  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
  0 siblings, 2 replies; 17+ messages in thread
From: Pedro Alves @ 2010-04-23 18:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker, Pierre Muller

On Friday 23 April 2010 18:29:33, Joel Brobecker wrote:
> > +gdb_test "start" ".*Temporary breakpoint.*"
> 
> Unfortunately, this does not work when testing with the remote protocol.
> Do use "runto", but use "*main" instead of "main", or something like
> that. That way, you should always stop before the instruction that
> causes the watchpoint to trigger.
> 

Or avoid those tricks, and simply put another statement before
the one that triggers the watchpoint:

int
main ()
{
+  myrec.y = 1;       <<<<<< runto main stops here instead.
  myrec.x = 5;
  myrec.y = 3.4;


-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA] Testcase for bug report 11531
  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
  1 sibling, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2010-04-23 18:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Pierre Muller

> Or avoid those tricks, and simply put another statement before
> the one that triggers the watchpoint:

Sometimes, you're so locked on your target that you forget about
the bigger picture :-).

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFA- v2] Testcase for bug report 11531 and fix for Solaris
  2010-04-23 18:16   ` Pedro Alves
  2010-04-23 18:25     ` Joel Brobecker
@ 2010-04-24 15:13     ` Pierre Muller
  2010-04-25 13:20       ` Joel Brobecker
  2010-04-25 20:10       ` [RFA- v2] " Pedro Alves
  1 sibling, 2 replies; 17+ messages in thread
From: Pierre Muller @ 2010-04-24 15:13 UTC (permalink / raw)
  To: 'Pedro Alves', gdb-patches; +Cc: 'Joel Brobecker'

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Friday, April 23, 2010 8:16 PM
> À : gdb-patches@sourceware.org
> Cc : Joel Brobecker; Pierre Muller
> Objet : Re: [RFA] Testcase for bug report 11531
> 
> On Friday 23 April 2010 18:29:33, Joel Brobecker wrote:
> > > +gdb_test "start" ".*Temporary breakpoint.*"
> >
> > Unfortunately, this does not work when testing with the remote
> protocol.
> > Do use "runto", but use "*main" instead of "main", or something like
> > that. That way, you should always stop before the instruction that
> > causes the watchpoint to trigger.
> >
> 
> Or avoid those tricks, and simply put another statement before
> the one that triggers the watchpoint:
> 
> int
> main ()
> {
> +  myrec.y = 1;       <<<<<< runto main stops here instead.


  I found another way: 
simply remove all breakpoints before 
setting the watchpoint.
 Tested with cygwin GDB and GDBserver,
both give 6 PASS.
 On a i386 solaris 32-bit GDB executable,
I get two failures that disappear
once I remove the macro from nm-i386-sol2.h.

Pierre Muller

PS1: Once this macro is removed, I
will submit a RFA for the complete removal 
of nm-i386-sol2.h.
PS2: The code inside infrun.c ought to 
be changed as well. If it is now non-functional
I propose to generate a compilation error
if the macro is still defined, this way
third party users will not be surprised that
it does not work...


ChangeLog entry:
2010-04-24  Pierre Muller  <muller@ics.u-strasbg.fr>

	* config/i386/nm-i386-sol2.h (CANNOT_STEP_HW_WATCHPOINTS):
	Remove macro.

testsuite/ChangeLog entry:

	PR breakpoints/11531.
	* testsuite/gdb.base/gdb11531.c: New file.
	* testsuite/gdb.base/gdb11531.exp: New file.

Index: config/i386/nm-i386sol2.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386sol2.h,v
retrieving revision 1.19
diff -u -p -r1.19 nm-i386sol2.h
--- config/i386/nm-i386sol2.h   1 Jan 2010 07:31:48 -0000       1.19
+++ config/i386/nm-i386sol2.h   24 Apr 2010 15:07:54 -0000
@@ -27,6 +27,8 @@
    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 */

 #endif /* NEW_PROC_API */
Index: testsuite/gdb.base/gdb11531.c
===================================================================
RCS file: testsuite/gdb.base/gdb11531.c
diff -N testsuite/gdb.base/gdb11531.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gdb11531.c	24 Apr 2010 13:20:02 -0000
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   Contributed by Pierre Muller.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Test for Solaris specific watchpoint problem.  */
+
+struct rec
+{
+ int x;
+ double y;
+ int z;
+};
+
+static struct rec myrec;
+
+int
+main ()
+{
+  myrec.x = 5;
+  myrec.y = 3.4;
+  myrec.z = 56;
+  myrec.x = 78;
+  return myrec.x;
+}
+
Index: testsuite/gdb.base/gdb11531.exp
===================================================================
RCS file: testsuite/gdb.base/gdb11531.exp
diff -N testsuite/gdb.base/gdb11531.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gdb11531.exp	24 Apr 2010 13:20:02 -0000
@@ -0,0 +1,63 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB stabs problem with qualified parameter of forward types.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "gdb11531"
+
+if { [prepare_for_testing $testfile.exp $testfile $testfile.c {debug}] } {
+    return -1;
+}
+
+
+if { ![runto main] } then {
+    fail "run to main"
+    return
+}
+
+# If the breakpoint is at the same instruction as the 
+# watchpoint value assignment
+# you can fall into the problem of the stepping over the breakpoint
+# location that can also trigger a watchpoint miss
+# This is not the problem reported here.
+# Thus we remove all breakpoints first.
+
+delete_breakpoints
+
+gdb_test "watch myrec.x" ".*atchpoint \[0-9\]+: myrec\.x" "Set watchpoint "
+
+gdb_test "stepi" \
+    "Old value = 0.*New value = 5.*" \
+    "watchpoint variable triggers at next"
+
+gdb_test "continue" \
+    "Old value = .*New value = 78.*" \
+    "watchpoint variable triggers at continue"
+
+gdb_test "continue" ".*Program exited.*" "Continue to program exit"  
+
+gdb_test "start" "" "restart"
+
+gdb_test "next" "Old value = 0.*New value = 5.*" "watchpoint triggers after
second start"
+

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v2] Testcase for bug report 11531 and fix for Solaris
  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-25 20:10       ` [RFA- v2] " Pedro Alves
  1 sibling, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2010-04-25 13:20 UTC (permalink / raw)
  To: Pierre Muller; +Cc: 'Pedro Alves', gdb-patches

> PS1: Once this macro is removed, I will submit a RFA for the complete
> removal of nm-i386-sol2.h.
> PS2: The code inside infrun.c ought to be changed as well. If it is
> now non-functional I propose to generate a compilation error if the
> macro is still defined, this way third party users will not be
> surprised that it does not work...

Since nm-i386-sol2.h was the last user of the CANNOT_STEP_HW_WATCHPOINTS
macro, we should just purge it entirely.  We cannot worry about third
parties that did not contribute their code (that includes people like me
who still has lots of uncontributed code in the AdaCore version of GDB).

> ChangeLog entry:
> 2010-04-24  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* config/i386/nm-i386-sol2.h (CANNOT_STEP_HW_WATCHPOINTS):
> 	Remove macro.

As per the above, let's just purge the whole thing. You can, at your
convinience, just delete the line from the nm file and do the rest
of the purge separately, so do everything all in one patch.

> testsuite/ChangeLog entry:
> 
> 	PR breakpoints/11531.
> 	* testsuite/gdb.base/gdb11531.c: New file.
> 	* testsuite/gdb.base/gdb11531.exp: New file.


> +# If the breakpoint is at the same instruction as the 
> +# watchpoint value assignment
> +# you can fall into the problem of the stepping over the breakpoint
> +# location that can also trigger a watchpoint miss
> +# This is not the problem reported here.
> +# Thus we remove all breakpoints first.

Suggest a different wording:

# The breakpoint is probably at the instruction where the value being
# watched (myrec.x) gets updated.  This is the instruction where we
# expect to receive a watchpoint notification when we do the "stepi"
# below.  However, having the breakpoint at the same location as this
# intruction can possibly interfere with our testcase, as stepping
# over the breakpoint in order to get past it may incorrectly lead
# to the debugger missing the watchpoint hit.  This would be a bug
# in GDB, but this is not the bug that we are trying to test here.
# So, we remove all breakpoints first.

> +gdb_test "stepi" \
> +    "Old value = 0.*New value = 5.*" \
> +    "watchpoint variable triggers at next"

I would prefer that you would verify that the old value and new values
match exactly. The wildcard matching prevents that, so the test would
still pass even if the debugger printed that the new value was 51274793875.
It seems that the use of a wildcard here is just a cheap way of matching
a new-line sequence - the typical way of matching new-lines, I've found,
is to use "\[\r\n\]+". It's been helpful to define a variable with a short
name that contains that regexp and use that variable, rather than repeat
manually the new-line regexp...

> +gdb_test "start" "" "restart"
> +
> +gdb_test "next" "Old value = 0.*New value = 5.*" "watchpoint triggers after
> second start"

Can you explain the purpose of this part of the testcase. Again, the use
of the start command in a gdb_test does not work with some configurations.

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v2] Testcase for bug report 11531 and fix for Solaris
  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-25 20:10       ` Pedro Alves
  2010-04-26 10:55         ` [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531) Pierre Muller
  1 sibling, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2010-04-25 20:10 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches, 'Joel Brobecker'

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  2010-04-25 20:10       ` [RFA- v2] " Pedro Alves
@ 2010-04-26 10:55         ` Pierre Muller
  2010-04-26 11:26           ` Pedro Alves
  2010-04-26 11:29           ` Mark Kettenis
  0 siblings, 2 replies; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 10:55 UTC (permalink / raw)
  To: 'Pedro Alves', 'Eli Zaretskii'
  Cc: gdb-patches, 'Joel Brobecker'



> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Sunday, April 25, 2010 10:10 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org; 'Joel Brobecker'
> Objet : Re: [RFA- v2] Testcase for bug report 11531 and fix for Solaris
> 
> 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.

  Thanks!
 
> 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.

  OK, that is also what I understood.
 
> 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).
  I have a patch ready for that, but 
didn't want to send it before this was resolved.


> 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.


  In that case, the configure.tgt script should probably report 
that Solaris versions <= 2.7 are not supported anymore.
This is what I added to configure.tgt below.


  Is this patch OK?

  Eli, is the simple removal from gdbint.texinfo OK, or should we
state in the internal docs that this macro was removed?

Pierre

PS: As agreed, I will wait until that part is removed
before resending the patch that removes nm-i386-sol2.h


  
ChangeLog entry:
2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR breakpoints/11531.
	* configure.tgt (Solaris target): Restrict support
	to versions >= 2.8.
	* infrun.c (CANNOT_STEP_HW_WATCHPOINTS): Remove macro.
	(resume): Remove code and comment related to this macro.

doc ChangeLog entry:

2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdbint.texinfo (CANNOT_STEP_HW_WATCHPOINTS): Remove explanation
	of macro deleted from GDB code.

Index: src/gdb/configure.tgt
===================================================================
RCS file: /cvs/src/src/gdb/configure.tgt,v
retrieving revision 1.231
diff -u -p -r1.231 configure.tgt
--- src/gdb/configure.tgt	20 Apr 2010 00:21:33 -0000	1.231
+++ src/gdb/configure.tgt	26 Apr 2010 07:47:51 -0000
@@ -197,8 +197,10 @@ i[34567]86-*-solaris2.1[0-9]* | x86_64-*
 			i386-sol2-tdep.o sol2-tdep.o \
 			corelow.o solib.o solib-svr4.o"
 	;;
-i[34567]86-*-solaris*)
-	# Target: Solaris x86
+i[34567]86-*-solaris2.[8-9] )
+	# Target: Solaris x86, only versions 2.8 and 2.9
+	# Versions <= 2.7 suffer a bug that was handled in older versions of

+	# GDB, but that code was removed.
 	gdb_target_obs="i386-tdep.o i387-tdep.o i386-sol2-tdep.o sol2-tdep.o
\
 			corelow.o solib.o solib-svr4.o"
 	;;
Index: src/gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.435
diff -u -p -r1.435 infrun.c
--- src/gdb/infrun.c	25 Mar 2010 20:48:53 -0000	1.435
+++ src/gdb/infrun.c	26 Apr 2010 07:35:07 -0000
@@ -179,16 +179,6 @@ show_debug_infrun (struct ui_file *file,
 #endif
 
 
-/* Convert the #defines into values.  This is temporary until wfi control
-   flow is completely sorted out.  */
-
-#ifndef CANNOT_STEP_HW_WATCHPOINTS
-#define CANNOT_STEP_HW_WATCHPOINTS 0
-#else
-#undef  CANNOT_STEP_HW_WATCHPOINTS
-#define CANNOT_STEP_HW_WATCHPOINTS 1
-#endif
-
 /* Tables of how to react to signals; the user sets them.  */
 
 static unsigned char *signal_stop;
@@ -1484,18 +1474,6 @@ resume (int step, enum target_signal sig
 			"trap_expected=%d\n",
  			step, sig, tp->trap_expected);
 
-  /* Some targets (e.g. Solaris x86) have a kernel bug when stepping
-     over an instruction that causes a page fault without triggering
-     a hardware watchpoint. The kernel properly notices that it shouldn't
-     stop, because the hardware watchpoint is not triggered, but it forgets
-     the step request and continues the program normally.
-     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.  */
-  if (CANNOT_STEP_HW_WATCHPOINTS && step)
-    remove_hw_watchpoints ();
-
-
   /* Normally, by the time we reach `resume', the breakpoints are either
      removed or inserted, as appropriate.  The exception is if we're
sitting
      at a permanent breakpoint; we need to step over it, but permanent
Index: src/gdb/doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.321
diff -u -p -r1.321 gdbint.texinfo
--- src/gdb/doc/gdbint.texinfo	10 Mar 2010 18:20:07 -0000	1.321
+++ src/gdb/doc/gdbint.texinfo	26 Apr 2010 07:40:23 -0000
@@ -781,11 +781,6 @@ inferior after a watchpoint has been hit
 when watchpoints trigger at the instruction following an interesting
 read or write.
 
-@findex CANNOT_STEP_HW_WATCHPOINTS
-@item CANNOT_STEP_HW_WATCHPOINTS
-If this is defined to a non-zero value, @value{GDBN} will remove all
-watchpoints before stepping the inferior.
-
 @findex STOPPED_BY_WATCHPOINT
 @item STOPPED_BY_WATCHPOINT (@var{wait_status})
 Return non-zero if stopped by a watchpoint.  @var{wait_status} is of

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFA- v3] Testcase for bug report 11531 and fix for Solaris
  2010-04-25 13:20       ` Joel Brobecker
@ 2010-04-26 11:24         ` Pierre Muller
  2010-04-26 16:49           ` Joel Brobecker
  0 siblings, 1 reply; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 11:24 UTC (permalink / raw)
  To: 'Joel Brobecker'; +Cc: 'Pedro Alves', gdb-patches

  Following Pedro's and Joel's comments
I simplified my test for bug report 11531
and used Joel's comment about the use of 
'delete_breakpoints' command.

I made the test to also succeed in case a software
watchpoint is used.

  Is this OK?

Pierre

2010-04-24  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR breakpoints/11531.
	* testsuite/gdb.base/gdb11531.c: New file.
	* testsuite/gdb.base/gdb11531.exp: New file.

Index: testsuite/gdb.base/gdb11531.c
===================================================================
RCS file: testsuite/gdb.base/gdb11531.c
diff -N testsuite/gdb.base/gdb11531.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gdb11531.c	26 Apr 2010 11:19:43 -0000
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   Contributed by Pierre Muller.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Test for Solaris specific watchpoint problem.  */
+
+struct rec
+{
+ int x;
+ double y;
+ int z;
+};
+
+static struct rec myrec;
+
+int
+main ()
+{
+  myrec.x = 5;
+  myrec.y = 3.4;
+  myrec.z = 56;
+  myrec.x = 78;
+  return myrec.x;
+}
+
Index: testsuite/gdb.base/gdb11531.exp
===================================================================
RCS file: testsuite/gdb.base/gdb11531.exp
diff -N testsuite/gdb.base/gdb11531.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gdb11531.exp	26 Apr 2010 11:19:43 -0000
@@ -0,0 +1,64 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB ibug report 11531.
+# This is a problem related to CANNOT_STEP_HW_WATCHPOINTS macro.
+# It affects Solaris native targets.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "gdb11531"
+
+if { [prepare_for_testing $testfile.exp $testfile $testfile.c {debug}] } {
+    return -1;
+}
+
+
+if { ![runto main] } then {
+    fail "run to main"
+    return
+}
+
+# The breakpoint is probably at the instruction where the value being
+# watched (myrec.x) gets updated.  This is the instruction where we
+# expect to receive a watchpoint notification when we do the "stepi"
+# below.  However, having the breakpoint at the same location as this
+# intruction can possibly interfere with our testcase, as stepping
+# over the breakpoint in order to get past it may incorrectly lead
+# to the debugger missing the watchpoint hit.  This would be a bug
+# in GDB, but this is not the bug that we are trying to test here.
+# So, we remove all breakpoints first.
+
+delete_breakpoints
+
+set nl "\[\r\n\]+"
+
+gdb_test "watch myrec.x" ".*atchpoint \[0-9\]+: myrec\.x" "Set watchpoint"
+
+gdb_test "next" \
+    ".*${nl}.*atchpoint \[0-9\]+: myrec\.x${nl}Old value = 0${nl}New value
= 5${nl}.*" \
+    "watchpoint variable triggers at next"
+
+gdb_test "continue" \
+    ".*${nl}.*atchpoint \[0-9\]+: myrec\.x${nl}Old value = 5${nl}New value
= 78${nl}.*" \
+    "watchpoint variable triggers at continue"
+

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  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:29           ` Mark Kettenis
  1 sibling, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2010-04-26 11:26 UTC (permalink / raw)
  To: Pierre Muller
  Cc: 'Eli Zaretskii', gdb-patches, 'Joel Brobecker'

On Monday 26 April 2010 08:51:18, Pierre Muller wrote:
>   Is this patch OK?

This is going in circles, but, why didn't you remove the macro
definition and the whole comment around it from the nm file?
The goal is remove the whole workaround, so you should remove
that too.  The goal of the exercise is to keep each patch
self-contained.  I know you'll remove the nm file afterwards, but,
let's pretend you wouldn't, or that someone would reject that
patch of yours for some reason we haven't thought of yet --- you'll
still want the macro and the comments around it to, _not_ end up
defined in there after this patch.

On Monday 26 April 2010 08:51:18, Pierre Muller wrote:
>   In that case, the configure.tgt script should probably report 
> that Solaris versions <= 2.7 are not supported anymore.
> This is what I added to configure.tgt below.

...

> -i[34567]86-*-solaris*)
> -       # Target: Solaris x86
> +i[34567]86-*-solaris2.[8-9] )
> +       # Target: Solaris x86, only versions 2.8 and 2.9
> +       # Versions <= 2.7 suffer a bug that was handled in older versions of
> 
> +       # GDB, but that code was removed.

It may still work, provided the user doesn't set watchpoints...
In any case, this won't prevent someone building gdb on such
solaris', because you'll still hit the "i[34567]86-*-*)" case
below (and --enable-targets=all will still build the files you're
skipping anyway), leaving the user with a built, but very crippled
GDB, with no hint where to look at.  I suggest leaving this change out
of the patch and focusing on a single thing at a time, otherwise
we'll never get this done.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  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:29           ` Mark Kettenis
  2010-04-26 11:52             ` Pierre Muller
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Kettenis @ 2010-04-26 11:29 UTC (permalink / raw)
  To: pierre.muller; +Cc: pedro, eliz, gdb-patches, brobecker

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Cc: <gdb-patches@sourceware.org>, "'Joel Brobecker'" <brobecker@adacore.com>
> Index: src/gdb/configure.tgt
> ===================================================================
> RCS file: /cvs/src/src/gdb/configure.tgt,v
> retrieving revision 1.231
> diff -u -p -r1.231 configure.tgt
> --- src/gdb/configure.tgt	20 Apr 2010 00:21:33 -0000	1.231
> +++ src/gdb/configure.tgt	26 Apr 2010 07:47:51 -0000
> @@ -197,8 +197,10 @@ i[34567]86-*-solaris2.1[0-9]* | x86_64-*
>  			i386-sol2-tdep.o sol2-tdep.o \
>  			corelow.o solib.o solib-svr4.o"
>  	;;
> -i[34567]86-*-solaris*)
> -	# Target: Solaris x86
> +i[34567]86-*-solaris2.[8-9] )
> +	# Target: Solaris x86, only versions 2.8 and 2.9
> +	# Versions <= 2.7 suffer a bug that was handled in older versions of

Can you add i[34567]86-*-solaris2.[0-7] to the list of obsolete
targets when doing this?  That way people get a clear message when
trying to configure GDB on older versions of Solaris.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  2010-04-26 11:26           ` Pedro Alves
@ 2010-04-26 11:50             ` Pierre Muller
  2010-04-26 11:56               ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 11:50 UTC (permalink / raw)
  To: 'Pedro Alves'
  Cc: 'Eli Zaretskii', gdb-patches, 'Joel Brobecker'



> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Monday, April 26, 2010 1:27 PM
> À : Pierre Muller
> Cc : 'Eli Zaretskii'; gdb-patches@sourceware.org; 'Joel Brobecker'
> Objet : Re: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code
> (was fix for bug 11531)
> 
> On Monday 26 April 2010 08:51:18, Pierre Muller wrote:
> >   Is this patch OK?
> 
> This is going in circles, but, why didn't you remove the macro
> definition and the whole comment around it from the nm file?


 Done here,
with configure.tgt part removed.
The macro only appears in ChangeLog after this patch.

What about that version?

Pierre

ChangeLog entry:
2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR breakpoints/11531.
	* config/i386/nm-i386sol2.h (CANNOT_STEP_HW_WATCHPOINTS): Remove
	macro definition and related comment.
	* infrun.c (CANNOT_STEP_HW_WATCHPOINTS): Remove macro.
	(resume): Remove code and comment related to this macro.

doc ChangeLog entry:

2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdbint.texinfo (CANNOT_STEP_HW_WATCHPOINTS): Remove explanation
	of macro deleted from GDB code.

Index: config/i386/nm-i386sol2.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386sol2.h,v
retrieving revision 1.19
diff -u -p -r1.19 nm-i386sol2.h
--- config/i386/nm-i386sol2.h	1 Jan 2010 07:31:48 -0000	1.19
+++ config/i386/nm-i386sol2.h	26 Apr 2010 11:44:55 -0000
@@ -19,14 +19,4 @@
 
 #ifdef NEW_PROC_API	/* Solaris 6 and above can do HW watchpoints */
 
-/* Solaris x86 2.6 and 2.7 targets have a kernel bug when stepping
-   over an instruction that causes a page fault without triggering
-   a hardware watchpoint. The kernel properly notices that it shouldn't
-   stop, because the hardware watchpoint is not triggered, but it forgets
-   the step request and continues the program normally.
-   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
-
 #endif /* NEW_PROC_API */
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.435
diff -u -p -r1.435 infrun.c
--- infrun.c	25 Mar 2010 20:48:53 -0000	1.435
+++ infrun.c	26 Apr 2010 11:44:52 -0000
@@ -179,16 +179,6 @@ show_debug_infrun (struct ui_file *file,
 #endif
 
 
-/* Convert the #defines into values.  This is temporary until wfi control
-   flow is completely sorted out.  */
-
-#ifndef CANNOT_STEP_HW_WATCHPOINTS
-#define CANNOT_STEP_HW_WATCHPOINTS 0
-#else
-#undef  CANNOT_STEP_HW_WATCHPOINTS
-#define CANNOT_STEP_HW_WATCHPOINTS 1
-#endif
-
 /* Tables of how to react to signals; the user sets them.  */
 
 static unsigned char *signal_stop;
@@ -1484,18 +1474,6 @@ resume (int step, enum target_signal sig
 			"trap_expected=%d\n",
  			step, sig, tp->trap_expected);
 
-  /* Some targets (e.g. Solaris x86) have a kernel bug when stepping
-     over an instruction that causes a page fault without triggering
-     a hardware watchpoint. The kernel properly notices that it shouldn't
-     stop, because the hardware watchpoint is not triggered, but it forgets
-     the step request and continues the program normally.
-     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.  */
-  if (CANNOT_STEP_HW_WATCHPOINTS && step)
-    remove_hw_watchpoints ();
-
-
   /* Normally, by the time we reach `resume', the breakpoints are either
      removed or inserted, as appropriate.  The exception is if we're
sitting
      at a permanent breakpoint; we need to step over it, but permanent
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.321
diff -u -p -r1.321 gdbint.texinfo
--- doc/gdbint.texinfo	10 Mar 2010 18:20:07 -0000	1.321
+++ doc/gdbint.texinfo	26 Apr 2010 11:44:55 -0000
@@ -781,11 +781,6 @@ inferior after a watchpoint has been hit
 when watchpoints trigger at the instruction following an interesting
 read or write.
 
-@findex CANNOT_STEP_HW_WATCHPOINTS
-@item CANNOT_STEP_HW_WATCHPOINTS
-If this is defined to a non-zero value, @value{GDBN} will remove all
-watchpoints before stepping the inferior.
-
 @findex STOPPED_BY_WATCHPOINT
 @item STOPPED_BY_WATCHPOINT (@var{wait_status})
 Return non-zero if stopped by a watchpoint.  @var{wait_status} is of

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  2010-04-26 11:29           ` Mark Kettenis
@ 2010-04-26 11:52             ` Pierre Muller
  0 siblings, 0 replies; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 11:52 UTC (permalink / raw)
  To: 'Mark Kettenis'; +Cc: pedro, eliz, gdb-patches, brobecker

> > Index: src/gdb/configure.tgt
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/configure.tgt,v
> > retrieving revision 1.231
> > diff -u -p -r1.231 configure.tgt
> > --- src/gdb/configure.tgt	20 Apr 2010 00:21:33 -0000	1.231
> > +++ src/gdb/configure.tgt	26 Apr 2010 07:47:51 -0000
> > @@ -197,8 +197,10 @@ i[34567]86-*-solaris2.1[0-9]* | x86_64-*
> >  			i386-sol2-tdep.o sol2-tdep.o \
> >  			corelow.o solib.o solib-svr4.o"
> >  	;;
> > -i[34567]86-*-solaris*)
> > -	# Target: Solaris x86
> > +i[34567]86-*-solaris2.[8-9] )
> > +	# Target: Solaris x86, only versions 2.8 and 2.9
> > +	# Versions <= 2.7 suffer a bug that was handled in older versions
> of
> 
> Can you add i[34567]86-*-solaris2.[0-7] to the list of obsolete
> targets when doing this?  That way people get a clear message when
> trying to configure GDB on older versions of Solaris.

  After Pedro's comments that these targets 
can still work (just that watchpoints might create problems)
I dropped for now this part of the patch.

  Is it possible and worthwhile to add some
warning about this issue in configure.tgt?

Pierre

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  2010-04-26 11:50             ` Pierre Muller
@ 2010-04-26 11:56               ` Pedro Alves
  2010-04-26 12:03                 ` Pierre Muller
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2010-04-26 11:56 UTC (permalink / raw)
  To: Pierre Muller
  Cc: 'Eli Zaretskii', gdb-patches, 'Joel Brobecker'

On Monday 26 April 2010 12:50:08, Pierre Muller wrote:
> > owner@sourceware.org] De la part de Pedro Alves
> > This is going in circles, but, why didn't you remove the macro
> > definition and the whole comment around it from the nm file?
> 
>  Done here,

Thanks.

> with configure.tgt part removed.
> The macro only appears in ChangeLog after this patch.
> 

> What about that version?
> 
> Pierre
> 
> ChangeLog entry:
> 2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	PR breakpoints/11531.
> 	* config/i386/nm-i386sol2.h (CANNOT_STEP_HW_WATCHPOINTS): Remove
> 	macro definition and related comment.
> 	* infrun.c (CANNOT_STEP_HW_WATCHPOINTS): Remove macro.
> 	(resume): Remove code and comment related to this macro.

This is okay, thanks.

> 
> doc ChangeLog entry:
> 
> 2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* gdbint.texinfo (CANNOT_STEP_HW_WATCHPOINTS): Remove explanation
> 	of macro deleted from GDB code.

This is obvious, IMO.

> 
> Index: config/i386/nm-i386sol2.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/i386/nm-i386sol2.h,v
> retrieving revision 1.19
> diff -u -p -r1.19 nm-i386sol2.h
> --- config/i386/nm-i386sol2.h	1 Jan 2010 07:31:48 -0000	1.19
> +++ config/i386/nm-i386sol2.h	26 Apr 2010 11:44:55 -0000
> @@ -19,14 +19,4 @@
>  
>  #ifdef NEW_PROC_API	/* Solaris 6 and above can do HW watchpoints */
>  
> -/* Solaris x86 2.6 and 2.7 targets have a kernel bug when stepping
> -   over an instruction that causes a page fault without triggering
> -   a hardware watchpoint. The kernel properly notices that it shouldn't
> -   stop, because the hardware watchpoint is not triggered, but it forgets
> -   the step request and continues the program normally.
> -   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
> -
>  #endif /* NEW_PROC_API */
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.435
> diff -u -p -r1.435 infrun.c
> --- infrun.c	25 Mar 2010 20:48:53 -0000	1.435
> +++ infrun.c	26 Apr 2010 11:44:52 -0000
> @@ -179,16 +179,6 @@ show_debug_infrun (struct ui_file *file,
>  #endif
>  
>  
> -/* Convert the #defines into values.  This is temporary until wfi control
> -   flow is completely sorted out.  */
> -
> -#ifndef CANNOT_STEP_HW_WATCHPOINTS
> -#define CANNOT_STEP_HW_WATCHPOINTS 0
> -#else
> -#undef  CANNOT_STEP_HW_WATCHPOINTS
> -#define CANNOT_STEP_HW_WATCHPOINTS 1
> -#endif
> -
>  /* Tables of how to react to signals; the user sets them.  */
>  
>  static unsigned char *signal_stop;
> @@ -1484,18 +1474,6 @@ resume (int step, enum target_signal sig
>  			"trap_expected=%d\n",
>   			step, sig, tp->trap_expected);
>  
> -  /* Some targets (e.g. Solaris x86) have a kernel bug when stepping
> -     over an instruction that causes a page fault without triggering
> -     a hardware watchpoint. The kernel properly notices that it shouldn't
> -     stop, because the hardware watchpoint is not triggered, but it forgets
> -     the step request and continues the program normally.
> -     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.  */
> -  if (CANNOT_STEP_HW_WATCHPOINTS && step)
> -    remove_hw_watchpoints ();
> -
> -
>    /* Normally, by the time we reach `resume', the breakpoints are either
>       removed or inserted, as appropriate.  The exception is if we're
> sitting
>       at a permanent breakpoint; we need to step over it, but permanent
> Index: doc/gdbint.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
> retrieving revision 1.321
> diff -u -p -r1.321 gdbint.texinfo
> --- doc/gdbint.texinfo	10 Mar 2010 18:20:07 -0000	1.321
> +++ doc/gdbint.texinfo	26 Apr 2010 11:44:55 -0000
> @@ -781,11 +781,6 @@ inferior after a watchpoint has been hit
>  when watchpoints trigger at the instruction following an interesting
>  read or write.
>  
> -@findex CANNOT_STEP_HW_WATCHPOINTS
> -@item CANNOT_STEP_HW_WATCHPOINTS
> -If this is defined to a non-zero value, @value{GDBN} will remove all
> -watchpoints before stepping the inferior.
> -
>  @findex STOPPED_BY_WATCHPOINT
>  @item STOPPED_BY_WATCHPOINT (@var{wait_status})
>  Return non-zero if stopped by a watchpoint.  @var{wait_status} is of
> 
> 


-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [RFA- v2] Remove CANNOT_STEP_HW_WATCHPOINTS related code (was fix for bug 11531)
  2010-04-26 11:56               ` Pedro Alves
@ 2010-04-26 12:03                 ` Pierre Muller
  0 siblings, 0 replies; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 12:03 UTC (permalink / raw)
  To: 'Pedro Alves'
  Cc: 'Eli Zaretskii', gdb-patches, 'Joel Brobecker'

> > ChangeLog entry:
> > 2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	PR breakpoints/11531.
> > 	* config/i386/nm-i386sol2.h (CANNOT_STEP_HW_WATCHPOINTS): Remove
> > 	macro definition and related comment.
> > 	* infrun.c (CANNOT_STEP_HW_WATCHPOINTS): Remove macro.
> > 	(resume): Remove code and comment related to this macro.
> 
> This is okay, thanks.

  Committed.
 
> >
> > doc ChangeLog entry:
> >
> > 2010-04-26  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	* gdbint.texinfo (CANNOT_STEP_HW_WATCHPOINTS): Remove explanation
> > 	of macro deleted from GDB code.
> 
> This is obvious, IMO.

  Also part of commit.

Pierre

  New patch for Solaris nm removal will follow.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFA- v3] Testcase for bug report 11531 and fix for Solaris
  2010-04-26 11:24         ` [RFA- v3] " Pierre Muller
@ 2010-04-26 16:49           ` Joel Brobecker
  2010-04-26 20:50             ` Pierre Muller
  0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2010-04-26 16:49 UTC (permalink / raw)
  To: Pierre Muller; +Cc: 'Pedro Alves', gdb-patches

> 2010-04-24  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	PR breakpoints/11531.
> 	* testsuite/gdb.base/gdb11531.c: New file.
> 	* testsuite/gdb.base/gdb11531.exp: New file.

Looks good to me :).

> +# Test GDB ibug report 11531.
             ^^^^^^

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [RFA- v3] Testcase for bug report 11531 and fix for Solaris
  2010-04-26 16:49           ` Joel Brobecker
@ 2010-04-26 20:50             ` Pierre Muller
  0 siblings, 0 replies; 17+ messages in thread
From: Pierre Muller @ 2010-04-26 20:50 UTC (permalink / raw)
  To: 'Joel Brobecker'; +Cc: 'Pedro Alves', gdb-patches

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Monday, April 26, 2010 6:50 PM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA- v3] Testcase for bug report 11531 and fix for Solaris
> 
> > 2010-04-24  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	PR breakpoints/11531.
> > 	* testsuite/gdb.base/gdb11531.c: New file.
> > 	* testsuite/gdb.base/gdb11531.exp: New file.
> 
> Looks good to me :).

  Thanks, committed,
 
> > +# Test GDB ibug report 11531.
>              ^^^^^^
after fixing this typo.

Pierre

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-04-26 20:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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       ` [RFA- v2] " Pedro Alves
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

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).