[PATCH/RFC] Signals & single-stepping

[PATCH/RFC] Signals & single-stepping

[Mark Kettenis]

A while ago Daniel made a change to infcmd.c which broke hitting
breakpoints in signal handlers while single-stepping on OpenBSD (and
probably other ptrace-based native targets):

2009-01-20  Daniel Jacobowitz  <dan@codesourcery.com>

        PR gdb/9346
        * infcmd.c (signal_command): Do not specify a resume PC.

The problem is that breakpoints are removed for single-stepping.  So
the diff below fixes things.  Now it isn't clear to me if this
approach doesn't reintroduce the problem from the PR that Daniel tried
to fix.

Comments?


Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.410
diff -u -p -r1.410 infrun.c
--- infrun.c	29 Sep 2009 00:53:04 -0000	1.410
+++ infrun.c	30 Sep 2009 12:55:46 -0000
@@ -1604,6 +1604,11 @@ proceed (CORE_ADDR addr, enum target_sig
   else if (!signal_program[tp->stop_signal])
     tp->stop_signal = TARGET_SIGNAL_0;
 
+  if (tp->stop_signal != TARGET_SIGNAL_0) {
+	  tp->trap_expected = 0;
+	  insert_breakpoints ();
+  }
+
   annotate_starting ();
 
   /* Make sure that output from GDB appears before output from the

Re: [PATCH/RFC] Signals & single-stepping

[Daniel Jacobowitz]

On Wed, Sep 30, 2009 at 04:40:57PM +0200, Mark Kettenis wrote:
> A while ago Daniel made a change to infcmd.c which broke hitting
> breakpoints in signal handlers while single-stepping on OpenBSD (and
> probably other ptrace-based native targets):
> 
> 2009-01-20  Daniel Jacobowitz  <dan@codesourcery.com>
> 
>         PR gdb/9346
>         * infcmd.c (signal_command): Do not specify a resume PC.
> 
> The problem is that breakpoints are removed for single-stepping.  So
> the diff below fixes things.  Now it isn't clear to me if this
> approach doesn't reintroduce the problem from the PR that Daniel tried
> to fix.
> 
> Comments?

Your patch doesn't reintroduce the problem from the PR, and the new
tests in interrupt.exp pass on x86_64-linux.  I would really love
someone else to volunteer to review it though - trap_expected confuses
me horribly.  I'd guess this change could lead to hitting (and
displaying) the breakpoint at the current PC a second time, which is
undesirable.  On the other hand it might fix some of the signal tests
on software single-step targets...

> 
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.410
> diff -u -p -r1.410 infrun.c
> --- infrun.c	29 Sep 2009 00:53:04 -0000	1.410
> +++ infrun.c	30 Sep 2009 12:55:46 -0000
> @@ -1604,6 +1604,11 @@ proceed (CORE_ADDR addr, enum target_sig
>    else if (!signal_program[tp->stop_signal])
>      tp->stop_signal = TARGET_SIGNAL_0;
>  
> +  if (tp->stop_signal != TARGET_SIGNAL_0) {
> +	  tp->trap_expected = 0;
> +	  insert_breakpoints ();
> +  }
> +
>    annotate_starting ();
>  
>    /* Make sure that output from GDB appears before output from the
> 

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH/RFC] Signals & single-stepping

[Pedro Alves]

On Wednesday 30 September 2009 17:25:13, Daniel Jacobowitz wrote:
> Your patch doesn't reintroduce the problem from the PR, and the new
> tests in interrupt.exp pass on x86_64-linux.  I would really love
> someone else to volunteer to review it though - trap_expected confuses
> me horribly.  I'd guess this change could lead to hitting (and
> displaying) the breakpoint at the current PC a second time, which is
> undesirable.

Yes, this messes with hit counts, reruns user breakpoint
commands, etc.  Even some internal breakpoints don't like to
be re-hit for no reason.  E.g., see linux-thread-db.c:check_event
"Cannot get thread event message".

I think the issue is that when stepping over a breakpoint,
for simplicity, GDB always removes all breakpoints.  What if
we made it remove only breakpoints at stop_pc?

-- 
Pedro Alves

Re: [PATCH/RFC] Signals & single-stepping

[Pedro Alves]

On Thursday 01 October 2009 15:51:18, Pedro Alves wrote:
> On Wednesday 30 September 2009 17:25:13, Daniel Jacobowitz wrote:
> > Your patch doesn't reintroduce the problem from the PR, and the new
> > tests in interrupt.exp pass on x86_64-linux.  I would really love
> > someone else to volunteer to review it though - trap_expected confuses
> > me horribly.  I'd guess this change could lead to hitting (and
> > displaying) the breakpoint at the current PC a second time, which is
> > undesirable.
> 
> Yes, this messes with hit counts, reruns user breakpoint
> commands, etc.  Even some internal breakpoints don't like to
> be re-hit for no reason.  E.g., see linux-thread-db.c:check_event
> "Cannot get thread event message".
> 
> I think the issue is that when stepping over a breakpoint,
> for simplicity, GDB always removes all breakpoints.  What if
> we made it remove only breakpoints at stop_pc?

Bah, this would miss the same breakpoint we're stepping over,
if the signal handler calls it:

void
sighandler (int signo)
{
  foo ();
}

void
foo ()
{
}

(gdb) b foo
(gdb) c
<stopped at foo>
(gdb) signal SIGUSR1
<the foo call in signal handler is missed on targets
that can't step into signal handlers>

If we go the breakpoint re-hit path, it would be
nicer if we detected rehits and ignore them.  I'm not sure
how to distinguish the situation above from a rehit
due to single-stepping the breakpoint insn -- check
if the current frame id changed?  If we go that
path, we should perhaps also declare that code that
checks for breakpoint hits behind infrun.c's back
(like that check_event) should be more permissive of
re-hits, or otherwise be moved to the core stop
bpstat mechanism.

There may be other issues with rehits I haven't
thought of.  I'm just thinking out loud.

-- 
Pedro Alves

Re: [PATCH/RFC] Signals & single-stepping

[Daniel Jacobowitz]

On Thu, Oct 01, 2009 at 05:15:00PM +0100, Pedro Alves wrote:
> (gdb) signal SIGUSR1
> <the foo call in signal handler is missed on targets
> that can't step into signal handlers>

IMO that's an improvement over missing all breakpoints on such
targets...

-- 
Daniel Jacobowitz
CodeSourcery