* [RFA] Fix a windows bug if two watchpoints are used
@ 2009-06-03 22:58 Pierre Muller
2009-06-03 23:21 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-03 22:58 UTC (permalink / raw)
To: gdb-patches
When using watchpoints with cygwin GDB,
I sometimes ended up getting a SIGTRAP
at the exact location of the watch.
I finally figured out a reproducible way of
generation this bug.
If I set two hardware watchpoints on
two integers ival1 and ival3,
dr[0] gets set to the address of ival1
and dr[1] to the address of ival3.
If I disable the first watchpoint,
but leave the second active,
dr[0] is changed to &ival3,
but dr[1] is not reset to zero,
as i386_low.reset_addr function is not set
in windows-nat.c.
The strange thing now is that on the next
hit of &ival3, dr[6] get the value 0xffff0ff3
as if both dr[0] and dr[1] have been hit...
This despite the fact that dr[7]
correctly says that only the first (i.e. dr[0]
debug register is active).
Now comes the tricky part, why does that generate a
SIGTRAP?
In i386_stopped_data_address,
the bits of dr_status_mirror (copied from current value of dr[6])
are checked from 0 to 3, but if hit is found for I,
the address value is taken from dr_mirror array.
Thus, according to dr[6], both debug registers 0 and 1
have been hit.
at i=0, the correct address of ival3 is found in dr_mirror[0],
but after for i=1, the value of dr_mirror[1] is used, but this
value is 0, as only one watch is active.
I don't really understand the reason why dr[6] gets the second bit
set, and to complicate things even more, this seems to be
processor or OS version dependent!
The bug shows up on a Dell Latitude D620 with Intel Core Duo T7200,
but not on a HP Pavilion with AMD Athlon ... both running Windows XP
(Dell Prof and HP Home, which might also make a difference).
I propose below a patch that adds
a testcase with the two watches, plus
a fix for windows-nat.c (implementing reset_addr)
plus an additional change to i386-nat.c
to further avoid similar problems on other native targets,
by resetting also the RW_LEN bits of the control register
when a given debug register is disabled.
Is this patch OK?
Pierre
ChangeLog entry:
2009-06-04 Pierre Muller <muller@ics.u-strasbg.fr>
* i386-nat.c (I386_DR_DISABLE): Also reset the RW_LEN part of
control register.
* windows-nat.c (cygwin_reset_dr): New forward declaration.
(windows_continue): Force debug registers overwrite if
debug_register_changed is set.
(windows_resume): Likewise.
(init_windows_ops): Register cygwin_reset_dr.
(cygwin_reset_dr): New function.
testsuite/ChangeLog entry:
2009-06-04 Pierre Muller <muller@ics.u-strasbg.fr>
* gdb.base/watchpoints.c: New file.
* gdb.base/watchpoints.exp: New test.
Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.22
diff -u -p -r1.22 i386-nat.c
--- i386-nat.c 14 May 2009 09:37:00 -0000 1.22
+++ i386-nat.c 3 Jun 2009 21:48:19 -0000
@@ -124,8 +124,9 @@ struct i386_dr_low_type i386_dr_low;
/* Disable the break/watchpoint in the I'th debug register. */
#define I386_DR_DISABLE(i) \
- dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)))
-
+ dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)) | \
+ 0xf << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i)))
+
/* Set in DR7 the RW and LEN fields for the I'th debug register. */
#define I386_DR_SET_RW_LEN(i,rwlen) \
do { \
Index: windows-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-nat.c,v
retrieving revision 1.192
diff -u -p -r1.192 windows-nat.c
--- windows-nat.c 24 May 2009 12:27:35 -0000 1.192
+++ windows-nat.c 3 Jun 2009 21:48:19 -0000
@@ -142,6 +142,7 @@ static int windows_thread_alive (struct
static void windows_kill_inferior (struct target_ops *);
static void cygwin_set_dr (int i, CORE_ADDR addr);
+static void cygwin_reset_dr (int i);
static void cygwin_set_dr7 (unsigned long val);
static unsigned long cygwin_get_dr6 (void);
@@ -1133,7 +1134,7 @@ windows_continue (DWORD continue_status,
for (th = &thread_head; (th = th->next) != NULL;)
if ((id == -1 || id == (int) th->id)
- && th->suspended)
+ && (th->suspended || debug_registers_changed))
{
if (debug_registers_changed)
{
@@ -1252,7 +1253,7 @@ windows_resume (struct target_ops *ops,
th->context.EFlags |= FLAG_TRACE_BIT;
}
- if (th->context.ContextFlags)
+ if (th->context.ContextFlags || debug_registers_changed)
{
if (debug_registers_changed)
{
@@ -1262,6 +1263,7 @@ windows_resume (struct target_ops *ops,
th->context.Dr3 = dr[3];
th->context.Dr6 = DR6_CLEAR_VALUE;
th->context.Dr7 = dr[7];
+ th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
}
CHECK (SetThreadContext (th->h, &th->context));
th->context.ContextFlags = 0;
@@ -2176,7 +2178,7 @@ init_windows_ops (void)
i386_dr_low.set_control = cygwin_set_dr7;
i386_dr_low.set_addr = cygwin_set_dr;
- i386_dr_low.reset_addr = NULL;
+ i386_dr_low.reset_addr = cygwin_reset_dr;
i386_dr_low.get_status = cygwin_get_dr6;
/* i386_dr_low.debug_register_length field is set by
@@ -2295,6 +2297,13 @@ cygwin_set_dr (int i, CORE_ADDR addr)
debug_registers_used = 1;
}
+/* Reset dr[i], to stay in sync with i386-nat dr_mirror. */
+static void
+cygwin_reset_dr (int i)
+{
+ cygwin_set_dr (i, 0);
+}
+
/* Pass the value VAL to the inferior in the DR7 debug control
register. Here we just store the address in D_REGS, the watchpoint
will be actually set up in windows_wait. */
Index: testsuite/gdb.base/watchpoints.c
===================================================================
RCS file: testsuite/gdb.base/watchpoints.c
diff -N testsuite/gdb.base/watchpoints.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/watchpoints.c 3 Jun 2009 21:48:19 -0000
@@ -0,0 +1,33 @@
+#include <stdio.h>
+#include <unistd.h>
+/*
+ * This source is mainly to test
+ * what happens when a watchpoint is removed
+ * what another watchpoint, inserted later is
+ * left active.
+ */
+
+int count = -1;
+int ival1 = -1;
+int ival2 = -1;
+int ival3 = -1;
+int ival4 = -1;
+
+int
+main ()
+{
+#ifdef usestubs
+ set_debug_traps();
+ breakpoint();
+#endif
+
+ for (count = 0; count < 4; count++) {
+ ival1 = count; ival2 = count;
+ ival3 = count; ival4 = count;
+ }
+
+ ival1 = count; ival2 = count; /* Outside loop */
+ ival3 = count; ival4 = count;
+
+ return 0;
+}
Index: testsuite/gdb.base/watchpoints.exp
===================================================================
RCS file: testsuite/gdb.base/watchpoints.exp
diff -N testsuite/gdb.base/watchpoints.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/watchpoints.exp 3 Jun 2009 21:48:19 -0000
@@ -0,0 +1,101 @@
+# Copyright 2009
+# 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/>.
+
+# This file was written by Pierre Muller. (muller@ics.u-strasbg.fr)
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "watchpoints"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set wp_set 1
+
+if [get_compiler_info ${binfile}] {
+ return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
{debug}] != "" } {
+ untested watchpoint.exp
+ return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+ runto_main
+ gdb_test "watch ival1" "" ""
+ gdb_test "watch ival3" "" ""
+
+ set timeout 600
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value =
-1.*New value = 0.*ival1 = count; ival2 = count;.*" "watchpoint hit, first
time"
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value =
-1.*New value = 0.*ival3 = count; ival4 = count;.*" "watchpoint hit, first
time"
+
+ # Check that the ival3 hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 1 time.*" "Watchpoint hit count
is 1"
+
+ # Continue until the next change for ival1, from 0 to 1.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value = 0.*New
value = 1.*ival1 = count; ival2 = count;.*" "watchpoint ival1 hit, second
time"
+
+ # Check that the hit count for ival1 is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival1\r\n\[ \t]+breakpoint already hit 2 times.*" "Watchpoint ival1 hit
count is 2"
+
+ # Continue until the next change for ival3, from 0 to 1.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 0.*New
value = 1.*ival3 = count; ival4 = count;.*" "watchpoint hit, second time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 2 times.*" "Watchpoint hit count
is 2"
+
+ # Continue until the next change, from 1 to 2.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value = 1.*New
value = 2.*ival1 = count; ival2 = count;.*" "watchpoint ival1 hit, third
time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival1\r\n\[ \t]+breakpoint already hit 3 times.*" "Watchpoint ival1 hit
count is 3"
+ # Disable ival1 watchpoint
+ gdb_test "disable 2" "" ""
+
+ # Continue until the next change, from 1 to 2.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 1.*New
value = 2.*ival3 = count; ival4 = count;.*" "watchpoint hit, third time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 3 times.*" "Watchpoint hit count
is 3"
+
+ # Continue until the next change, from 2 to 3.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 2.*New
value = 3.*ival3 = count; ival4 = count;.*" "watchpoint hit, fourth time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 4 times.*" "Watchpoint hit count
is 4"
+
+ # Continue until the next change, from 3 to 4.
+ # Note that this one is outside the loop.
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 3.*New
value = 4.*ival3 = count; ival4 = count;.*" "watchpoint hit, fifth time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 5 times.*" "Watchpoint hit count
is 5"
+
+
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA] Fix a windows bug if two watchpoints are used
2009-06-03 22:58 [RFA] Fix a windows bug if two watchpoints are used Pierre Muller
@ 2009-06-03 23:21 ` Eli Zaretskii
2009-06-03 23:33 ` Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-03 23:21 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Date: Thu, 4 Jun 2009 00:58:09 +0200
>
> Now comes the tricky part, why does that generate a
> SIGTRAP?
> In i386_stopped_data_address,
> the bits of dr_status_mirror (copied from current value of dr[6])
> are checked from 0 to 3, but if hit is found for I,
> the address value is taken from dr_mirror array.
>
> Thus, according to dr[6], both debug registers 0 and 1
> have been hit.
> at i=0, the correct address of ival3 is found in dr_mirror[0],
> but after for i=1, the value of dr_mirror[1] is used, but this
> value is 0, as only one watch is active.
Shouldn't we instead fix the logic of i386_stopped_data_address, to
get out of the loop on the first watchpoint that is found to be hit?
The function does not support more than one watchpoint anyway, so why
continue checking the bits in dr[6] after we've found one set already?
Would such a change fix your problem without the other complications?
Btw, I don't understand this part of i386_stopped_data_address:
if (maint_show_dr && addr == 0)
i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
Isn't that backwards? why display the address if it is zero?
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFA] Fix a windows bug if two watchpoints are used
2009-06-03 23:21 ` Eli Zaretskii
@ 2009-06-03 23:33 ` Pierre Muller
2009-06-04 3:16 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-03 23:33 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : Thursday, June 04, 2009 1:21 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix a windows bug if two watchpoints are used
>
> > From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> > Date: Thu, 4 Jun 2009 00:58:09 +0200
> >
> > Now comes the tricky part, why does that generate a
> > SIGTRAP?
> > In i386_stopped_data_address,
> > the bits of dr_status_mirror (copied from current value of dr[6])
> > are checked from 0 to 3, but if hit is found for I,
> > the address value is taken from dr_mirror array.
> >
> > Thus, according to dr[6], both debug registers 0 and 1
> > have been hit.
> > at i=0, the correct address of ival3 is found in dr_mirror[0],
> > but after for i=1, the value of dr_mirror[1] is used, but this
> > value is 0, as only one watch is active.
>
> Shouldn't we instead fix the logic of i386_stopped_data_address, to
> get out of the loop on the first watchpoint that is found to be hit?
> The function does not support more than one watchpoint anyway, so why
> continue checking the bits in dr[6] after we've found one set already?
>
> Would such a change fix your problem without the other complications?
It would hide the problem.
But what happens if you have different watchpoints
on the same address (say one 'watch' and one 'awatch')?
Are you sure your suggestion would not affect
such cases?
> Btw, I don't understand this part of i386_stopped_data_address:
>
> if (maint_show_dr && addr == 0)
> i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
>
> Isn't that backwards? why display the address if it is zero?
I think this is because if addr is non-zero, you already
have a call to i386_show_dr before with "watchpoint hit".
This call is simply to state that stopped_data_address
was called but didn't find a hit.
But the correct condition should be (rc == 0) instead
of (addr == 0) as setting a watchpoint at (CORE_ADDR) 0
should also work on target where this address is not protected.
Pierre
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA] Fix a windows bug if two watchpoints are used
2009-06-03 23:33 ` Pierre Muller
@ 2009-06-04 3:16 ` Eli Zaretskii
2009-06-04 3:29 ` Christopher Faylor
2009-06-04 6:34 ` [RFA-v2] " Pierre Muller
0 siblings, 2 replies; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-04 3:16 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 4 Jun 2009 01:33:27 +0200
> Content-Language: en-us
>
> > Shouldn't we instead fix the logic of i386_stopped_data_address, to
> > get out of the loop on the first watchpoint that is found to be hit?
> > The function does not support more than one watchpoint anyway, so why
> > continue checking the bits in dr[6] after we've found one set already?
> >
> > Would such a change fix your problem without the other complications?
>
> It would hide the problem.
Why hide, and what problem are we talking about? The situation you
describe has no rational explanation, and looks more like a Windows
bug than anything else: you in effect show a contradiction between two
debug registers that should tell a coherent story, but don't. Fixing
such problems without a good understanding of their exact reasons is
always a bit phenomenological. My phenomenology is based on the
premise that the OS uses the debug registers in the order we scan the
bits in dr[6], so the first one we find set has better chances to be
consistent with what really happened than anything else.
> But what happens if you have different watchpoints
> on the same address (say one 'watch' and one 'awatch')?
> Are you sure your suggestion would not affect
> such cases?
It will work even in those cases, yes. We only support multiple
watchpoints that break simultaneously if they watch the same address,
anyway (there's only one address that i386_stopped_data_address
returns). The i386 debug register support code will use a single
debug register for watching such an address, no matter how many
watchpoints the user sets and of what kind. We do this sharing of
debug registers entirely in GDB (see i386_insert_aligned_watchpoint
and the dr_ref_count[] array it uses); the OS is never told to use
more than one debug register for every address we watch, even if we
watch it with several watchpoints. The callers of
i386_stopped_data_address take the address it returns, and check all
the watchpoints that watch this address to see which one(s) of them
triggered and which did not. That code is in breakpoint.c, AFAIR.
> > Btw, I don't understand this part of i386_stopped_data_address:
> >
> > if (maint_show_dr && addr == 0)
> > i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
> >
> > Isn't that backwards? why display the address if it is zero?
> I think this is because if addr is non-zero, you already
> have a call to i386_show_dr before with "watchpoint hit".
> This call is simply to state that stopped_data_address
> was called but didn't find a hit.
> But the correct condition should be (rc == 0) instead
> of (addr == 0) as setting a watchpoint at (CORE_ADDR) 0
> should also work on target where this address is not protected.
Yes, I agree.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA] Fix a windows bug if two watchpoints are used
2009-06-04 3:16 ` Eli Zaretskii
@ 2009-06-04 3:29 ` Christopher Faylor
2009-06-04 6:34 ` [RFA-v2] " Pierre Muller
1 sibling, 0 replies; 21+ messages in thread
From: Christopher Faylor @ 2009-06-04 3:29 UTC (permalink / raw)
To: gdb-patches
On Wed, Jun 03, 2009 at 11:16:39PM -0400, Eli Zaretskii wrote:
>Pierre Muller wrote:
>>>Shouldn't we instead fix the logic of i386_stopped_data_address, to get
>>>out of the loop on the first watchpoint that is found to be hit? The
>>>function does not support more than one watchpoint anyway, so why
>>>continue checking the bits in dr[6] after we've found one set already?
>>>
>>>Would such a change fix your problem without the other complications?
>>
>>It would hide the problem.
>
>Why hide, and what problem are we talking about? The situation you
>describe has no rational explanation, and looks more like a Windows bug
>than anything else: you in effect show a contradiction between two
>debug registers that should tell a coherent story, but don't. Fixing
>such problems without a good understanding of their exact reasons is
>always a bit phenomenological. My phenomenology is based on the
>premise that the OS uses the debug registers in the order we scan the
>bits in dr[6], so the first one we find set has better chances to be
>consistent with what really happened than anything else.
>
>>But what happens if you have different watchpoints on the same address
>>(say one 'watch' and one 'awatch')? Are you sure your suggestion would
>>not affect such cases?
>
>It will work even in those cases, yes. We only support multiple
>watchpoints that break simultaneously if they watch the same address,
>anyway (there's only one address that i386_stopped_data_address
>returns). The i386 debug register support code will use a single debug
>register for watching such an address, no matter how many watchpoints
>the user sets and of what kind. We do this sharing of debug registers
>entirely in GDB (see i386_insert_aligned_watchpoint and the
>dr_ref_count[] array it uses); the OS is never told to use more than
>one debug register for every address we watch, even if we watch it with
>several watchpoints. The callers of i386_stopped_data_address take the
>address it returns, and check all the watchpoints that watch this
>address to see which one(s) of them triggered and which did not. That
>code is in breakpoint.c, AFAIR.
FWIW, I agree with Eli's assessments here.
cgf
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 3:16 ` Eli Zaretskii
2009-06-04 3:29 ` Christopher Faylor
@ 2009-06-04 6:34 ` Pierre Muller
2009-06-04 7:12 ` Eli Zaretskii
1 sibling, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-04 6:34 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches
Considering the objections to my first patch, I
here submit another patch that
make i386_stopped_data_address
return as soon as one hit is found.
The change in I386_DR_DISABLE is still
useful for the second test inside the i386_stopped_data_address
function.
This patch does fix my windows specific problem.
Is this OK?
Pierre
2009-06-04 Pierre Muller <muller@ics.u-strasbg.fr>
* i386-nat.c (I386_DR_DISABLE): Also reset the
RW_LEN part of the control register.
(i386_stopped_data_address): Return at the first hit.
Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.22
diff -u -p -r1.22 i386-nat.c
--- i386-nat.c 14 May 2009 09:37:00 -0000 1.22
+++ i386-nat.c 4 Jun 2009 06:25:20 -0000
@@ -124,8 +124,9 @@ struct i386_dr_low_type i386_dr_low;
/* Disable the break/watchpoint in the I'th debug register. */
#define I386_DR_DISABLE(i) \
- dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)))
-
+ dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)) | \
+ 0xf << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i)))
+
/* Set in DR7 the RW and LEN fields for the I'th debug register. */
#define I386_DR_SET_RW_LEN(i,rwlen) \
do { \
@@ -542,7 +543,6 @@ i386_stopped_data_address (struct target
{
CORE_ADDR addr = 0;
int i;
- int rc = 0;
dr_status_mirror = i386_dr_low.get_status ();
@@ -557,17 +557,16 @@ i386_stopped_data_address (struct target
&& I386_DR_GET_RW_LEN (i) != 0)
{
addr = dr_mirror[i];
- rc = 1;
+ *addr_p = addr;
if (maint_show_dr)
i386_show_dr ("watchpoint_hit", addr, -1, hw_write);
+ return 1;
}
}
- if (maint_show_dr && addr == 0)
+ if (maint_show_dr)
i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
- if (rc)
- *addr_p = addr;
- return rc;
+ return 0;
}
static int
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 6:34 ` [RFA-v2] " Pierre Muller
@ 2009-06-04 7:12 ` Eli Zaretskii
2009-06-04 7:33 ` Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-04 7:12 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 4 Jun 2009 08:33:52 +0200
> Content-Language: en-us
>
> Considering the objections to my first patch, I
> here submit another patch that
> make i386_stopped_data_address
> return as soon as one hit is found.
This part is fine with me.
> The change in I386_DR_DISABLE is still
> useful for the second test inside the i386_stopped_data_address
> function.
What do we need that for? Does the Intel spec say that these its need
to be zeroed for disabled debug registers?
> This patch does fix my windows specific problem.
Is your problem fixed without the change in I386_DR_DISABLE?
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 7:12 ` Eli Zaretskii
@ 2009-06-04 7:33 ` Pierre Muller
2009-06-04 10:31 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-04 7:33 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : Thursday, June 04, 2009 9:12 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA-v2] Fix a windows bug if two watchpoints are used
>
> > From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> > Cc: <gdb-patches@sourceware.org>
> > Date: Thu, 4 Jun 2009 08:33:52 +0200
> > Content-Language: en-us
> >
> > Considering the objections to my first patch, I
> > here submit another patch that
> > make i386_stopped_data_address
> > return as soon as one hit is found.
>
> This part is fine with me.
>
> > The change in I386_DR_DISABLE is still
> > useful for the second test inside the i386_stopped_data_address
> > function.
>
> What do we need that for? Does the Intel spec say that these its need
> to be zeroed for disabled debug registers?
>
> > This patch does fix my windows specific problem.
>
> Is your problem fixed without the change in I386_DR_DISABLE?
My problem is indeed fixed without that part,
but without my change to I386_DR_DISABLE, the second check
in i386_stopped_data_address:
ALL_DEBUG_REGISTERS(i)
{
if (I386_DR_WATCH_HIT (i)
/* This second condition makes sure DRi is set up for a data
watchpoint, not a hardware breakpoint. The reason is
that GDB doesn't call the target_stopped_data_address
method except for data watchpoints. In other words, I'm
being paranoiac. */
&& I386_DR_GET_RW_LEN (i) != 0)
is not reliable as the return value of I386_DR_GET_RW_LEN (i)
is non-zero if I was used before... Even if it was disabled
later!
Pierre
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 7:33 ` Pierre Muller
@ 2009-06-04 10:31 ` Eli Zaretskii
2009-06-04 14:52 ` Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-04 10:31 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 4 Jun 2009 09:33:06 +0200
> Content-Language: en-us
>
> My problem is indeed fixed without that part,
> but without my change to I386_DR_DISABLE, the second check
> in i386_stopped_data_address:
>
> ALL_DEBUG_REGISTERS(i)
> {
> if (I386_DR_WATCH_HIT (i)
> /* This second condition makes sure DRi is set up for a data
> watchpoint, not a hardware breakpoint. The reason is
> that GDB doesn't call the target_stopped_data_address
> method except for data watchpoints. In other words, I'm
> being paranoiac. */
> && I386_DR_GET_RW_LEN (i) != 0)
>
> is not reliable as the return value of I386_DR_GET_RW_LEN (i)
> is non-zero if I was used before... Even if it was disabled
> later!
This is C: if the result of the first test is false, the result of the
second test is not important, right?
Or am I missing something?
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 10:31 ` Eli Zaretskii
@ 2009-06-04 14:52 ` Pierre Muller
2009-06-04 15:22 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-04 14:52 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches
> > My problem is indeed fixed without that part,
> > but without my change to I386_DR_DISABLE, the second check
> > in i386_stopped_data_address:
> >
> > ALL_DEBUG_REGISTERS(i)
> > {
> > if (I386_DR_WATCH_HIT (i)
> > /* This second condition makes sure DRi is set up for a data
> > watchpoint, not a hardware breakpoint. The reason is
> > that GDB doesn't call the target_stopped_data_address
> > method except for data watchpoints. In other words, I'm
> > being paranoiac. */
> > && I386_DR_GET_RW_LEN (i) != 0)
> >
> > is not reliable as the return value of I386_DR_GET_RW_LEN (i)
> > is non-zero if I was used before... Even if it was disabled
> > later!
>
> This is C: if the result of the first test is false, the result of the
> second test is not important, right?
>
> Or am I missing something?
If all works fine, there is no problem, you are right,
but otherwise...
If, like for my cygwin problem,
dr_status_mirror has both bits 1 and 2 set
while only the first debug register is used according
to previous calls of the i386-nat functions.
This is probably an OS problem, but the whole thread is related to that...
In that situation, the second test, which is supposed to check if indeed
we have
that second register used as a hardware watchpoint, and not a
hardware breakpoint, according to the comment, fails.
Maybe we should use
ALL_DEBUG_REGISTERS(i)
{
if (I386_DR_WATCH_HIT (i)
&& !I386_DR_VACANT(i)
&& I386_DR_GET_RW_LEN (i) != 0)
In that case, I think that the code should be fine even if
the underlying OS reports a spurious bit of the debug status register as
set.
Pierre
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 14:52 ` Pierre Muller
@ 2009-06-04 15:22 ` Eli Zaretskii
2009-06-16 22:43 ` [PING][RFA-v2] " Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-04 15:22 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 4 Jun 2009 16:51:51 +0200
> Content-Language: en-us
>
> Maybe we should use
> ALL_DEBUG_REGISTERS(i)
> {
> if (I386_DR_WATCH_HIT (i)
> && !I386_DR_VACANT(i)
> && I386_DR_GET_RW_LEN (i) != 0)
I have no objections to adding the test for I386_DR_VACANT.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PING][RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-04 15:22 ` Eli Zaretskii
@ 2009-06-16 22:43 ` Pierre Muller
2009-06-22 20:56 ` Joel Brobecker
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-06-16 22:43 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : Thursday, June 04, 2009 5:23 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA-v2] Fix a windows bug if two watchpoints are used
>
> > From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> > Cc: <gdb-patches@sourceware.org>
> > Date: Thu, 4 Jun 2009 16:51:51 +0200
> > Content-Language: en-us
> >
> > Maybe we should use
> > ALL_DEBUG_REGISTERS(i)
> > {
> > if (I386_DR_WATCH_HIT (i)
> > && !I386_DR_VACANT(i)
> > && I386_DR_GET_RW_LEN (i) != 0)
>
> I have no objections to adding the test for I386_DR_VACANT.
Here also, I don't know if there still is an official
maintainer for i386-nat.c code, or is
Eli's approval enough for this?
Pierre
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PING][RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-16 22:43 ` [PING][RFA-v2] " Pierre Muller
@ 2009-06-22 20:56 ` Joel Brobecker
2009-06-23 1:04 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Joel Brobecker @ 2009-06-22 20:56 UTC (permalink / raw)
To: Pierre Muller; +Cc: 'Eli Zaretskii', gdb-patches
> Here also, I don't know if there still is an official
> maintainer for i386-nat.c code, or is
> Eli's approval enough for this?
Eli is a Global Maintainer, so his approval is as good as any one else.
I'm not sure that Eli's message was meant as an actual approval, though,
so you might want to double-check with him.
--
Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PING][RFA-v2] Fix a windows bug if two watchpoints are used
2009-06-22 20:56 ` Joel Brobecker
@ 2009-06-23 1:04 ` Eli Zaretskii
2009-09-22 12:55 ` [RFA-v3] " Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2009-06-23 1:04 UTC (permalink / raw)
To: Joel Brobecker; +Cc: muller, gdb-patches
> Date: Mon, 22 Jun 2009 13:56:16 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: 'Eli Zaretskii' <eliz@gnu.org>, gdb-patches@sourceware.org
>
> > Here also, I don't know if there still is an official
> > maintainer for i386-nat.c code, or is
> > Eli's approval enough for this?
>
> Eli is a Global Maintainer, so his approval is as good as any one else.
It's more than that in this case, actually: I originally wrote the x86
watchpoint support code, first just for DJGPP, then the more general
version for all x86 targets.
> I'm not sure that Eli's message was meant as an actual approval, though,
> so you might want to double-check with him.
I intentionally didn't say explicitly that I approve the code, because
I'd still want to hear Mark's approval. Mark is traveling, IIRC, but
if he intends to take a look, I'd suggest to wait for that. If Mark
says not to wait for him, you can see my mail as approval.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFA-v3] Fix a windows bug if two watchpoints are used
2009-06-23 1:04 ` Eli Zaretskii
@ 2009-09-22 12:55 ` Pierre Muller
2009-09-24 3:04 ` Eli Zaretskii
2009-09-24 17:54 ` Joel Brobecker
0 siblings, 2 replies; 21+ messages in thread
From: Pierre Muller @ 2009-09-22 12:55 UTC (permalink / raw)
To: 'Eli Zaretskii', 'Joel Brobecker',
'Mark Kettenis'
Cc: gdb-patches
Mark never answered that thread,
as a result, this one line patch that fixes
a problem with multiple watches for windows
is not in 7.0 branch :(
The thread started here:
http://sourceware.org/ml/gdb-patches/2009-06/msg00054.html
with
http://sourceware.org/ml/gdb-patches/2009-06/msg00416.html
containing the final one code line patch.
Mark, could you give your impression?
Thanks in advance,
Pierre Muller
Pascal language support maintainer for GDB
PS: I tested the patch on Compile Farm, on a amd64 linux,
no failure on new watchpoints.exp test,
nor on CVS head nor with patched version.
2009-09-22 Pierre Muller <muller@ics.u-strasbg.fr>
* i386-nat.c (i386_stopped_data_address): Also check that
hitted watch register is not vacant.
gdb/testsuite ChangeLog entry:
2009-09-22 Pierre Muller <muller@ics.u-strasbg.fr>
New test for two watchpoints, with disabling of
the first inserted.
* testsuite/gdb.base/watchpoints.c: New file.
* testsuite/gdb.base/watchpoints.exp: New file.
Index: src/gdb/i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.24
diff -u -p -r1.24 i386-nat.c
--- src/gdb/i386-nat.c 2 Jul 2009 17:21:06 -0000 1.24
+++ src/gdb/i386-nat.c 22 Sep 2009 08:25:36 -0000
@@ -555,7 +555,10 @@ i386_stopped_data_address (struct target
that GDB doesn't call the target_stopped_data_address
method except for data watchpoints. In other words, I'm
being paranoiac. */
- && I386_DR_GET_RW_LEN (i) != 0)
+ && I386_DR_GET_RW_LEN (i) != 0
+ /* This third condition makes sure DRi is not vacant, this
+ avoids false positives in windows-nat.c. */
+ && !I386_DR_VACANT (i))
{
addr = dr_mirror[i];
rc = 1;
Index: src/gdb/testsuite/gdb.base/watchpoints.c
===================================================================
RCS file: testsuite/gdb.base/watchpoints.c
diff -N testsuite/gdb.base/watchpoints.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.base/watchpoints.c 25 Jun 2009 22:44:17 -0000
@@ -0,0 +1,33 @@
+#include <stdio.h>
+#include <unistd.h>
+/*
+ * This source is mainly to test
+ * what happens when a watchpoint is removed
+ * while another watchpoint, inserted later is
+ * left active.
+ */
+
+int count = -1;
+int ival1 = -1;
+int ival2 = -1;
+int ival3 = -1;
+int ival4 = -1;
+
+int
+main ()
+{
+#ifdef usestubs
+ set_debug_traps();
+ breakpoint();
+#endif
+
+ for (count = 0; count < 4; count++) {
+ ival1 = count; ival2 = count;
+ ival3 = count; ival4 = count;
+ }
+
+ ival1 = count; ival2 = count; /* Outside loop */
+ ival3 = count; ival4 = count;
+
+ return 0;
+}
Index: src/gdb/testsuite/gdb.base/watchpoints.exp
===================================================================
RCS file: testsuite/gdb.base/watchpoints.exp
diff -N testsuite/gdb.base/watchpoints.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.base/watchpoints.exp 25 Jun 2009 22:44:17 -0000
@@ -0,0 +1,101 @@
+# Copyright 2009
+# 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/>.
+
+# This file was written by Pierre Muller. (muller@ics.u-strasbg.fr)
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "watchpoints"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set wp_set 1
+
+if [get_compiler_info ${binfile}] {
+ return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
{debug}] != "" } {
+ untested watchpoint.exp
+ return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+ runto_main
+ gdb_test "watch ival1" "" ""
+ gdb_test "watch ival3" "" ""
+
+ set timeout 600
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value =
-1.*New value = 0.*ival1 = count; ival2 = count;.*" "watchpoint hit, first
time"
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value =
-1.*New value = 0.*ival3 = count; ival4 = count;.*" "watchpoint hit, first
time"
+
+ # Check that the ival3 hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 1 time.*" "Watchpoint hit count
is 1"
+
+ # Continue until the next change for ival1, from 0 to 1.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value = 0.*New
value = 1.*ival1 = count; ival2 = count;.*" "watchpoint ival1 hit, second
time"
+
+ # Check that the hit count for ival1 is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival1\r\n\[ \t]+breakpoint already hit 2 times.*" "Watchpoint ival1 hit
count is 2"
+
+ # Continue until the next change for ival3, from 0 to 1.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 0.*New
value = 1.*ival3 = count; ival4 = count;.*" "watchpoint hit, second time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 2 times.*" "Watchpoint hit count
is 2"
+
+ # Continue until the next change, from 1 to 2.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival1.*Old value = 1.*New
value = 2.*ival1 = count; ival2 = count;.*" "watchpoint ival1 hit, third
time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival1\r\n\[ \t]+breakpoint already hit 3 times.*" "Watchpoint ival1 hit
count is 3"
+ # Disable ival1 watchpoint
+ gdb_test "disable 2" "" ""
+
+ # Continue until the next change, from 1 to 2.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 1.*New
value = 2.*ival3 = count; ival4 = count;.*" "watchpoint hit, third time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 3 times.*" "Watchpoint hit count
is 3"
+
+ # Continue until the next change, from 2 to 3.
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 2.*New
value = 3.*ival3 = count; ival4 = count;.*" "watchpoint hit, fourth time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 4 times.*" "Watchpoint hit count
is 4"
+
+ # Continue until the next change, from 3 to 4.
+ # Note that this one is outside the loop.
+
+ gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*ival3.*Old value = 3.*New
value = 4.*ival3 = count; ival4 = count;.*" "watchpoint hit, fifth time"
+
+ # Check that the hit count is reported correctly
+ gdb_test "info break" ".*watchpoint\[ \t\]+keep\[ \t\]+y\[
\t\]+ival3\r\n\[ \t]+breakpoint already hit 5 times.*" "Watchpoint hit count
is 5"
+
+
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-22 12:55 ` [RFA-v3] " Pierre Muller
@ 2009-09-24 3:04 ` Eli Zaretskii
2009-09-24 17:54 ` Joel Brobecker
1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2009-09-24 3:04 UTC (permalink / raw)
To: Pierre Muller; +Cc: brobecker, mark.kettenis, gdb-patches
> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Tue, 22 Sep 2009 14:54:26 +0200
>
> Mark never answered that thread,
> as a result, this one line patch that fixes
> a problem with multiple watches for windows
> is not in 7.0 branch :(
>
> The thread started here:
> http://sourceware.org/ml/gdb-patches/2009-06/msg00054.html
>
> with
> http://sourceware.org/ml/gdb-patches/2009-06/msg00416.html
> containing the final one code line patch.
>
> Mark, could you give your impression?
If Mark does not respond by tomorrow, please commit this to both
branch and trunk. Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-22 12:55 ` [RFA-v3] " Pierre Muller
2009-09-24 3:04 ` Eli Zaretskii
@ 2009-09-24 17:54 ` Joel Brobecker
2009-09-24 22:08 ` Pierre Muller
1 sibling, 1 reply; 21+ messages in thread
From: Joel Brobecker @ 2009-09-24 17:54 UTC (permalink / raw)
To: Pierre Muller
Cc: 'Eli Zaretskii', 'Mark Kettenis', gdb-patches
> PS: I tested the patch on Compile Farm, on a amd64 linux,
> no failure on new watchpoints.exp test,
> nor on CVS head nor with patched version.
I just wanted to make sure that this is relevant to amd64 as well
as i386... Otherwise, it'd be nice if you could test it on x86 as well
before committing to the branch (being paranoid, sorry).
Thanks!
--
Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-24 17:54 ` Joel Brobecker
@ 2009-09-24 22:08 ` Pierre Muller
2009-09-25 1:53 ` Joel Brobecker
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-09-24 22:08 UTC (permalink / raw)
To: 'Joel Brobecker'
Cc: 'Eli Zaretskii', 'Mark Kettenis', gdb-patches
I only wanted to have the fix in the branch,
I don't insist on having the new testsuite files
in 7.0
I was thinking of resubmitting the
watchpoints.exp and watchpoints.c separately again
for main branch only.
Should I commit all together?
I might be able to test it tomorrow on
a Ubuntu inside VMware.
As you prefer!
Pierre Muller
Pascal language support maintainer for GDB
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Thursday, September 24, 2009 7:54 PM
> À : Pierre Muller
> Cc : 'Eli Zaretskii'; 'Mark Kettenis'; gdb-patches@sourceware.org
> Objet : Re: [RFA-v3] Fix a windows bug if two watchpoints are used
>
> > PS: I tested the patch on Compile Farm, on a amd64 linux,
> > no failure on new watchpoints.exp test,
> > nor on CVS head nor with patched version.
>
> I just wanted to make sure that this is relevant to amd64 as well
> as i386... Otherwise, it'd be nice if you could test it on x86 as well
> before committing to the branch (being paranoid, sorry).
>
> Thanks!
> --
> Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-24 22:08 ` Pierre Muller
@ 2009-09-25 1:53 ` Joel Brobecker
2009-09-25 15:32 ` Pierre Muller
0 siblings, 1 reply; 21+ messages in thread
From: Joel Brobecker @ 2009-09-25 1:53 UTC (permalink / raw)
To: Pierre Muller
Cc: 'Eli Zaretskii', 'Mark Kettenis', gdb-patches
> I only wanted to have the fix in the branch,
> I don't insist on having the new testsuite files
> in 7.0
I wouldn't worry about the testcase files, they do not impact
the debugger functionality, so they are not a big risk. The risk
is with the patch itself, so I was asking whether there was a way
you could test this on x86 before checking in the branch.
Normally, testing on one architecture is sufficient, but
I'm getting a little (too?) paranoid...
In other words, if you commit the patch, go ahead and commit
the testcase as well. It doesn't matter whether it's on the branch
or not.
--
Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-25 1:53 ` Joel Brobecker
@ 2009-09-25 15:32 ` Pierre Muller
2009-09-25 16:07 ` Joel Brobecker
0 siblings, 1 reply; 21+ messages in thread
From: Pierre Muller @ 2009-09-25 15:32 UTC (permalink / raw)
To: 'Joel Brobecker'
Cc: 'Eli Zaretskii', 'Mark Kettenis', gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Friday, September 25, 2009 3:54 AM
> À : Pierre Muller
> Cc : 'Eli Zaretskii'; 'Mark Kettenis'; gdb-patches@sourceware.org
> Objet : Re: [RFA-v3] Fix a windows bug if two watchpoints are used
>
> > I only wanted to have the fix in the branch,
> > I don't insist on having the new testsuite files
> > in 7.0
>
> I wouldn't worry about the testcase files, they do not impact
> the debugger functionality, so they are not a big risk. The risk
> is with the patch itself, so I was asking whether there was a way
> you could test this on x86 before checking in the branch.
> Normally, testing on one architecture is sufficient, but
> I'm getting a little (too?) paranoid...
I finally was able to test it on a
ubuntu 9.04 server (VMWare inside a Windows XP machine)
gdb-6.8.91-20090925
(no gnatmake, no xml support...)
Pierre@d620-muller ~/gdb-7pre/log/i386-ubuntu
$ grep "^FAIL" gdb-normal.sum | gawk -F: '{print $1 " " $2}' | uniq -c
1 FAIL gdb.base/auxv.exp
2 FAIL gdb.base/break.exp
26 FAIL gdb.base/catch-syscall.exp
1 FAIL gdb.base/default.exp
3 FAIL gdb.base/longjmp.exp
2 FAIL gdb.base/sepdebug.exp
2 FAIL gdb.dwarf2/dw2-compressed.exp
1 FAIL gdb.mi/mi-nsmoribund.exp
1 FAIL gdb.server/ext-run.exp
Pierre@d620-muller ~/gdb-7pre/log/i386-ubuntu
$ tail gdb-normal.sum
# of expected passes 12077
# of unexpected failures 39
# of expected failures 43
# of known failures 33
# of unresolved testcases 2
# of untested testcases 72
# of unsupported tests 69
/usr/local/src/gdb-7pre/build/gdb/testsuite/../../gdb/gdb version
6.8.91.200909
25 -nw -nx
After adding patch to i386-nat.c
$ diff gdb-normal.sum gdb.sum
1c1
< Test Run By vadmin on Fri Sep 25 08:20:08 2009
---
> Test Run By vadmin on Fri Sep 25 09:44:56 2009
12750c12750
< PASS: gdb.threads/threxit-hop-specific.exp: get past the thread specific
break
point
---
> FAIL: gdb.threads/threxit-hop-specific.exp: get past the thread specific
break
point
13000,13001c13000,13001
< # of expected passes 12077
< # of unexpected failures 39
---
> # of expected passes 12076
> # of unexpected failures 40
The additional failure test does not use watchpoint,
thus I suspect it to be some instable test.
>>>>>>> Start of test threxit-hop (patched version that fails)
Running
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads/threxit-hop-speci
fic.exp ...
Executing on host: gcc
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads/th
rexit-hop-specific.c -I/usr/local/src/gdb-7pre/build/gdb/testsuite -g
-lpthrea
ds -lm -o
/usr/local/src/gdb-7pre/build/gdb/testsuite/gdb.threads/threxit-hop-
specific (timeout = 300)
/usr/bin/ld: cannot find -lpthreads
collect2: ld returned 1 exit status
compiler exited with status 1
output is:
/usr/bin/ld: cannot find -lpthreads
collect2: ld returned 1 exit status
Executing on host: gcc
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads/th
rexit-hop-specific.c -I/usr/local/src/gdb-7pre/build/gdb/testsuite -g
-lpthrea
d -lm -o
/usr/local/src/gdb-7pre/build/gdb/testsuite/gdb.threads/threxit-hop-s
pecific (timeout = 300)
PASS: gdb.threads/threxit-hop-specific.exp: successfully compiled posix
threads
test case
warning: Can not parse XML syscalls information; XML support was disabled at
com
pile time.
GNU gdb (GDB) 6.8.91.20090925
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
(gdb) set height 0
(gdb) set width 0
(gdb) dir
Reinitialize source path to empty? (y or n) y
Source directories searched: $cdir:$cwd
(gdb) dir ../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads
Source directories searched:
/usr/local/src/gdb-7pre/build/gdb/testsuite/../../.
./gdb-6.8.91.20090925/gdb/testsuite/gdb.threads:$cdir:$cwd
(gdb) kill
The program is not being run.
(gdb) file
/usr/local/src/gdb-7pre/build/gdb/testsuite/gdb.threads/threxit-hop-s
pecific
Reading symbols from
/usr/local/src/gdb-7pre/build/gdb/testsuite/gdb.threads/thr
exit-hop-specific...done.
(gdb) delete breakpoints
(gdb) info breakpoints
No breakpoints or watchpoints.
(gdb) break main
Breakpoint 1 at 0x804850c: file
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.t
hreads/threxit-hop-specific.c, line 39.
(gdb) run
Starting program:
/usr/local/src/gdb-7pre/build/gdb/testsuite/gdb.threads/threxi
t-hop-specific
[Thread debugging using libthread_db enabled]
Breakpoint 1, main (argc=<value optimized out>, argv=<value optimized out>)
at .
./../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads/threxit-hop-specific.c
:39
39 pthread_create (&thread, NULL, thread_function, NULL);
(gdb) break thread_function
Breakpoint 2 at 0x80484ea: file
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.t
hreads/threxit-hop-specific.c, line 26.
(gdb) continue
Continuing.
[New Thread 0x403c9b90 (LWP 23629)]
[Switching to Thread 0x403c9b90 (LWP 23629)]
Breakpoint 2, thread_function (arg=0x0) at
../../../gdb-6.8.91.20090925/gdb/test
suite/gdb.threads/threxit-hop-specific.c:26
26 pthread_exit (NULL);
(gdb) PASS: gdb.threads/threxit-hop-specific.exp: continue to thread start
break 44 thread 2
Breakpoint 3 at 0x8048542: file
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.t
hreads/threxit-hop-specific.c, line 44.
(gdb) PASS: gdb.threads/threxit-hop-specific.exp: set thread specific
breakpoint
break 47
Breakpoint 4 at 0x8048547: file
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.t
hreads/threxit-hop-specific.c, line 47.
(gdb) next
0x400904b1 in siglongjmp () from /lib/tls/i686/cmov/libc.so.6
(gdb) FAIL: gdb.threads/threxit-hop-specific.exp: get past the thread
specific b
reakpoint
testcase
../../../gdb-6.8.91.20090925/gdb/testsuite/gdb.threads/threxit-hop-spec
ific.exp completed in 2 seconds
>>>>>>> End of test threxit-hop
> In other words, if you commit the patch, go ahead and commit
> the testcase as well. It doesn't matter whether it's on the branch
> or not.
Joel, is it OK now?
May I commit the patch with the testsuite in both main and 7.0 branch?
Pierre
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFA-v3] Fix a windows bug if two watchpoints are used
2009-09-25 15:32 ` Pierre Muller
@ 2009-09-25 16:07 ` Joel Brobecker
0 siblings, 0 replies; 21+ messages in thread
From: Joel Brobecker @ 2009-09-25 16:07 UTC (permalink / raw)
To: Pierre Muller
Cc: 'Eli Zaretskii', 'Mark Kettenis', gdb-patches
> Joel, is it OK now?
> May I commit the patch with the testsuite in both main and 7.0 branch?
Yes, this is OK. Thanks for doing the testing for me.
--
Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2009-09-25 16:07 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-03 22:58 [RFA] Fix a windows bug if two watchpoints are used Pierre Muller
2009-06-03 23:21 ` Eli Zaretskii
2009-06-03 23:33 ` Pierre Muller
2009-06-04 3:16 ` Eli Zaretskii
2009-06-04 3:29 ` Christopher Faylor
2009-06-04 6:34 ` [RFA-v2] " Pierre Muller
2009-06-04 7:12 ` Eli Zaretskii
2009-06-04 7:33 ` Pierre Muller
2009-06-04 10:31 ` Eli Zaretskii
2009-06-04 14:52 ` Pierre Muller
2009-06-04 15:22 ` Eli Zaretskii
2009-06-16 22:43 ` [PING][RFA-v2] " Pierre Muller
2009-06-22 20:56 ` Joel Brobecker
2009-06-23 1:04 ` Eli Zaretskii
2009-09-22 12:55 ` [RFA-v3] " Pierre Muller
2009-09-24 3:04 ` Eli Zaretskii
2009-09-24 17:54 ` Joel Brobecker
2009-09-24 22:08 ` Pierre Muller
2009-09-25 1:53 ` Joel Brobecker
2009-09-25 15:32 ` Pierre Muller
2009-09-25 16:07 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox