Re: [PATCH] testsuite: Add (extensive) hardware breakpoint testing

["Maciej W. Rozycki" <macro@codesourcery.com>]

On Fri, 11 Nov 2011, Joel Brobecker wrote:

> > >  The consistency of failures between i686-linux-gnu and i686-mingw makes
> > > me fairly sure that's a bug in x86 support in GDB of some sort rather than
> > > a problem with my setup -- is that a known bug?
> > 
> > OOC, have you tried amd64-linux?

 I have now, and the result is the same -- breakpoints silently missed.

> > {i386,amd64}-linux are important enough targets that I think this
> > should be fixed for 7.4. Joel?
> 
> I think we should look at the context as well before making a decision:
>   - Is that a regression? If it's been like that in previous versions,
>     then maybe it's OK for it to fail for another version...
>   - Are hardware breakpoints used much? Probably by the people who
>     debug programs in ROM(/flash?).

 Plus self-modifying code -- although I agree that hardware execution 
breakpoints are less needed with hosted systems than on bare-iron targets.  

> We should probably try to investigate the problem quickly and determine
> the extent of the problem. If we think we should fix that for 7.4, and
> someone is willing to take responsibility for it, then let's make it
> a high priority item on our list, and not release 7.4 without the fix.

 Well, OOC I looked at the relevant source and the bug (and consequently 
the fix) is quite obvious -- the debug registers are only actually pushed 
down to hardware on data breakpoints and not execution breakpoints.

 I'm getting:

# of expected passes            69
# of unexpected failures        2

now on x86_64-linux-gnu.  The two failures are some matching problems with 
my hbreak2.exp -- I'll investigate and fix that up.

 OK to apply?

2011-11-11  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* i386-nat.c (i386_insert_hw_breakpoint): Call 
	i386_update_inferior_debug_regs.
	(i386_remove_hw_breakpoint): Likewise.

  Maciej

gdb-i386-hbreak.diff
Index: gdb-fsf-trunk-quilt/gdb/i386-nat.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/i386-nat.c	2011-11-07 13:42:30.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/i386-nat.c	2011-11-11 18:47:40.555635587 +0000
@@ -684,9 +684,15 @@ i386_insert_hw_breakpoint (struct gdbarc
 {
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
-  int retval = i386_insert_aligned_watchpoint (&dr_mirror,
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = dr_mirror;
+  int retval = i386_insert_aligned_watchpoint (&local_state,
 					       addr, len_rw) ? EBUSY : 0;
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (&local_state);
+
   if (maint_show_dr)
     i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
 
@@ -702,9 +708,15 @@ i386_remove_hw_breakpoint (struct gdbarc
 {
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
-  int retval = i386_remove_aligned_watchpoint (&dr_mirror,
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = dr_mirror;
+  int retval = i386_remove_aligned_watchpoint (&local_state,
 					       addr, len_rw);
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (&local_state);
+
   if (maint_show_dr)
     i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);