single-step breakpoints

single-step breakpoints

[Mark Kettenis]

Hi Daniel,

There a slight problem with insert_single_step_breakpoint() and
remove_single_step_breakpoints().  On OpenBSD, inserting a breakpoint
into the kernel-provided signal trampoline, may fail.  Unfortunately,
the caller of insert_single_step_breakpoint() never notices this, and
calls remove_single_step_breakpoints() to remove the breakpoints.
Unfortunately that makes us hit the gdb_assert() in there.

This patch makes us avoid this while still making an attempt to catch
misuse of the interface.


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* breakpoint.c (remove_single_step_breakpoints): Bail out early if
	no breakpoints are inserted.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.225
diff -u -p -r1.225 breakpoint.c
--- breakpoint.c 18 Apr 2006 19:20:06 -0000 1.225
+++ breakpoint.c 25 Apr 2006 20:03:21 -0000
@@ -7726,6 +7726,9 @@ insert_single_step_breakpoint (CORE_ADDR
 void
 remove_single_step_breakpoints (void)
 {
+  if (single_step_breakpoints[0] == NULL && single_step_breakpoints[1] == NULL)
+    return;
+
   gdb_assert (single_step_breakpoints[0] != NULL);
 
   /* See insert_single_step_breakpoint for more about this deprecated

Re: single-step breakpoints

[Daniel Jacobowitz]

On Tue, Apr 25, 2006 at 10:39:36PM +0200, Mark Kettenis wrote:
> Hi Daniel,
> 
> There a slight problem with insert_single_step_breakpoint() and
> remove_single_step_breakpoints().  On OpenBSD, inserting a breakpoint
> into the kernel-provided signal trampoline, may fail.  Unfortunately,
> the caller of insert_single_step_breakpoint() never notices this, and
> calls remove_single_step_breakpoints() to remove the breakpoints.
> Unfortunately that makes us hit the gdb_assert() in there.
> 
> This patch makes us avoid this while still making an attempt to catch
> misuse of the interface.

Hmm - this is also true on some GNU/Linux targets, although none that
use software single step (yet - though I think I know how to trigger it
on ARM, some kernel fixes would also be needed).

My question is, is there something more useful we could be doing? 
If the single step breakpoint fails, then continuing will make us lose
control.  Maybe it should be an error().

-- 
Daniel Jacobowitz
CodeSourcery

Re: single-step breakpoints

[Mark Kettenis]

> Date: Tue, 25 Apr 2006 17:02:00 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Tue, Apr 25, 2006 at 10:39:36PM +0200, Mark Kettenis wrote:
> > Hi Daniel,
> > 
> > There a slight problem with insert_single_step_breakpoint() and
> > remove_single_step_breakpoints().  On OpenBSD, inserting a breakpoint
> > into the kernel-provided signal trampoline, may fail.  Unfortunately,
> > the caller of insert_single_step_breakpoint() never notices this, and
> > calls remove_single_step_breakpoints() to remove the breakpoints.
> > Unfortunately that makes us hit the gdb_assert() in there.
> > 
> > This patch makes us avoid this while still making an attempt to catch
> > misuse of the interface.
> 
> Hmm - this is also true on some GNU/Linux targets, although none that
> use software single step (yet - though I think I know how to trigger it
> on ARM, some kernel fixes would also be needed).
> 
> My question is, is there something more useful we could be doing? 
> If the single step breakpoint fails, then continuing will make us lose
> control.  Maybe it should be an error().

That may not be such a bad idea.  But we'll need to adjust
gdb.base/sigbpt.exp since it will end up in an infinite loop trying to
step the target.

I'll look a bit more into it later this week.  Need to get some sleep now.

Mark

Re: single-step breakpoints

[Mark Kettenis]

> Date: Tue, 25 Apr 2006 23:54:38 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> > Date: Tue, 25 Apr 2006 17:02:00 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > On Tue, Apr 25, 2006 at 10:39:36PM +0200, Mark Kettenis wrote:
> > > Hi Daniel,
> > > 
> > > There a slight problem with insert_single_step_breakpoint() and
> > > remove_single_step_breakpoints().  On OpenBSD, inserting a breakpoint
> > > into the kernel-provided signal trampoline, may fail.  Unfortunately,
> > > the caller of insert_single_step_breakpoint() never notices this, and
> > > calls remove_single_step_breakpoints() to remove the breakpoints.
> > > Unfortunately that makes us hit the gdb_assert() in there.
> > > 
> > > This patch makes us avoid this while still making an attempt to catch
> > > misuse of the interface.
> > 
> > Hmm - this is also true on some GNU/Linux targets, although none that
> > use software single step (yet - though I think I know how to trigger it
> > on ARM, some kernel fixes would also be needed).
> > 
> > My question is, is there something more useful we could be doing? 
> > If the single step breakpoint fails, then continuing will make us lose
> > control.  Maybe it should be an error().
> 
> That may not be such a bad idea.  But we'll need to adjust
> gdb.base/sigbpt.exp since it will end up in an infinite loop trying to
> step the target.
> 
> I'll look a bit more into it later this week.  Need to get some sleep now.

Yes, erroring out is defenitely a better approach than warn and have
things run away.  Here's a patch and some adjustments to the testsuite
to prevent the tests from going into an infinite loop.

ok?


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* breakpoint.c (insert_single_step_breakpoint): Make a failure to
	insert a single-step breakpoint an error instead of a warning.

	* breakpoint.c (remove_single_step_breakpoints): Bail out early if
	no breakpoints are inserted.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.225
diff -u -p -r1.225 breakpoint.c
--- breakpoint.c 18 Apr 2006 19:20:06 -0000 1.225
+++ breakpoint.c 29 Apr 2006 23:11:12 -0000
@@ -7717,7 +7717,7 @@ insert_single_step_breakpoint (CORE_ADDR
 
   *bpt_p = deprecated_insert_raw_breakpoint (next_pc);
   if (*bpt_p == NULL)
-    warning (_("Could not insert single-step breakpoint at 0x%s"),
+    error (_("Could not insert single-step breakpoint at 0x%s"),
 	     paddr_nz (next_pc));
 }
 
Index: testsuite/ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* gdb.base/sigbpt.exp (stepi_out): FAIL when inserting a
	single-step breakpoint fails; make this a KFAIL on
	sparc*-*-openbsd*.
	* gdb.base/siginfo.exp: Likewise.
	* gdb.base/sigstep.exp (advance, advancei): Likewise.

Index: testsuite/gdb.base/sigbpt.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/sigbpt.exp,v
retrieving revision 1.5
diff -u -p -r1.5 sigbpt.exp
--- testsuite/gdb.base/sigbpt.exp 27 Apr 2005 16:35:14 -0000 1.5
+++ testsuite/gdb.base/sigbpt.exp 29 Apr 2006 23:11:21 -0000
@@ -166,6 +166,10 @@ proc stepi_out { name args } {
     # trampoline, and back to the instruction that faulted.
     set test "${name}; stepi out of handler"
     gdb_test_multiple "stepi" "$test" {
+	-re "Could not insert single-step breakpoint.*$gdb_prompt $" {
+	    setup_kfail "sparc*-*-openbsd*" gdb/1736
+	    fail "$test (could not insert single-step breakpoint)"
+	}
 	-re "keeper.*$gdb_prompt $" {
 	    send_gdb "stepi\n"
 	    exp_continue
Index: testsuite/gdb.base/siginfo.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/siginfo.exp,v
retrieving revision 1.2
diff -u -p -r1.2 siginfo.exp
--- testsuite/gdb.base/siginfo.exp 23 Apr 2004 16:44:25 -0000 1.2
+++ testsuite/gdb.base/siginfo.exp 29 Apr 2006 23:11:21 -0000
@@ -75,6 +75,10 @@ gdb_expect_list "backtrace for nexti" ".
 # Check that GDB can step the inferior back to main
 set test "step out of handler"
 gdb_test_multiple "step" "${test}" {
+    -re "Could not insert single-step breakpoint.*$gdb_prompt $" {
+	setup_kfail sparc*-*-openbsd* gdb/1736
+	fail "$test (could not insert single-step breakpoint)"
+    }
     -re "done = 1;.*${gdb_prompt} $" {
 	send_gdb "$i\n"
 	exp_continue
Index: testsuite/gdb.base/sigstep.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/sigstep.exp,v
retrieving revision 1.10
diff -u -p -r1.10 sigstep.exp
--- testsuite/gdb.base/sigstep.exp 18 Jul 2005 19:23:54 -0000 1.10
+++ testsuite/gdb.base/sigstep.exp 29 Apr 2006 23:11:22 -0000
@@ -79,6 +79,10 @@ proc advance { i } {
 
     set test "$prefix; leave handler"
     gdb_test_multiple "$i" "${test}" {
+	-re "Could not insert single-step breakpoint.*$gdb_prompt $" {
+	    setup_kfail "sparc*-*-openbsd*" gdb/1736
+	    fail "$test (could not insert single-step breakpoint)"
+	}
 	-re "done = 1;.*${gdb_prompt} $" {
 	    send_gdb "$i\n"
 	    exp_continue -continue_timer
@@ -122,6 +126,10 @@ proc advancei { i } {
             fail "$test (could not set breakpoint)"
 	    return
         }
+	-re "Could not insert single-step breakpoint.*$gdb_prompt $" {
+	    setup_kfail "sparc*-*-openbsd*" gdb/1736
+	    fail "$test (could not insert single-step breakpoint)"
+	}
 	-re "done = 1;.*${gdb_prompt} $" {
 	    send_gdb "$i\n"
 	    exp_continue -continue_timer

Re: single-step breakpoints

[Daniel Jacobowitz]

On Sun, Apr 30, 2006 at 01:14:03AM +0200, Mark Kettenis wrote:
> Yes, erroring out is defenitely a better approach than warn and have
> things run away.  Here's a patch and some adjustments to the testsuite
> to prevent the tests from going into an infinite loop.
> 
> ok?
> 
> 
> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* breakpoint.c (insert_single_step_breakpoint): Make a failure to
> 	insert a single-step breakpoint an error instead of a warning.
> 
> 	* breakpoint.c (remove_single_step_breakpoints): Bail out early if
> 	no breakpoints are inserted.

Yeah, I think this is OK.

Ideally, of course, we wouldn't error here.  But I'm not sure what
would be more sensible to do.  We could quietly finish instead, but
that would go more than one instruction - quite bad.


-- 
Daniel Jacobowitz
CodeSourcery

Re: single-step breakpoints

[Mark Kettenis]

> Date: Mon, 1 May 2006 09:54:52 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> > Index: ChangeLog
> > from  Mark Kettenis  <kettenis@gnu.org>
> > 
> > 	* breakpoint.c (insert_single_step_breakpoint): Make a failure to
> > 	insert a single-step breakpoint an error instead of a warning.
> > 
> > 	* breakpoint.c (remove_single_step_breakpoints): Bail out early if
> > 	no breakpoints are inserted.
> 
> Yeah, I think this is OK.

Thanks, committed.

> Ideally, of course, we wouldn't error here.  But I'm not sure what
> would be more sensible to do.  We could quietly finish instead, but
> that would go more than one instruction - quite bad.

Actually the error is not too bad.  It warns you that single-stepping
is not possible and allows you to find another way of continuing.

Mark