Re: [RFC] Catch exception after stepped over watchpoint.

[Yao Qi <yao at codesourcery dot com>]

On 06/28/2013 03:36 PM, Yao Qi wrote:
> Hi,
> When I test mips h/w watchpoint gdbserver patches V2, I get an internal
> error in gdb.  Here are the steps to reproduce it with my patches,
> 
> (gdb) target remote mips:1234
> (gdb) watch *global_ptr
> Hardware watchpoint 1: *global_ptr
> (gdb) watch **global_ptr
> Hardware watchpoint 2: **global_ptr
> (gdb) c
> Continuing.
> Warning:
> Could not insert hardware watchpoint 1.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
> 
> (gdb) kill
> Kill the program being debugged? (y or n) y
> (gdb) target remote mips:1234
> Remote debugging using mips:1234
> gdb/infrun.c:3889:
> internal-error: handle_inferior_event: Assertion `ptid_equal
> (inferior_ptid, singlestep_ptid)' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> 
> I got this internal error seldom when the V1 patches were tested.  I
> bypass it as it is unrelated.  Nowadays, I got this internal error
> every time, even with V1 patches, so I have to fix it at first.
> 
> Note that my mips board only has one watch register, and we request
> two here.  After some analysis, this internal error is caused by
> exception is thrown when stepping over watchpoint (by software single
> step).
> 
> When the inferior hit a watchpoint, GDB gets a stop and steps over the
> watchpoint.  GDB will remove all the breakpoints, perform single step,
> wait, and insert breakpoints again.
> 
> Let us look at the infrun debug on how GDB behaves with one
> watchpoint,
> 
> ....
> nfrun: stopped by watchpoint
> infrun: stopped data address = 0x120011bd0
> 
>   // stepping over a watchpoint
> 
> infrun: prepare_to_wait
> infrun: target_wait (7512 [Thread 7512], status) =
> infrun:   7512 [Thread 7512],
> infrun:   status->kind = stopped, signal = SIGTRAP
> infrun: infwait_nonstep_watch_state
> 
>   // watchpoints are inserted here, and single step
> 
> infrun: TARGET_WAITKIND_STOPPED
> infrun: stop_pc = 0x1200012f8
> infrun: software single step trap for Thread 7512
> infrun: BPSTAT_WHAT_STOP_NOISY
> infrun: stop_stepping
> 
>   // single step is done
> 
> Hardware watchpoint 1: *global_ptr
> 
> and here is how GDB behaves with two watchpoints,
> 
> infrun: stopped by watchpoint
> infrun: stopped data address = 0x120011bd0
> 
>   // stepping over a watchpoint
> 
> infrun: prepare_to_wait
> infrun: target_wait (7516 [Thread 7516], status) =
> infrun:   7516 [Thread 7516],
> infrun:   status->kind = stopped, signal = SIGTRAP
> infrun: infwait_nonstep_watch_state
> 
>   // watchpoints are being inserted, but an exception is thrown
> 
> Warning:
> Could not insert hardware watchpoint 1.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
> 
> As we can see, exception is thrown when insert breakpoints (because
> GDB requests two watchpoint registers while the board only has one),
> but the state on software single step is not cleared
> (singlestep_breakpoints_inserted_p is still true, which is wrong).
> When GDB connects to the remote again, it goes to the path triggers
> the assert.  This problem may exist on other targets which 1) use
> software single step 2) support hardware watchpoint.  Probably, we can
> reproduce it on some arm boards.  I didn't do that because I don't
> have such board on hand.
> 
> This patch catches the exception around insert_breakpoints and clear the
> state on software single step.
> 
> Note that I tried to call stop_stepping (ecs) and return after print
> exception, but the watchpoint hit is not processed in this way.  The
> output of GDB is:
> 
> (gdb) c
> Continuing.
> Warning:
> Could not insert hardware watchpoint 1.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
> 
> func4 () at /scratch/yqi/mips-linux/src/gdb-2013.05/gdb/testsuite/gdb.base/watchpoint.c:135
> 135       buf[0] = 7;
> 
> users are confused that why program stops here.  In my current
> approach, the output looks more clear,
> 
> (gdb) c
> Continuing.
> Warning:
> Could not insert hardware watchpoint 1.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoints.
> 
> Hardware watchpoint 1: *global_ptr
> 
> Old value = <unreadable>
> New value = 3 '\003'
> func4 () at /scratch/yqi/mips-linux/src/gdb-2013.05/gdb/testsuite/gdb.base/watchpoint.c:135
> 135       buf[0] = 7;
> 
> Regression tested on x86_64 and mips (with my h/w watchpoint patches).
> Comments?

Ping.  I also attach the patch with spaces ignored.

-- 
Yao (éå)

---
 gdb/infrun.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index dc1036d..81b132d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3301,17 +3301,35 @@ handle_inferior_event (struct execution_control_state *ecs)
       break;
 
     case infwait_nonstep_watch_state:
+      {
+	volatile struct gdb_exception e;
+
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog,
 			    "infrun: infwait_nonstep_watch_state\n");
+
+	TRY_CATCH (e, RETURN_MASK_ERROR)
+	  {
       insert_breakpoints ();
+	  }
+	if (e.reason < 0)
+	  {
+	    if (singlestep_breakpoints_inserted_p)
+	      {
+		/* Pull the single step breakpoints out of the target.  */
+		remove_single_step_breakpoints ();
+		singlestep_breakpoints_inserted_p = 0;
+	      }
+
+	    exception_print (gdb_stderr, e);
+	  }
 
       /* FIXME-maybe: is this cleaner than setting a flag?  Does it
          handle things like signals arriving and other things happening
          in combination correctly?  */
       stepped_after_stopped_by_watchpoint = 1;
+      }
       break;
-
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
     }
-- 
1.7.7.6