RE: [RFC] add h/w breakpoints to x86-linux gdbserver

["Pierre Muller" <muller at ics dot u-strasbg dot fr>]

  Doug, I had a small problem applying your patch,
and, of course, I wanted to extend it to also work 
on win32 gdbserver.

  For the test, I propose below another scheme:
run the test for all targets and accept also
the answer "No hardware breakpoint support for this target"
unless the target_info gdb,hardware_breakpoints is set,
in which case a failure is also issued on that answer.


Pierre Muller
Pascal language support maintainer for GDB




2009-07-01  Doug Evans  <dje@google.com>
	Pierre Muller  <muller@ics.u-strasbg.fr>

	Add h/w breakpoints to x86-linux and i386-win32 gdbserver.
	* i386-low.h (i386_low_insert_hw_breakpoint): Declare.
	(i386_low_remove_hw_breakpoint): Declare.
	* i386-low.c (i386_low_insert_hw_breakpoint): New function.
	(i386_low_remove_hw_breakpoint): New function.
	* linux-x86-low.c (x86_insert_point): Handle h/w breakpoint packets.
	(x86_remove_point): Ditto.
	* win32-386-low.c (i386_insert_point): Handle h/w breakpoint
packets.
	(i386_remove_point): Ditto.

	* gdb.base/hbreak.exp: New file.
	* gdb.base/hbreak.c: New file.


Index: testsuite/gdb.base/hbreak.c
===================================================================
RCS file: testsuite/gdb.base/hbreak.c
diff -N testsuite/gdb.base/hbreak.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/hbreak.c	1 Jul 2009 08:25:56 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.
*/
+
+void
+break_function (void)
+{
+}
+
+int
+main ()
+{
+  break_function ();
+  return 0;
+}
Index: testsuite/gdb.base/hbreak.exp
===================================================================
RCS file: testsuite/gdb.base/hbreak.exp
diff -N testsuite/gdb.base/hbreak.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/hbreak.exp	1 Jul 2009 08:25:56 -0000
@@ -0,0 +1,74 @@
+#   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/>.
+
+# Test hbreak.
+# Targets need to add "set_board_info gdb,hardware_breakpoints 1"
+# to their board files to get a failure if hardware breakpoint can not be
set.
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+set testfile "hbreak"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
{debug}] != "" } {
+    untested hbreak.exp
+    return
+}
+
+if [get_compiler_info ${binfile}] {
+    return
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+# We can't do hbreaks yet as the "exec" target doesn't support hbreak.
+# Once we get the x86 linux child target instantiated, for example,
+# then we can do hbreak.  So first run to main.
+# ??? Arguably broken, not sure how to fix.
+
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+
+send_gdb "hbreak break_function\n"
+gdb_expect {
+    -re "Hardware assisted breakpoint 2 at .*: file .*${srcfile}, line .*"
{
+	pass "hardware breakpoint on break_function"
+    }
+    -re "No hardware breakpoint support in the target." {
+	if { [target_info exists gdb,hardware_breakpoints] } {
+	    fail "hardware breakpoint on break_function"
+	} else {
+	    pass "hardware breakpoint is not supported"
+	    return
+	}
+    }
+    timeout { fail "hardware breakpoint on break_function" }
+}
+
+gdb_test "continue" \
+         ".*Breakpoint 2, break_function ().*" \
+         "run to break_function"
+
+gdb_continue_to_end "hbreak"
Index: gdbserver/i386-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/i386-low.c,v
retrieving revision 1.1
diff -u -p -r1.1 i386-low.c
--- gdbserver/i386-low.c	30 Jun 2009 21:31:32 -0000	1.1
+++ gdbserver/i386-low.c	1 Jul 2009 08:25:56 -0000
@@ -575,3 +575,37 @@ i386_low_stopped_by_watchpoint (struct i
   CORE_ADDR addr = 0;
   return i386_low_stopped_data_address (state, &addr);
 }
+ 
+/* Support for h/w breakpoints.  */
+
+/* Insert a hardware-assisted breakpoint at ADDR.
+   Return 0 on success, -1 on failure.  */
+
+int
+i386_low_insert_hw_breakpoint (struct i386_debug_reg_state *state,
+			       CORE_ADDR addr)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
+
+  if (debug_hw_points)
+    i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
+
+/* Remove a hardware-assisted breakpoint at ADDR.
+   Return 0 on success, -1 on failure.  */
+
+int
+i386_low_remove_hw_breakpoint (struct i386_debug_reg_state *state,
+			       CORE_ADDR addr)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
+
+  if (debug_hw_points)
+    i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
Index: gdbserver/i386-low.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/i386-low.h,v
retrieving revision 1.1
diff -u -p -r1.1 i386-low.h
--- gdbserver/i386-low.h	30 Jun 2009 21:31:32 -0000	1.1
+++ gdbserver/i386-low.h	1 Jul 2009 08:25:56 -0000
@@ -82,6 +82,16 @@ extern int i386_low_stopped_data_address
 /* Return true if the inferior has some watchpoint that triggered.
    Otherwise return false.  */
 extern int i386_low_stopped_by_watchpoint (struct i386_debug_reg_state
*state);
+
+/* Insert a hardware-assisted breakpoint at ADDR.
+   Return 0 on success, -1 on failure.  */
+extern int i386_low_insert_hw_breakpoint (struct i386_debug_reg_state
*state,
+					  CORE_ADDR addr);
+
+/* Remove a hardware-assisted breakpoint at ADDR.
+   Return 0 on success, -1 on failure.  */
+extern int i386_low_remove_hw_breakpoint (struct i386_debug_reg_state
*state,
+					  CORE_ADDR addr);
 

 /* Each target needs to provide several low-level functions
    that will be called to insert watchpoints and hardware breakpoints
Index: gdbserver/linux-x86-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-low.c,v
retrieving revision 1.3
diff -u -p -r1.3 linux-x86-low.c
--- gdbserver/linux-x86-low.c	30 Jun 2009 16:35:25 -0000	1.3
+++ gdbserver/linux-x86-low.c	1 Jul 2009 08:25:57 -0000
@@ -433,6 +433,9 @@ x86_insert_point (char type, CORE_ADDR a
   struct process_info *proc = current_process ();
   switch (type)
     {
+    case '1':
+      return i386_low_insert_hw_breakpoint
(&proc->private->arch_private->debug_reg_state,
+					    addr);
     case '2':
     case '3':
     case '4':
@@ -450,6 +453,9 @@ x86_remove_point (char type, CORE_ADDR a
   struct process_info *proc = current_process ();
   switch (type)
     {
+    case '1':
+      return i386_low_remove_hw_breakpoint
(&proc->private->arch_private->debug_reg_state,
+					    addr);
     case '2':
     case '3':
     case '4':
Index: gdbserver/win32-i386-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-i386-low.c,v
retrieving revision 1.15
diff -u -p -r1.15 win32-i386-low.c
--- gdbserver/win32-i386-low.c	30 Jun 2009 16:35:25 -0000	1.15
+++ gdbserver/win32-i386-low.c	1 Jul 2009 08:25:57 -0000
@@ -76,6 +76,9 @@ i386_insert_point (char type, CORE_ADDR 
 {
   switch (type)
     {
+    case '1':
+      return i386_low_insert_hw_breakpoint (&debug_reg_state, addr);
+
     case '2':
     case '3':
     case '4':
@@ -92,6 +95,9 @@ i386_remove_point (char type, CORE_ADDR 
 {
   switch (type)
     {
+    case '1':
+      return i386_low_remove_hw_breakpoint (&debug_reg_state, addr);
+
     case '2':
     case '3':
     case '4':