Re: x86 watchpoints bug (Re: ping: Re: PATCH : allow to set length of hw watchpoints (e.g. for Valgrind gdbserver))

Pedro Alves
2011-05-31T21:29:12Z

On Tuesday 31 May 2011 20:07:09, Pedro Alves wrote:
> First things first. This assertion is actually bogus ( and I'm to blame
> for it :-) ). Patch below. We get here with regnum == 3, which is quite
> valid. This means gdbserver is asserting whenever a watchpoint on DR3
> triggers. Vis:
>
> (gdb) watch s1
> (gdb) watch s2
> (gdb) watch s3
>
> (gdb) b 24
> (gdb) c
> ...
>
> on gdbserver side we have:
>
> stopped_data_addr:
> CONTROL (DR7): 51150155 STATUS (DR6): 00000000
> DR0: addr=0x603768, ref.count=1 DR1: addr=0x60376a, ref.count=1
> DR2: addr=0x609a08, ref.count=1 DR3: addr=0x60d8e8, ref.count=1
> ^^^^^^^^
>
> so:
>
> (gdb) p p = 0x60d8e8
> (gdb) c
>
> ... puff!
>
> ../../../src/gdb/gdbserver/linux-x86-low.c:511: A problem internal to GDBserver has been detected.
> Assertion `DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR' failed.
>
> Program exited with code 01.
> (gdb)
>

Now with new testcase. Applied to both mainline and 7.3.

Thanks,
--
Pedro Alves

2011-05-31 Pedro Alves <pedro@codesourcery.com>

gdb/gdbserver/
* linux-x86-low.c (i386_dr_low_get_addr): Fix off by one in
assertion.
* win32-i386-low.c (i386_dr_low_get_addr): Ditto.

gdb/testsuite/
* gdb.arch/i386-dr3-watch.c: New file.
* gdb.arch/i386-dr3-watch.exp: New file.

---
gdb/gdbserver/linux-x86-low.c | 2 -
gdb/gdbserver/win32-i386-low.c | 2 -
gdb/testsuite/gdb.arch/i386-dr3-watch.c | 43 ++++++++++++++++++++++++++
gdb/testsuite/gdb.arch/i386-dr3-watch.exp | 49 ++++++++++++++++++++++++++++++
4 files changed, 94 insertions(+), 2 deletions(-)

Index: src/gdb/gdbserver/linux-x86-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-x86-low.c 2011-05-31 22:04:53.000000000 +0100
+++ src/gdb/gdbserver/linux-x86-low.c 2011-05-31 22:05:24.224956802 +0100
@@ -508,7 +508,7 @@ i386_dr_low_get_addr (int regnum)
ptid_t ptid = ptid_of (lwp);

/* DR6 and DR7 are retrieved with some other way. */
- gdb_assert (DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR);
+ gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);

return x86_linux_dr_get (ptid, regnum);
}
Index: src/gdb/gdbserver/win32-i386-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-i386-low.c 2011-05-31 22:04:53.000000000 +0100
+++ src/gdb/gdbserver/win32-i386-low.c 2011-05-31 22:05:24.224956802 +0100
@@ -61,7 +61,7 @@ i386_dr_low_set_addr (const struct i386_
CORE_ADDR
i386_dr_low_get_addr (int regnum)
{
- gdb_assert (DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR);
+ gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);

return debug_reg_state.dr_mirror[regnum];
}
Index: src/gdb/testsuite/gdb.arch/i386-dr3-watch.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.arch/i386-dr3-watch.c 2011-05-31 21:41:33.764957296 +0100
@@ -0,0 +1,43 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+int i1;
+char gap1[32];
+
+int i2;
+char gap2[32];
+
+int i3;
+char gap3[32];
+
+int i4;
+
+void
+trigger (void)
+{
+ i1 = 1;
+ i2 = 2;
+ i3 = 3;
+ i4 = 4;
+}
+
+int
+main ()
+{
+ trigger ();
+ return 0;
+}
Index: src/gdb/testsuite/gdb.arch/i386-dr3-watch.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.arch/i386-dr3-watch.exp 2011-05-31 22:07:16.714956763 +0100
@@ -0,0 +1,49 @@
+# Copyright 2011 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/>.
+
+# x86 and amd64 gdbserver had a bug where a watchpoint triggered by
+# the DR3 debug register would trip on a bogus assertion.
+
+# This test relies on being able to set 4 hardware watchpoints. Since
+# that is not a valid assumption across most targets, and we're
+# testing a x86 specific bug, skip everywhere else.
+if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
+ return 0
+}
+
+set testfile "i386-dr3-watch"
+set srcfile ${testfile}.c
+
+if [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug $additional_flags}] {
+ untested "failed to compile ${testfile}"
+ return -1
+}
+
+if ![runto_main] then {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_test_no_output "set breakpoint always-inserted on"
+
+gdb_test "watch i1" "Hardware watchpoint .*: i1"
+gdb_test "watch i2" "Hardware watchpoint .*: i2"
+gdb_test "watch i3" "Hardware watchpoint .*: i3"
+gdb_test "watch i4" "Hardware watchpoint .*: i4"
+
+gdb_test "c" "Hardware watchpoint.*: i1.*" "continue to i1 watchpoint"
+gdb_test "c" "Hardware watchpoint.*: i2.*" "continue to i2 watchpoint"
+gdb_test "c" "Hardware watchpoint.*: i3.*" "continue to i3 watchpoint"
+gdb_test "c" "Hardware watchpoint.*: i4.*" "continue to i4 watchpoint"