Re: Automatically use hardware watchpoints

[Vladimir Prus <vladimir@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

Vladimir Prus wrote:

> 
> Hello!
> 
> At the moment, gdb is not very good at debugging applications in flash:
> - you must remember to use 'hbreak' and not 'break'
> - a number of commands (like "next") want to use breakpoints
>         internally. Since software breakpoints do no work, those commands
>         do no work either.
> 
> This patch makes insert_bp_location automatically decide what breakpoint
> type should be used if the target provides a memory map.

And here's a version of this patch that actually includes doc changes.

- Volodya


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: AUTO_HARDWARE.diff --]
[-- Type: text/x-diff; name="AUTO_HARDWARE.diff", Size: 6695 bytes --]

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.847
diff -u -r1.847 Makefile.in
--- Makefile.in	17 Oct 2006 20:17:44 -0000	1.847
+++ Makefile.in	1 Nov 2006 11:12:53 -0000
@@ -1832,7 +1832,8 @@
 	$(gdb_string_h) $(demangle_h) $(annotate_h) $(symfile_h) \
 	$(objfiles_h) $(source_h) $(linespec_h) $(completer_h) $(gdb_h) \
 	$(ui_out_h) $(cli_script_h) $(gdb_assert_h) $(block_h) $(solib_h) \
-	$(solist_h) $(observer_h) $(exceptions_h) $(gdb_events_h) $(mi_common_h)
+	$(solist_h) $(observer_h) $(exceptions_h) $(gdb_events_h) $(mi_common_h) \
+	$(memattr_h)
 bsd-kvm.o: bsd-kvm.c $(defs_h) $(cli_cmds_h) $(command_h) $(frame_h) \
 	$(regcache_h) $(target_h) $(value_h) $(gdbcore_h) $(gdb_assert_h) \
 	$(readline_h) $(bsd_kvm_h)
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.231
diff -u -r1.231 breakpoint.c
--- breakpoint.c	19 Oct 2006 15:58:25 -0000	1.231
+++ breakpoint.c	1 Nov 2006 11:12:55 -0000
@@ -53,6 +53,7 @@
 #include "solist.h"
 #include "observer.h"
 #include "exceptions.h"
+#include "memattr.h"
 
 #include "gdb-events.h"
 #include "mi/mi-common.h"
@@ -231,6 +232,22 @@
 		    value);
 }
 
+/* If 1, gdb will automatically use hardware breakpoints for breakpoints
+   set with "break" but falling in read-only memory. 
+   If 0, gdb will warn about such breakpoints, but won't automatically
+   use hardware breakpoints.  */
+static int automatic_hardware_breakpoints;
+static void
+show_automatic_hardware_breakpoints (struct ui_file *file, int from_tty,
+				     struct cmd_list_element *c,
+				     const char *value)
+{
+  fprintf_filtered (file, _("\
+Automatic usage of hardware breakpoints is %s.\n"),
+		    value);
+}
+
+
 void _initialize_breakpoint (void);
 
 extern int addressprint;	/* Print machine addresses? */
@@ -794,6 +811,57 @@
   if (bpt->loc_type == bp_loc_software_breakpoint
       || bpt->loc_type == bp_loc_hardware_breakpoint)
     {
+      if (bpt->owner->type != bp_hardware_breakpoint)
+	{
+	  /* If the explicitly specified breakpoint type
+	     is not hardware breakpoint, check the memory map to see
+	     if the breakpoint address is in read only memory or not.
+	     Two important cases are:
+	     - location type is not hardware breakpoint, memory
+	     is readonly.  We change the type of the location to
+	     hardware breakpoint.
+	     - location type is hardware breakpoint, memory is read-write.
+	     This means we've previously made the location hardware one, but
+	     then the memory map changed, so we undo.
+	     
+	     When breakpoints are removed, remove_breakpoints will
+	     use location types we've just set here, the only possible
+	     problem is that memory map has changed during running program,
+	     but it's not going to work anyway with current gdb.  */
+	  struct mem_region *mr 
+	    = lookup_mem_region (bpt->target_info.placed_address);
+	  
+	  if (mr)
+	    {
+	      if (automatic_hardware_breakpoints)
+		{
+		  int changed = 0;
+		  enum bp_loc_type new_type;
+		  
+		  if (mr->attrib.mode != MEM_RW)
+		    new_type = bp_loc_hardware_breakpoint;
+		  else 
+		    new_type = bp_loc_software_breakpoint;
+		  
+		  if (new_type != bpt->loc_type)
+		    {
+		      static int said = 0;
+		      bpt->loc_type = new_type;
+		      if (!said)
+			{
+			  fprintf_filtered (gdb_stdout, 
+					    _("Note: automatically using hardware breakpoints for read-only addresses."));
+			  said = 1;
+			}
+		    }
+		}
+	      else if (bpt->loc_type == bp_loc_software_breakpoint
+		       && mr->attrib.mode != MEM_RW)	    
+		warning (_("cannot set software breakpoint at readonly address %s"),
+			 paddr (bpt->address));
+	    }
+	}
+        
       /* First check to see if we have to handle an overlay.  */
       if (overlay_debugging == ovly_off
 	  || bpt->section == NULL
@@ -1235,6 +1303,9 @@
     if (b->inserted)
       {
 	remove_breakpoint (b, mark_inserted);
+	/* Note: since we insert a breakpoint right after removing,
+	   any decisions about automatically using hardware breakpoints
+	   made in insert_bp_location are preserved. */
 	if (b->loc_type == bp_loc_hardware_breakpoint)
 	  val = target_insert_hw_breakpoint (&b->target_info);
 	else
@@ -8127,4 +8198,18 @@
 				&breakpoint_show_cmdlist);
 
   pending_break_support = AUTO_BOOLEAN_AUTO;
+
+  add_setshow_boolean_cmd ("auto-hw", no_class,
+			   &automatic_hardware_breakpoints, _("\
+Set automatic usage of hardware breakpoints."), _("\
+Show automatic usage of hardware breakpoints."), _("\
+If set, the debugger will automatically use hardware breakpoints for\n\
+breakpoints set with \"break\" but falling in read-only memory.  If not set,\n\
+a warning will be emitted for such breakpoints."),
+			   NULL,
+			   show_automatic_hardware_breakpoints,
+			   &breakpoint_set_cmdlist,
+			   &breakpoint_show_cmdlist);
+  
+  automatic_hardware_breakpoints = 1;
 }
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.366
diff -u -r1.366 gdb.texinfo
--- doc/gdb.texinfo	27 Oct 2006 22:23:20 -0000	1.366
+++ doc/gdb.texinfo	1 Nov 2006 11:13:05 -0000
@@ -3094,6 +3094,32 @@
 occurred since the time the breakpoint was disabled and one or more
 of these loads could resolve the location.
 
+@cindex automatic hardware breakpoints
+For some targets, @value{GDBN} can automatically decide if hardware or
+software breakpoints should be used, depending on whether the
+breakpoint address is read-only or read-write. This applies to
+breakpoints set with the ``break'' command as well as to internal
+breakpoints set by commands like ``next'' and ``finish''. For
+breakpoints set with ``hbreak'', @value{GDBN} will always use hardware 
+breakpoints.
+
+This automatic behaviour can be controlled with the following commands::
+
+@kindex set breakpoint auto-hw
+@kindex show breakpoint auto-hw
+@table @code
+@item set breakpoint auto-hw on
+This is the default behavior.  When @value{GDBN} sets a breakpoint, it
+will try to use the target memory map to decide if software or hardware
+breakpoint must be used.
+
+@item set breakpoint auto-hw off
+This indicates @value{GDBN} should not automatically select breakpoint
+type.  If target provides a memory map, @value{GDBN} will warn when
+trying to set software breakpoint at a read-only address.
+@end table
+
+
 @cindex negative breakpoint numbers
 @cindex internal @value{GDBN} breakpoints
 @value{GDBN} itself sometimes sets breakpoints in your program for