public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
@ 2014-10-14 17:48 Pedro Alves
  2014-10-14 18:27 ` Eli Zaretskii
  2014-10-15 11:12 ` Yao Qi
  0 siblings, 2 replies; 14+ messages in thread
From: Pedro Alves @ 2014-10-14 17:48 UTC (permalink / raw)
  To: gdb-patches

I noticed that "si" behaves differently when a "handle nostop" signal
arrives while the step is in progress, depending on whether the
program was stopped at a breakpoint when "si" was entered.
Specifically, in case GDB needs to step off a breakpoint, the handler
is skipped and the program stops in the next "mainline" instruction.
Otherwise, the "si" stops in the first instruction of the signal
handler.

I was surprised the testsuite doesn't catch this difference.  Turns
out gdb.base/sigstep.exp covers a bunch of cases related to stepping
and signal handlers, but does not test stepi nor nexti, only
step/next/continue.

My first reaction was that stopping in the signal handler was the
correct thing to do, as it's where the next user-visible instruction
that is executed is.  I considered then "nexti" -- a signal handler
could be reasonably considered a subroutine call to step over, it'd
seem intuitive to me that "nexti" would skip it.

But then, I realized that signals that arrive while a plain/line
"step" is in progress _also_ have their handler skipped.  A user might
well be excused for being confused by this, given:

  (gdb) help step
  Step program until it reaches a different source line.

And the signal handler's sources will be in different source lines,
after all.

I think that having to explain that "stepi" steps into handlers, (and
that "nexti" wouldn't according to my reasoning above), while "step"
does, is a sign of an awkward interface.

E.g., if a user truly is interested in stepping into signal handlers,
then it's odd that she has to either force the signal to "handle
stop", or recall to do "stepi" whenever such a signal might be
delivered.  For that use case, it'd seem nicer to me if "step" also
stepped into handlers.

This suggests to me that we either need a global "step-into-handlers"
setting, or perhaps better, make "handle pass/nopass stop/nostop
print/noprint" have have an additional axis - "handle
stepinto/nostepinto", so that the user could configure whether
handlers for specific signals should be stepped into.

In any case, I think it's simpler (and thus better) for all step
commands to behave the same.  This commit thus makes "si/ni" skip
handlers for "handle nostop" signals that arrive while the command was
already in progress, like step/next do.

To be clear, nothing changes if the program was stopped for a signal,
and the user enters a stepping command _then_ -- GDB still steps into
the handler.  The change concerns signals that don't cause a stop and
that arrive while the step is in progress.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-14  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_signal_stop): Also skip handlers when a random
	signal arrives while handling a "stepi" or a "nexti".  Set the
	thread's 'step_after_step_resume_breakpoint' flag.

gdb/doc/
2014-10-14  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Signals): Clarify stepping and signal handlers.

gdb/testsuite/
2014-10-14  Pedro Alves  <palves@redhat.com>

	* gdb.base/sigstep.c (dummy): New global.
	(main): Issue a couple writes to the new global.
	* gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New
	procedures.
	(skip_over_handler): Use test_skip_handler.
	(top level): Call skip_over_handler for stepi and nexti too.
	(breakpoint_over_handler): Use test_skip_handler.
	(top level): Call breakpoint_over_handler for stepi and nexti too.
---
 gdb/doc/gdb.texinfo                |  9 +++++++
 gdb/infrun.c                       |  4 ++-
 gdb/testsuite/gdb.base/sigstep.c   |  7 +++--
 gdb/testsuite/gdb.base/sigstep.exp | 52 ++++++++++++++++++++++++++++----------
 4 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 429c650..46c327e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5526,6 +5526,11 @@ Their full names are:
 @value{GDBN} should not stop your program when this signal happens.  It may
 still print a message telling you that the signal has come in.
 
+If this signal arrives while a stepping command (e.g., @code{step}) is
+in progress, the signal's handler is skipped (though still executed if
+@code{pass} is in effect; see below).  @value{GDBN} will still stop
+your program if the handler hits a breakpoint.
+
 @item stop
 @value{GDBN} should stop your program when this signal happens.  This implies
 the @code{print} keyword as well.
@@ -5558,6 +5563,10 @@ after @value{GDBN} reports a signal, you can use the @code{handle}
 command with @code{pass} or @code{nopass} to control whether your
 program sees that signal when you continue.
 
+If a stepping command is issued after the program stops for a signal,
+and @code{pass} is in effect for that signal, @value{GDBN} steps into
+the signal's handler (if the target supports it).
+
 The default is set to @code{nostop}, @code{noprint}, @code{pass} for
 non-erroneous signals such as @code{SIGALRM}, @code{SIGWINCH} and
 @code{SIGCHLD}, and to @code{stop}, @code{print}, @code{pass} for the
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d61cc12..3682765 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4455,7 +4455,8 @@ handle_signal_stop (struct execution_control_state *ecs)
 
       if (ecs->event_thread->control.step_range_end != 0
 	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
-	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	      || ecs->event_thread->control.step_range_end == 1)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4475,6 +4476,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "single-step range\n");
 
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
+	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c
index aa2384a..25a4647 100644
--- a/gdb/testsuite/gdb.base/sigstep.c
+++ b/gdb/testsuite/gdb.base/sigstep.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 
 static volatile int done;
+static volatile int dummy;
 
 static void
 handler (int sig)
@@ -74,8 +75,10 @@ main ()
 	      return 1;
 	    }
 	}
-      /* Wait.  */
-      while (!done);
+      /* Wait.  Issue a couple writes to a dummy volatile var to be
+	 reasonably sure our simple "get-next-pc" logic doesn't
+	 stumble on branches.  */
+      dummy = 0; dummy = 1; while (!done);
       done = 0;
     }
   return 0;
diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index 184d46e..b589e12 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -269,9 +269,35 @@ proc skip_to_handler_entry { i } {
     gdb_test "clear *handler" ".*" "$prefix; clear handler"
 }
 
-skip_to_handler_entry step
-skip_to_handler_entry next
-skip_to_handler_entry continue
+foreach cmd {stepi nexti step next continue} {
+    skip_to_handler_entry $cmd
+}
+
+# Get the address of where a single-step should land.
+proc get_next_pc {test} {
+    global gdb_prompt
+    global hex
+
+    set next ""
+    gdb_test_multiple "x/2i \$pc" $test {
+	-re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	    set next $expect_out(1,string)
+	    pass $test
+	}
+    }
+
+    return $next
+}
+
+proc test_skip_handler {prefix i} {
+    if {$i == "stepi" || $i == "nexti"} {
+	set next_pc [get_next_pc "$prefix; get next PC"]
+	gdb_test "$i" "dummy = 0.*" "$prefix; performing $i"
+	gdb_test "p /x \$pc" " = $next_pc" "$prefix; advanced"
+    } else {
+	gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    }
+}
 
 # Try stepping when there's a signal pending but no breakpoints.
 # Should skip the handler advancing to the next line.
@@ -295,13 +321,13 @@ proc skip_over_handler { i } {
 
     # Make the signal pending
     sleep 1
-    
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+
+    test_skip_handler $prefix $i
 }
 
-skip_over_handler step
-skip_over_handler next
-skip_over_handler continue
+foreach cmd {stepi nexti step next continue} {
+    skip_over_handler $cmd
+}
 
 # Try stepping when there's a signal pending, a pre-existing
 # breakpoint at the current instruction, and a breakpoint in the
@@ -385,7 +411,7 @@ breakpoint_to_handler_entry continue
 
 # Try stepping when there's a signal pending, and a pre-existing
 # breakpoint at the current instruction, and no breakpoint in the
-# handler.  Should advance to the next line.
+# handler.  Should advance to the next line/instruction.
 
 proc breakpoint_over_handler { i } {
     global gdb_prompt
@@ -409,10 +435,10 @@ proc breakpoint_over_handler { i } {
     # Make the signal pending
     sleep 1
     
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    test_skip_handler $prefix $i
     gdb_test "clear $infinite_loop" ".*" "$prefix; clear infinite loop"
 }
 
-breakpoint_over_handler step
-breakpoint_over_handler next
-breakpoint_over_handler continue
+foreach cmd {stepi nexti step next continue} {
+    breakpoint_over_handler $cmd
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 17:48 [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives Pedro Alves
@ 2014-10-14 18:27 ` Eli Zaretskii
  2014-10-14 18:49   ` Pedro Alves
  2014-10-15 11:12 ` Yao Qi
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2014-10-14 18:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 14 Oct 2014 18:48:30 +0100
> 
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -5526,6 +5526,11 @@ Their full names are:
>  @value{GDBN} should not stop your program when this signal happens.  It may
>  still print a message telling you that the signal has come in.
>  
> +If this signal arrives while a stepping command (e.g., @code{step}) is
> +in progress, the signal's handler is skipped (though still executed if
> +@code{pass} is in effect; see below).  @value{GDBN} will still stop
> +your program if the handler hits a breakpoint.

This description is confusing.  For starters, it only mentions some of
the possible setting of signal handling, and keeps silence about the
rest.  Either we should describe what happens with all of them, one by
one, or (better) says something that will explain how we handle them
all, at once.

Also, I believe the description of stepping should mention this
aspect, with a cross-reference to here.

> +If a stepping command is issued after the program stops for a signal,
> +and @code{pass} is in effect for that signal, @value{GDBN} steps into
> +the signal's handler (if the target supports it).

Again, this left me wondering.  E.g., if the program stops for a
signal, then we are already in the signal handler, no?  So the fact
that stepping commands continue there is a no-brainer, right?  Or a I
again confused?

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 18:27 ` Eli Zaretskii
@ 2014-10-14 18:49   ` Pedro Alves
  2014-10-14 18:56     ` Pedro Alves
  2014-10-14 19:22     ` Eli Zaretskii
  0 siblings, 2 replies; 14+ messages in thread
From: Pedro Alves @ 2014-10-14 18:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/14/2014 07:27 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 14 Oct 2014 18:48:30 +0100
>>
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -5526,6 +5526,11 @@ Their full names are:
>>  @value{GDBN} should not stop your program when this signal happens.  It may
>>  still print a message telling you that the signal has come in.
>>  
>> +If this signal arrives while a stepping command (e.g., @code{step}) is
>> +in progress, the signal's handler is skipped (though still executed if
>> +@code{pass} is in effect; see below).  @value{GDBN} will still stop
>> +your program if the handler hits a breakpoint.
> 
> This description is confusing.  For starters, it only mentions some of
> the possible setting of signal handling, and keeps silence about the
> rest.  Either we should describe what happens with all of them, one by
> one, or (better) says something that will explain how we handle them
> all, at once.

This paragraph is added to the "nostop" entry of the table.  It directly
relates to the entry in question:

Specifically, it's a preemptive response to the question I'd have if
I read the paragraph just above, which talks about the signal but
leaves the question of the signal handler open:

 @table @code
 @item nostop
 @value{GDBN} should not stop your program when this signal happens.  It may
 still print a message telling you that the signal has come in.

 If this signal arrives while a stepping command (e.g., @code{step}) is
 in progress, the signal's handler is skipped (though still executed if
 @code{pass} is in effect; see below).  @value{GDBN} will still stop
 your program if the handler hits a breakpoint.

I could extend the "stop" item:

 @item stop
 @value{GDBN} should stop your program when this signal happens.  This implies
 the @code{print} keyword as well.

Like:

+ The signal is not visible to the program until you continue.

WDYT?


This is also said further below, after the table (and is what the
"see below" referred to):

  When a signal stops your program, the signal is not visible to the
  program until you
  continue.  Your program sees the signal then, if @code{pass} is in
  effect for the signal in question @emph{at that time}.  In other words,
  after @value{GDBN} reports a signal, you can use the @code{handle}
  command with @code{pass} or @code{nopass} to control whether your
  program sees that signal when you continue.

 +If a stepping command is issued after the program stops for a signal,
 +and @code{pass} is in effect for that signal, @value{GDBN} steps into
 +the signal's handler (if the target supports it).

The '+' lines are what I'm adding.
> 
> Also, I believe the description of stepping should mention this
> aspect, with a cross-reference to here.

OK.

> 
>> +If a stepping command is issued after the program stops for a signal,
>> +and @code{pass} is in effect for that signal, @value{GDBN} steps into
>> +the signal's handler (if the target supports it).
> 
> Again, this left me wondering.  E.g., if the program stops for a
> signal, then we are already in the signal handler, no?

No, we intercept the signal before the program sees it.  See above.

> So the fact
> that stepping commands continue there is a no-brainer, right?  Or a I
> again confused?

The latter.  :-)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 18:49   ` Pedro Alves
@ 2014-10-14 18:56     ` Pedro Alves
  2014-10-14 19:22     ` Eli Zaretskii
  1 sibling, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2014-10-14 18:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/14/2014 07:49 PM, Pedro Alves wrote:
> On 10/14/2014 07:27 PM, Eli Zaretskii wrote:
>>> From: Pedro Alves <palves@redhat.com>
>>> Date: Tue, 14 Oct 2014 18:48:30 +0100
>>>
>>> --- a/gdb/doc/gdb.texinfo
>>> +++ b/gdb/doc/gdb.texinfo
>>> @@ -5526,6 +5526,11 @@ Their full names are:
>>>  @value{GDBN} should not stop your program when this signal happens.  It may
>>>  still print a message telling you that the signal has come in.
>>>  
>>> +If this signal arrives while a stepping command (e.g., @code{step}) is
>>> +in progress, the signal's handler is skipped (though still executed if
>>> +@code{pass} is in effect; see below).  @value{GDBN} will still stop
>>> +your program if the handler hits a breakpoint.
>>
>> This description is confusing.  For starters, it only mentions some of
>> the possible setting of signal handling, and keeps silence about the
>> rest.  Either we should describe what happens with all of them, one by
>> one, or (better) says something that will explain how we handle them
>> all, at once.
> 
> This paragraph is added to the "nostop" entry of the table.  It directly
> relates to the entry in question:
> 
> Specifically, it's a preemptive response to the question I'd have if
> I read the paragraph just above, which talks about the signal but
> leaves the question of the signal handler open:
> 
>  @table @code
>  @item nostop
>  @value{GDBN} should not stop your program when this signal happens.  It may
>  still print a message telling you that the signal has come in.
> 
>  If this signal arrives while a stepping command (e.g., @code{step}) is
>  in progress, the signal's handler is skipped (though still executed if
>  @code{pass} is in effect; see below).  @value{GDBN} will still stop
>  your program if the handler hits a breakpoint.
> 
> I could extend the "stop" item:
> 
>  @item stop
>  @value{GDBN} should stop your program when this signal happens.  This implies
>  the @code{print} keyword as well.
> 
> Like:
> 
> + The signal is not visible to the program until you continue.
> 
> WDYT?
> 
> 
> This is also said further below, after the table (and is what the
> "see below" referred to):
> 
>   When a signal stops your program, the signal is not visible to the
>   program until you
>   continue.  Your program sees the signal then, if @code{pass} is in
>   effect for the signal in question @emph{at that time}.  In other words,
>   after @value{GDBN} reports a signal, you can use the @code{handle}
>   command with @code{pass} or @code{nopass} to control whether your
>   program sees that signal when you continue.
> 
>  +If a stepping command is issued after the program stops for a signal,
>  +and @code{pass} is in effect for that signal, @value{GDBN} steps into
>  +the signal's handler (if the target supports it).
> 
> The '+' lines are what I'm adding.

Would this tweak below make it clearer?  The contrast against stepping
mainline code is really the point I'm trying to make:

 If a stepping command is issued after the program stops for a signal,
 and @code{pass} is in effect for that signal, @value{GDBN} steps into
 the signal's handler, instead of stepping the mainline code, if
 the target supports it.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 18:49   ` Pedro Alves
  2014-10-14 18:56     ` Pedro Alves
@ 2014-10-14 19:22     ` Eli Zaretskii
  2014-10-15 13:40       ` Pedro Alves
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2014-10-14 19:22 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Tue, 14 Oct 2014 19:49:40 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> >> +If this signal arrives while a stepping command (e.g., @code{step}) is
> >> +in progress, the signal's handler is skipped (though still executed if
> >> +@code{pass} is in effect; see below).  @value{GDBN} will still stop
> >> +your program if the handler hits a breakpoint.
> > 
> > This description is confusing.  For starters, it only mentions some of
> > the possible setting of signal handling, and keeps silence about the
> > rest.  Either we should describe what happens with all of them, one by
> > one, or (better) says something that will explain how we handle them
> > all, at once.
> 
> This paragraph is added to the "nostop" entry of the table.

I was talking about the "pass" part, not about "nostop".

> It directly relates to the entry in question:
> 
> Specifically, it's a preemptive response to the question I'd have if
> I read the paragraph just above, which talks about the signal but
> leaves the question of the signal handler open:
> 
>  @table @code
>  @item nostop
>  @value{GDBN} should not stop your program when this signal happens.  It may
>  still print a message telling you that the signal has come in.
> 
>  If this signal arrives while a stepping command (e.g., @code{step}) is
>  in progress, the signal's handler is skipped (though still executed if
>  @code{pass} is in effect; see below).  @value{GDBN} will still stop
>  your program if the handler hits a breakpoint.

Sorry, I'm still in the woods.

> I could extend the "stop" item:
> 
>  @item stop
>  @value{GDBN} should stop your program when this signal happens.  This implies
>  the @code{print} keyword as well.
> 
> Like:
> 
> + The signal is not visible to the program until you continue.

I think this just muddies the water, sorry.

> This is also said further below, after the table (and is what the
> "see below" referred to):
> 
>   When a signal stops your program, the signal is not visible to the
>   program until you
>   continue.  Your program sees the signal then, if @code{pass} is in
>   effect for the signal in question @emph{at that time}.  In other words,
>   after @value{GDBN} reports a signal, you can use the @code{handle}
>   command with @code{pass} or @code{nopass} to control whether your
>   program sees that signal when you continue.

This is also confusing.

>  +If a stepping command is issued after the program stops for a signal,
>  +and @code{pass} is in effect for that signal, @value{GDBN} steps into
>  +the signal's handler (if the target supports it).
> 
> The '+' lines are what I'm adding.

I think this text mixes 2 different things: (1) how the signal
handling affects whether the signal gets to the program or not, and is
it announced or not; and (2) the fine details of when the signal
becomes "known" to the program and how stepping commands affect and
are affected by that.

It might be the simplest to separate the two issues, and describe each
one on its own.

> >> +If a stepping command is issued after the program stops for a signal,
> >> +and @code{pass} is in effect for that signal, @value{GDBN} steps into
> >> +the signal's handler (if the target supports it).
> > 
> > Again, this left me wondering.  E.g., if the program stops for a
> > signal, then we are already in the signal handler, no?
> 
> No, we intercept the signal before the program sees it.  See above.

But you wrote "the program stops for a signal".  "The program stops"
means (or at least could be interpreted as meaning) the signal was
already seen by the program, and the program then stopped.

See how this is confusing?

If you want to describe the fine details of signal handling by GDB,
you need to tell more, and be very careful with your wording.

> > So the fact
> > that stepping commands continue there is a no-brainer, right?  Or a I
> > again confused?
> 
> The latter.  :-)

Then our readers will be even more confused.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 17:48 [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives Pedro Alves
  2014-10-14 18:27 ` Eli Zaretskii
@ 2014-10-15 11:12 ` Yao Qi
  2014-10-15 13:44   ` Pedro Alves
  1 sibling, 1 reply; 14+ messages in thread
From: Yao Qi @ 2014-10-15 11:12 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> I think that having to explain that "stepi" steps into handlers, (and
> that "nexti" wouldn't according to my reasoning above), while "step"
> does, is a sign of an awkward interface.
>

I suspect you meant "step" does NOT, right?

> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d61cc12..3682765 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4455,7 +4455,8 @@ handle_signal_stop (struct execution_control_state *ecs)
>  
>        if (ecs->event_thread->control.step_range_end != 0
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Is it still needed?

>  	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
> -	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
> +	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
> +	      || ecs->event_thread->control.step_range_end == 1)
>  	  && frame_id_eq (get_stack_frame_id (frame),
>  			  ecs->event_thread->control.step_stack_frame_id)
>  	  && ecs->event_thread->control.step_resume_breakpoint == NULL)

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-14 19:22     ` Eli Zaretskii
@ 2014-10-15 13:40       ` Pedro Alves
  2014-10-15 14:31         ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2014-10-15 13:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/14/2014 08:22 PM, Eli Zaretskii wrote:
>> Date: Tue, 14 Oct 2014 19:49:40 +0100
>> From: Pedro Alves <palves@redhat.com>

> I think this text mixes 2 different things: (1) how the signal
> handling affects whether the signal gets to the program or not, and is
> it announced or not; and (2) the fine details of when the signal
> becomes "known" to the program and how stepping commands affect and
> are affected by that.
> 
> It might be the simplest to separate the two issues, and describe each
> one on its own.

I'll give that a try.

> 
>>>> +If a stepping command is issued after the program stops for a signal,
>>>> +and @code{pass} is in effect for that signal, @value{GDBN} steps into
>>>> +the signal's handler (if the target supports it).
>>>
>>> Again, this left me wondering.  E.g., if the program stops for a
>>> signal, then we are already in the signal handler, no?
>>
>> No, we intercept the signal before the program sees it.  See above.
> 
> But you wrote "the program stops for a signal".  "The program stops"
> means (or at least could be interpreted as meaning) the signal was
> already seen by the program, and the program then stopped.
> 
> See how this is confusing?

I don't see how one would be confused, as the paragraph just above
says "When a signal stops your program", and I feel that the that
wording I chose follows naturally from that.

But, anyway, I'll try to clarify this.

>>> So the fact
>>> that stepping commands continue there is a no-brainer, right?  Or a I
>>> again confused?
>>
>> The latter.  :-)
> 
> Then our readers will be even more confused.

Eheh, so true...  I often say that myself.

I'll come up with a new version once I have a chance.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-15 11:12 ` Yao Qi
@ 2014-10-15 13:44   ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2014-10-15 13:44 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 10/15/2014 12:08 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> I think that having to explain that "stepi" steps into handlers, (and
>> that "nexti" wouldn't according to my reasoning above), while "step"
>> does, is a sign of an awkward interface.
>>
> 
> I suspect you meant "step" does NOT, right?

Gah.  Indeed.

> 
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index d61cc12..3682765 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -4455,7 +4455,8 @@ handle_signal_stop (struct execution_control_state *ecs)
>>  
>>        if (ecs->event_thread->control.step_range_end != 0
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Is it still needed?
> 

Hmm, yeah, looks like it never was.

>>  	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
>> -	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
>> +	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
>> +	      || ecs->event_thread->control.step_range_end == 1)
>>  	  && frame_id_eq (get_stack_frame_id (frame),
>>  			  ecs->event_thread->control.step_stack_frame_id)
>>  	  && ecs->event_thread->control.step_resume_breakpoint == NULL)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-15 13:40       ` Pedro Alves
@ 2014-10-15 14:31         ` Eli Zaretskii
  2014-10-25 14:00           ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2014-10-15 14:31 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Wed, 15 Oct 2014 14:40:49 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> On 10/14/2014 08:22 PM, Eli Zaretskii wrote:
> >> Date: Tue, 14 Oct 2014 19:49:40 +0100
> >> From: Pedro Alves <palves@redhat.com>
> 
> > I think this text mixes 2 different things: (1) how the signal
> > handling affects whether the signal gets to the program or not, and is
> > it announced or not; and (2) the fine details of when the signal
> > becomes "known" to the program and how stepping commands affect and
> > are affected by that.
> > 
> > It might be the simplest to separate the two issues, and describe each
> > one on its own.
> 
> I'll give that a try.

Thanks.  Let me know if I can help.

> >> No, we intercept the signal before the program sees it.  See above.
> > 
> > But you wrote "the program stops for a signal".  "The program stops"
> > means (or at least could be interpreted as meaning) the signal was
> > already seen by the program, and the program then stopped.
> > 
> > See how this is confusing?
> 
> I don't see how one would be confused, as the paragraph just above
> says "When a signal stops your program", and I feel that the that
> wording I chose follows naturally from that.

As long as we didn't try to talk about fine details that happen at
that time, it was okay.

> I'll come up with a new version once I have a chance.

Thanks in advance.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-15 14:31         ` Eli Zaretskii
@ 2014-10-25 14:00           ` Pedro Alves
  2014-10-27 17:39             ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2014-10-25 14:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/15/2014 03:30 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> On 10/14/2014 08:22 PM, Eli Zaretskii wrote:
>>> I think this text mixes 2 different things: (1) how the signal
>>> handling affects whether the signal gets to the program or not, and is
>>> it announced or not; and (2) the fine details of when the signal
>>> becomes "known" to the program and how stepping commands affect and
>>> are affected by that.
>>>
>>> It might be the simplest to separate the two issues, and describe each
>>> one on its own.
>>
>> I'll give that a try.
> 
> Thanks.  Let me know if I can help.

Alright, here's a new, expanded version.

Let me know what you think.

From f95a5005ede386ce026b095ebeb48f238392fffa Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 25 Oct 2014 14:14:15 +0100
Subject: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal
 arrives

I noticed that "si" behaves differently when a "handle nostop" signal
arrives while the step is in progress, depending on whether the
program was stopped at a breakpoint when "si" was entered.
Specifically, in case GDB needs to step off a breakpoint, the handler
is skipped and the program stops in the next "mainline" instruction.
Otherwise, the "si" stops in the first instruction of the signal
handler.

I was surprised the testsuite doesn't catch this difference.  Turns
out gdb.base/sigstep.exp covers a bunch of cases related to stepping
and signal handlers, but does not test stepi nor nexti, only
step/next/continue.

My first reaction was that stopping in the signal handler was the
correct thing to do, as it's where the next user-visible instruction
that is executed is.  I considered then "nexti" -- a signal handler
could be reasonably considered a subroutine call to step over, it'd
seem intuitive to me that "nexti" would skip it.

But then, I realized that signals that arrive while a plain/line
"step" is in progress _also_ have their handler skipped.  A user might
well be excused for being confused by this, given:

  (gdb) help step
  Step program until it reaches a different source line.

And the signal handler's sources will be in different source lines,
after all.

I think that having to explain that "stepi" steps into handlers, (and
that "nexti" wouldn't according to my reasoning above), while "step"
does not, is a sign of an awkward interface.

E.g., if a user truly is interested in stepping into signal handlers,
then it's odd that she has to either force the signal to "handle
stop", or recall to do "stepi" whenever such a signal might be
delivered.  For that use case, it'd seem nicer to me if "step" also
stepped into handlers.

This suggests to me that we either need a global "step-into-handlers"
setting, or perhaps better, make "handle pass/nopass stop/nostop
print/noprint" have have an additional axis - "handle
stepinto/nostepinto", so that the user could configure whether
handlers for specific signals should be stepped into.

In any case, I think it's simpler (and thus better) for all step
commands to behave the same.  This commit thus makes "si/ni" skip
handlers for "handle nostop" signals that arrive while the command was
already in progress, like step/next do.

To be clear, nothing changes if the program was stopped for a signal,
and the user enters a stepping command _then_ -- GDB still steps into
the handler.  The change concerns signals that don't cause a stop and
that arrive while the step is in progress.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-25  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_signal_stop): Also skip handlers when a random
	signal arrives while handling a "stepi" or a "nexti".  Set the
	thread's 'step_after_step_resume_breakpoint' flag.

gdb/doc/
2014-10-25  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Continuing and Stepping): Add cross reference to
	info on stepping and signal handlers.
	(Signals): Explain stepping and signal handlers.  Add context
	index entries, and cross references.

gdb/testsuite/
2014-10-25  Pedro Alves  <palves@redhat.com>

	* gdb.base/sigstep.c (dummy): New global.
	(main): Issue a couple writes to the new global.
	* gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New
	procedures.
	(skip_over_handler): Use test_skip_handler.
	(top level): Call skip_over_handler for stepi and nexti too.
	(breakpoint_over_handler): Use test_skip_handler.
	(top level): Call breakpoint_over_handler for stepi and nexti too.
---
 gdb/doc/gdb.texinfo                | 57 +++++++++++++++++++++++++++++++++++++-
 gdb/infrun.c                       |  7 +++--
 gdb/testsuite/gdb.base/sigstep.c   |  7 +++--
 gdb/testsuite/gdb.base/sigstep.exp | 52 +++++++++++++++++++++++++---------
 4 files changed, 104 insertions(+), 19 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a1b8ac7..af1c1c7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5079,7 +5079,9 @@ line of source code, or one machine instruction (depending on what
 particular command you use).  Either when continuing or when stepping,
 your program may stop even sooner, due to a breakpoint or a signal.  (If
 it stops due to a signal, you may want to use @code{handle}, or use
-@samp{signal 0} to resume execution.  @xref{Signals, ,Signals}.)
+@samp{signal 0} to resume execution (@pxref{Signals, ,Signals}),
+or you may step into the signal's handler (@pxref{stepping and signal
+handlers}).)
 
 @table @code
 @kindex continue
@@ -5573,6 +5575,56 @@ a result of the fatal signal once it saw the signal.  To prevent this,
 you can continue with @samp{signal 0}.  @xref{Signaling, ,Giving your
 Program a Signal}.
 
+@cindex stepping and signal handlers
+@anchor{stepping and signal handlers}
+
+@value{GDBN} optimizes for stepping the mainline code.  If a signal
+that has @code{handle nostop} and @code{handle pass} set arrives while
+a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
+in progress, @value{GDBN} lets the signal handler run and then resumes
+stepping the mainline code once the signal handler returns.  In other
+words, @value{GDBN} steps over the signal handler.  If the signal has
+@code{handle noprint} set, then you won't even hear about it.  This
+prevents signals that you've specified as not interesting (with
+@code{handle nostop}) from changing the focus of debugging
+unexpectedly.  Note that the signal handler itself may still hit a
+breakpoint, stop for another signal that has @code{handle stop} in
+effect, or for any other event that normally results in stopping the
+stepping command sooner.
+
+@cindex stepping into signal handlers
+@anchor{stepping into signal handlers}
+
+If the program was stopped for a signal (that is, stopped before the
+program sees it), due to @code{handle stop} being set, and
+@code{handle pass} is in effect for that signal too, and your program
+handles the signal, a stepping command such as for example
+@code{stepi} or @code{step} steps @emph{into} the signal's handler (if
+the target supports it).
+
+Likewise, if the @code{queue-signal} command was used to queue a
+signal to be delivered to the current thread when execution of the
+thread resumes (@pxref{Signaling, ,Giving your Program a Signal}),
+then a stepping command steps into the signal's handler.
+
+Here's an example, using @code{stepi} to step to the first instruction
+of @code{SIGUSR1}'s handler:
+
+@smallexample
+(@value{GDBP}) handle SIGUSR1
+Signal        Stop      Print   Pass to program Description
+SIGUSR1       Yes       Yes     Yes             User defined signal 1
+(@value{GDBP}) c
+Continuing.
+
+Program received signal SIGUSR1, User defined signal 1.
+main () sigusr1.c:28
+28        p = 0;
+(@value{GDBP}) si
+sigusr1_handler () at sigusr1.c:9
+9       @{
+@end smallexample
+
 @cindex extra signal information
 @anchor{extra signal information}
 
@@ -16654,6 +16706,9 @@ be used to pass a signal whose handling state has been set to @code{nopass}
 @end table
 @c @end group
 
+@xref{stepping into signal handlers}, for information on how stepping
+commands behave when the thread has a signal queued.
+
 @node Returning
 @section Returning from a Function
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 90a3123..df053e2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4463,9 +4463,9 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  return;
 	}
 
-      if (ecs->event_thread->control.step_range_end != 0
-	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
-	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
+      if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
+	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	      || ecs->event_thread->control.step_range_end == 1)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4485,6 +4485,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "single-step range\n");
 
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
+	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c
index aa2384a..25a4647 100644
--- a/gdb/testsuite/gdb.base/sigstep.c
+++ b/gdb/testsuite/gdb.base/sigstep.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 
 static volatile int done;
+static volatile int dummy;
 
 static void
 handler (int sig)
@@ -74,8 +75,10 @@ main ()
 	      return 1;
 	    }
 	}
-      /* Wait.  */
-      while (!done);
+      /* Wait.  Issue a couple writes to a dummy volatile var to be
+	 reasonably sure our simple "get-next-pc" logic doesn't
+	 stumble on branches.  */
+      dummy = 0; dummy = 1; while (!done);
       done = 0;
     }
   return 0;
diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index 184d46e..b589e12 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -269,9 +269,35 @@ proc skip_to_handler_entry { i } {
     gdb_test "clear *handler" ".*" "$prefix; clear handler"
 }
 
-skip_to_handler_entry step
-skip_to_handler_entry next
-skip_to_handler_entry continue
+foreach cmd {stepi nexti step next continue} {
+    skip_to_handler_entry $cmd
+}
+
+# Get the address of where a single-step should land.
+proc get_next_pc {test} {
+    global gdb_prompt
+    global hex
+
+    set next ""
+    gdb_test_multiple "x/2i \$pc" $test {
+	-re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	    set next $expect_out(1,string)
+	    pass $test
+	}
+    }
+
+    return $next
+}
+
+proc test_skip_handler {prefix i} {
+    if {$i == "stepi" || $i == "nexti"} {
+	set next_pc [get_next_pc "$prefix; get next PC"]
+	gdb_test "$i" "dummy = 0.*" "$prefix; performing $i"
+	gdb_test "p /x \$pc" " = $next_pc" "$prefix; advanced"
+    } else {
+	gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    }
+}
 
 # Try stepping when there's a signal pending but no breakpoints.
 # Should skip the handler advancing to the next line.
@@ -295,13 +321,13 @@ proc skip_over_handler { i } {
 
     # Make the signal pending
     sleep 1
-    
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+
+    test_skip_handler $prefix $i
 }
 
-skip_over_handler step
-skip_over_handler next
-skip_over_handler continue
+foreach cmd {stepi nexti step next continue} {
+    skip_over_handler $cmd
+}
 
 # Try stepping when there's a signal pending, a pre-existing
 # breakpoint at the current instruction, and a breakpoint in the
@@ -385,7 +411,7 @@ breakpoint_to_handler_entry continue
 
 # Try stepping when there's a signal pending, and a pre-existing
 # breakpoint at the current instruction, and no breakpoint in the
-# handler.  Should advance to the next line.
+# handler.  Should advance to the next line/instruction.
 
 proc breakpoint_over_handler { i } {
     global gdb_prompt
@@ -409,10 +435,10 @@ proc breakpoint_over_handler { i } {
     # Make the signal pending
     sleep 1
     
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    test_skip_handler $prefix $i
     gdb_test "clear $infinite_loop" ".*" "$prefix; clear infinite loop"
 }
 
-breakpoint_over_handler step
-breakpoint_over_handler next
-breakpoint_over_handler continue
+foreach cmd {stepi nexti step next continue} {
+    breakpoint_over_handler $cmd
+}
-- 
1.9.3


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-25 14:00           ` Pedro Alves
@ 2014-10-27 17:39             ` Eli Zaretskii
  2014-10-27 19:44               ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2014-10-27 17:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Sat, 25 Oct 2014 15:00:40 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> >>> It might be the simplest to separate the two issues, and describe each
> >>> one on its own.
> >>
> >> I'll give that a try.
> > 
> > Thanks.  Let me know if I can help.
> 
> Alright, here's a new, expanded version.
> 
> Let me know what you think.

Thanks, this is a huge improvement.  I have only a couple of minor
stylistic suggestions:

> +@cindex stepping and signal handlers
> +@anchor{stepping and signal handlers}
> +
> +@value{GDBN} optimizes for stepping the mainline code.  If a signal
> +that has @code{handle nostop} and @code{handle pass} set arrives while
> +a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
> +in progress, @value{GDBN} lets the signal handler run and then resumes
> +stepping the mainline code once the signal handler returns.  In other
> +words, @value{GDBN} steps over the signal handler.  If the signal has
> +@code{handle noprint} set, then you won't even hear about it.  This
> +prevents signals that you've specified as not interesting (with

I would suggest to use a semi-colon, not a period, before the last
"This".  That's because the last sentence is logically an immediate
continuation of the one before it.  By putting a full stop between
them we create a potential for misunderstanding to what "this" refers,
since the previous text described 2 different situations.  Using a
semi-colon removes that danger.

For the same reason, it might be better to make "If the signal has
'handle noprint' ..." start a new paragraph.

> +@cindex stepping into signal handlers
> +@anchor{stepping into signal handlers}

I would remove this @cindex entry: it doesn't add anything useful to
the previous one, and will likely point to the same page.

> +If the program was stopped for a signal (that is, stopped before the
> +program sees it), due to @code{handle stop} being set, and
> +@code{handle pass} is in effect for that signal too, and your program
> +handles the signal, a stepping command such as for example
> +@code{stepi} or @code{step} steps @emph{into} the signal's handler (if
> +the target supports it).

This is a mouthful, not in the least because of excessive use of past
tense.  How about this variant instead:

  If you set @code{handle stop} for a signal, @value{GDBN} stops your
  program and announces the signal when it arrives, before the program
  sees it.  If you also set @code{handle pass} for that signal, and
  your program sets up a handler for it, then issuing a stepping
  command, such as @code{step} or @code{stepi}, when your program is
  stopped due to the signal will step @emph{into} the signal handler
  (if the target supports that).

> +Likewise, if the @code{queue-signal} command was used to queue a
> +signal to be delivered to the current thread when execution of the

Please reword in active tens ("... if you use the @code{queue-signal}
command to queue ...").

> +thread resumes (@pxref{Signaling, ,Giving your Program a Signal}),
> +then a stepping command steps into the signal's handler.

Not sure I understand the sequence here.  First, I queue-signal, then
the signal is delivered and the thread stops, and _then_ I issue si?
I guess the "when execution of the thread resumes" confused me.

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-27 17:39             ` Eli Zaretskii
@ 2014-10-27 19:44               ` Pedro Alves
  2014-10-27 19:58                 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2014-10-27 19:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/27/2014 05:38 PM, Eli Zaretskii wrote:
>> Date: Sat, 25 Oct 2014 15:00:40 +0100
>> From: Pedro Alves <palves@redhat.com>

> Thanks, this is a huge improvement.  I have only a couple of minor
> stylistic suggestions:
> 

Thanks.

>> +@cindex stepping and signal handlers
>> +@anchor{stepping and signal handlers}
>> +
>> +@value{GDBN} optimizes for stepping the mainline code.  If a signal
>> +that has @code{handle nostop} and @code{handle pass} set arrives while
>> +a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
>> +in progress, @value{GDBN} lets the signal handler run and then resumes
>> +stepping the mainline code once the signal handler returns.  In other
>> +words, @value{GDBN} steps over the signal handler.  If the signal has
>> +@code{handle noprint} set, then you won't even hear about it.  This
>> +prevents signals that you've specified as not interesting (with
> 
> I would suggest to use a semi-colon, not a period, before the last
> "This".  That's because the last sentence is logically an immediate
> continuation of the one before it.  By putting a full stop between
> them we create a potential for misunderstanding to what "this" refers,
> since the previous text described 2 different situations.  Using a
> semi-colon removes that danger.

Hmm, I don't think I agree with that.  I'm not really trying to
describ 2 situations.  The "this" refers to GDB stepping over the
signal handler.  The noprint issue is secondary here, and I
guess an example shows better what I'm talking about:

(gdb) list 28
28        p = 0;
29        p = 0;
30        p = 0;
(gdb) n
28        p = 0;
(gdb) info inferiors 
  Num  Description       Executable        
* 1    process 25468     /home/pedro/gdb/tests/signal
(gdb) shell kill -SIGUSR1 25468
(gdb) handle SIGUSR1 nostop pass print
Signal        Stop      Print   Pass to program Description
SIGUSR1       No        Yes     Yes             User defined signal 1
(gdb) si

Program received signal SIGUSR1, User defined signal 1.
29        p = 0;
(gdb) 

That stepped over the signal handler, from line 28 to 29, but we
still heard about the signal.

Vs:

(gdb) 
28        p = 0;
(gdb) handle SIGUSR1 nostop noprint pass
Signal        Stop      Print   Pass to program Description
SIGUSR1       No        No      Yes             User defined signal 1
(gdb) info inferiors 
  Num  Description       Executable        
* 1    process 25484     /home/pedro/gdb/tests/signal
(gdb) shell kill -SIGUSR1 25484
(gdb) si
29        p = 0;
(gdb) 

That stepped over the signal handler, from line 28 to 29, and we
didn't even hear about the signal.

So, perhaps this variant is clearer?

@value{GDBN} optimizes for stepping the mainline code.  If a signal
that has @code{handle nostop} and @code{handle pass} set arrives while
a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
in progress, @value{GDBN} lets the signal handler run and then resumes
stepping the mainline code once the signal handler returns.  In other
words, @value{GDBN} steps over the signal handler.  This prevents
signals that you've specified as not interesting (with @code{handle
nostop}) from changing the focus of debugging unexpectedly.  Note that
the signal handler itself may still hit a breakpoint, stop for another
signal that has @code{handle stop} in effect, or for any other event
that normally results in stopping the stepping command sooner.  Also
note that @value{GDBN} still informs you that the program received a
signal if @code{handle print} is set.


> 
> For the same reason, it might be better to make "If the signal has
> 'handle noprint' ..." start a new paragraph.

Yes, though perhaps a new paragraph is unnecessary.  See above.

> 
>> +@cindex stepping into signal handlers
>> +@anchor{stepping into signal handlers}
> 
> I would remove this @cindex entry: it doesn't add anything useful to
> the previous one, and will likely point to the same page.

I'd prefer to keep it, if you don't mind.  The queue-signal reference wants
to point here directly, and I can imagine the text above expanding in
directions not relevant for that cross reference.  I'd like to
have a place where I can point at when the topic of wanting to debug
a handler without knowing exactly which function that is comes up.

> 
>> +If the program was stopped for a signal (that is, stopped before the
>> +program sees it), due to @code{handle stop} being set, and
>> +@code{handle pass} is in effect for that signal too, and your program
>> +handles the signal, a stepping command such as for example
>> +@code{stepi} or @code{step} steps @emph{into} the signal's handler (if
>> +the target supports it).
> 
> This is a mouthful, not in the least because of excessive use of past
> tense.  How about this variant instead:
> 
>   If you set @code{handle stop} for a signal, @value{GDBN} stops your
>   program and announces the signal when it arrives, before the program
>   sees it.  

I think this part ends up being redundant with what is already
said further above, around:

"When a signal stops your program, the signal is not visible to the
program until you
continue.  Your program sees the signal then, if @code{pass} is in
effect for the signal in question @emph{at that time}. "

I'm now thinking that we can just remove that part, and use the rest of
your paragraph below:

>   If you also set @code{handle pass} for that signal, and
>   your program sets up a handler for it, then issuing a stepping
>   command, such as @code{step} or @code{stepi}, when your program is
>   stopped due to the signal will step @emph{into} the signal handler
>   (if the target supports that).

Sounds clear enough to me (with a minor tweak to make it stand on
its own).

> 
>> +Likewise, if the @code{queue-signal} command was used to queue a
>> +signal to be delivered to the current thread when execution of the
> 
> Please reword in active tens ("... if you use the @code{queue-signal}
> command to queue ...").
> 

Done.

>> +thread resumes (@pxref{Signaling, ,Giving your Program a Signal}),
>> +then a stepping command steps into the signal's handler.
> 
> Not sure I understand the sequence here.  First, I queue-signal, then
> the signal is delivered and the thread stops, and _then_ I issue si?
> I guess the "when execution of the thread resumes" confused me.

Sounds like you're thinking of "queue-signal" like "kill" from the shell,
but that's not how "queue-signal" works.  "queue-signal" instead
passes the signal to the program immediately as if the thread had
_already_ stopped for the signal.  GDB doesn't intercept
the signal anymore.

I've added a queue-signal example.  Hopefully that makes things clearer.

Let me know how this version looks.

From fd292ad2a1c10ec5a4f902535c7d3c29d9cbc1e1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 27 Oct 2014 19:27:10 +0000
Subject: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal
 arrives

I noticed that "si" behaves differently when a "handle nostop" signal
arrives while the step is in progress, depending on whether the
program was stopped at a breakpoint when "si" was entered.
Specifically, in case GDB needs to step off a breakpoint, the handler
is skipped and the program stops in the next "mainline" instruction.
Otherwise, the "si" stops in the first instruction of the signal
handler.

I was surprised the testsuite doesn't catch this difference.  Turns
out gdb.base/sigstep.exp covers a bunch of cases related to stepping
and signal handlers, but does not test stepi nor nexti, only
step/next/continue.

My first reaction was that stopping in the signal handler was the
correct thing to do, as it's where the next user-visible instruction
that is executed is.  I considered then "nexti" -- a signal handler
could be reasonably considered a subroutine call to step over, it'd
seem intuitive to me that "nexti" would skip it.

But then, I realized that signals that arrive while a plain/line
"step" is in progress _also_ have their handler skipped.  A user might
well be excused for being confused by this, given:

  (gdb) help step
  Step program until it reaches a different source line.

And the signal handler's sources will be in different source lines,
after all.

I think that having to explain that "stepi" steps into handlers, (and
that "nexti" wouldn't according to my reasoning above), while "step"
does not, is a sign of an awkward interface.

E.g., if a user truly is interested in stepping into signal handlers,
then it's odd that she has to either force the signal to "handle
stop", or recall to do "stepi" whenever such a signal might be
delivered.  For that use case, it'd seem nicer to me if "step" also
stepped into handlers.

This suggests to me that we either need a global "step-into-handlers"
setting, or perhaps better, make "handle pass/nopass stop/nostop
print/noprint" have have an additional axis - "handle
stepinto/nostepinto", so that the user could configure whether
handlers for specific signals should be stepped into.

In any case, I think it's simpler (and thus better) for all step
commands to behave the same.  This commit thus makes "si/ni" skip
handlers for "handle nostop" signals that arrive while the command was
already in progress, like step/next do.

To be clear, nothing changes if the program was stopped for a signal,
and the user enters a stepping command _then_ -- GDB still steps into
the handler.  The change concerns signals that don't cause a stop and
that arrive while the step is in progress.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_signal_stop): Also skip handlers when a random
	signal arrives while handling a "stepi" or a "nexti".  Set the
	thread's 'step_after_step_resume_breakpoint' flag.

gdb/doc/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Continuing and Stepping): Add cross reference to
	info on stepping and signal handlers.
	(Signals): Explain stepping and signal handlers.  Add context
	index entries, and cross references.

gdb/testsuite/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* gdb.base/sigstep.c (dummy): New global.
	(main): Issue a couple writes to the new global.
	* gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New
	procedures.
	(skip_over_handler): Use test_skip_handler.
	(top level): Call skip_over_handler for stepi and nexti too.
	(breakpoint_over_handler): Use test_skip_handler.
	(top level): Call breakpoint_over_handler for stepi and nexti too.
---
 gdb/doc/gdb.texinfo                | 68 +++++++++++++++++++++++++++++++++++++-
 gdb/infrun.c                       |  7 ++--
 gdb/testsuite/gdb.base/sigstep.c   |  7 ++--
 gdb/testsuite/gdb.base/sigstep.exp | 55 ++++++++++++++++++++++--------
 4 files changed, 118 insertions(+), 19 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a1b8ac7..9b32217 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5079,7 +5079,9 @@ line of source code, or one machine instruction (depending on what
 particular command you use).  Either when continuing or when stepping,
 your program may stop even sooner, due to a breakpoint or a signal.  (If
 it stops due to a signal, you may want to use @code{handle}, or use
-@samp{signal 0} to resume execution.  @xref{Signals, ,Signals}.)
+@samp{signal 0} to resume execution (@pxref{Signals, ,Signals}),
+or you may step into the signal's handler (@pxref{stepping and signal
+handlers}).)
 
 @table @code
 @kindex continue
@@ -5573,6 +5575,67 @@ a result of the fatal signal once it saw the signal.  To prevent this,
 you can continue with @samp{signal 0}.  @xref{Signaling, ,Giving your
 Program a Signal}.
 
+@cindex stepping and signal handlers
+@anchor{stepping and signal handlers}
+
+@value{GDBN} optimizes for stepping the mainline code.  If a signal
+that has @code{handle nostop} and @code{handle pass} set arrives while
+a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
+in progress, @value{GDBN} lets the signal handler run and then resumes
+stepping the mainline code once the signal handler returns.  In other
+words, @value{GDBN} steps over the signal handler.  This prevents
+signals that you've specified as not interesting (with @code{handle
+nostop}) from changing the focus of debugging unexpectedly.  Note that
+the signal handler itself may still hit a breakpoint, stop for another
+signal that has @code{handle stop} in effect, or for any other event
+that normally results in stopping the stepping command sooner.  Also
+note that @value{GDBN} still informs you that the program received a
+signal if @code{handle print} is set.
+
+@cindex stepping into signal handlers
+@anchor{stepping into signal handlers}
+
+If you set @code{handle pass} for a signal, and your program sets up a
+handler for it, then issuing a stepping command, such as @code{step}
+or @code{stepi}, when your program is stopped due to the signal will
+step @emph{into} the signal handler (if the target supports that).
+
+Likewise, if you use the @code{queue-signal} command to queue a signal
+to be delivered to the current thread when execution of the thread
+resumes (@pxref{Signaling, ,Giving your Program a Signal}), then a
+stepping command will step into the signal handler.
+
+Here's an example, using @code{stepi} to step to the first instruction
+of @code{SIGUSR1}'s handler:
+
+@smallexample
+(@value{GDBP}) handle SIGUSR1
+Signal        Stop      Print   Pass to program Description
+SIGUSR1       Yes       Yes     Yes             User defined signal 1
+(@value{GDBP}) c
+Continuing.
+
+Program received signal SIGUSR1, User defined signal 1.
+main () sigusr1.c:28
+28        p = 0;
+(@value{GDBP}) si
+sigusr1_handler () at sigusr1.c:9
+9       @{
+@end smallexample
+
+The same, but using @code{queue-signal} instead of waiting for the
+program to receive the signal first:
+
+@smallexample
+(@value{GDBP}) n
+28        p = 0;
+(@value{GDBP}) queue-signal SIGUSR1
+(@value{GDBP}) si
+sigusr1_handler () at sigusr1.c:9
+9       @{
+(@value{GDBP})
+@end smallexample
+
 @cindex extra signal information
 @anchor{extra signal information}
 
@@ -16654,6 +16717,9 @@ be used to pass a signal whose handling state has been set to @code{nopass}
 @end table
 @c @end group
 
+@xref{stepping into signal handlers}, for information on how stepping
+commands behave when the thread has a signal queued.
+
 @node Returning
 @section Returning from a Function
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 90a3123..df053e2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4463,9 +4463,9 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  return;
 	}
 
-      if (ecs->event_thread->control.step_range_end != 0
-	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
-	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
+      if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
+	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	      || ecs->event_thread->control.step_range_end == 1)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4485,6 +4485,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "single-step range\n");
 
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
+	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c
index aa2384a..cc69184 100644
--- a/gdb/testsuite/gdb.base/sigstep.c
+++ b/gdb/testsuite/gdb.base/sigstep.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 
 static volatile int done;
+static volatile int dummy;
 
 static void
 handler (int sig)
@@ -74,8 +75,10 @@ main ()
 	      return 1;
 	    }
 	}
-      /* Wait.  */
-      while (!done);
+      /* Wait.  Issue a couple writes to a dummy volatile var to be
+	 reasonably sure our simple "get-next-pc" logic doesn't
+	 stumble on branches.  */
+      dummy = 0; dummy = 0; while (!done);
       done = 0;
     }
   return 0;
diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index 184d46e..53152b8 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -269,9 +269,38 @@ proc skip_to_handler_entry { i } {
     gdb_test "clear *handler" ".*" "$prefix; clear handler"
 }
 
-skip_to_handler_entry step
-skip_to_handler_entry next
-skip_to_handler_entry continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    skip_to_handler_entry $cmd
+}
+
+# Get the address of where a single-step should land.
+
+proc get_next_pc {test} {
+    global gdb_prompt
+    global hex
+
+    set next ""
+    gdb_test_multiple "x/2i \$pc" $test {
+	-re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	    set next $expect_out(1,string)
+	    pass $test
+	}
+    }
+
+    return $next
+}
+
+# Test that the command skipped over the handler.
+
+proc test_skip_handler {prefix i} {
+    if {$i == "stepi" || $i == "nexti"} {
+	set next_pc [get_next_pc "$prefix; get next PC"]
+	gdb_test "$i" "dummy = 0.*" "$prefix; performing $i"
+	gdb_test "p /x \$pc" " = $next_pc" "$prefix; advanced"
+    } else {
+	gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    }
+}
 
 # Try stepping when there's a signal pending but no breakpoints.
 # Should skip the handler advancing to the next line.
@@ -295,13 +324,13 @@ proc skip_over_handler { i } {
 
     # Make the signal pending
     sleep 1
-    
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+
+    test_skip_handler $prefix $i
 }
 
-skip_over_handler step
-skip_over_handler next
-skip_over_handler continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    skip_over_handler $cmd
+}
 
 # Try stepping when there's a signal pending, a pre-existing
 # breakpoint at the current instruction, and a breakpoint in the
@@ -385,7 +414,7 @@ breakpoint_to_handler_entry continue
 
 # Try stepping when there's a signal pending, and a pre-existing
 # breakpoint at the current instruction, and no breakpoint in the
-# handler.  Should advance to the next line.
+# handler.  Should advance to the next line/instruction.
 
 proc breakpoint_over_handler { i } {
     global gdb_prompt
@@ -409,10 +438,10 @@ proc breakpoint_over_handler { i } {
     # Make the signal pending
     sleep 1
     
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    test_skip_handler $prefix $i
     gdb_test "clear $infinite_loop" ".*" "$prefix; clear infinite loop"
 }
 
-breakpoint_over_handler step
-breakpoint_over_handler next
-breakpoint_over_handler continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    breakpoint_over_handler $cmd
+}
-- 
1.9.3


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-27 19:44               ` Pedro Alves
@ 2014-10-27 19:58                 ` Eli Zaretskii
  2014-10-27 20:34                   ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2014-10-27 19:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Mon, 27 Oct 2014 19:44:22 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> So, perhaps this variant is clearer?
> 
> @value{GDBN} optimizes for stepping the mainline code.  If a signal
> that has @code{handle nostop} and @code{handle pass} set arrives while
> a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
> in progress, @value{GDBN} lets the signal handler run and then resumes
> stepping the mainline code once the signal handler returns.  In other
> words, @value{GDBN} steps over the signal handler.  This prevents
> signals that you've specified as not interesting (with @code{handle
> nostop}) from changing the focus of debugging unexpectedly.  Note that
> the signal handler itself may still hit a breakpoint, stop for another
> signal that has @code{handle stop} in effect, or for any other event
> that normally results in stopping the stepping command sooner.  Also
> note that @value{GDBN} still informs you that the program received a
> signal if @code{handle print} is set.

Yes, this is OK.

> >> +@cindex stepping into signal handlers
> >> +@anchor{stepping into signal handlers}
> > 
> > I would remove this @cindex entry: it doesn't add anything useful to
> > the previous one, and will likely point to the same page.
> 
> I'd prefer to keep it, if you don't mind.  The queue-signal reference wants
> to point here directly, and I can imagine the text above expanding in
> directions not relevant for that cross reference.

Both entries use the same words, so why do we need both?  There's one
paragraph of only a dozen of text lines between them.

> I'd like to have a place where I can point at when the topic of
> wanting to debug a handler without knowing exactly which function
> that is comes up.

For pointing, you have the anchor; I didn't say to delete that.  Index
entries cannot point.  In fact, some Info readers land you at the
beginning of the node, not at the place where the entry was in
Texinfo.

> I'm now thinking that we can just remove that part, and use the rest of
> your paragraph below:
> 
> >   If you also set @code{handle pass} for that signal, and
> >   your program sets up a handler for it, then issuing a stepping
> >   command, such as @code{step} or @code{stepi}, when your program is
> >   stopped due to the signal will step @emph{into} the signal handler
> >   (if the target supports that).

Fine with me.

> >> +thread resumes (@pxref{Signaling, ,Giving your Program a Signal}),
> >> +then a stepping command steps into the signal's handler.
> > 
> > Not sure I understand the sequence here.  First, I queue-signal, then
> > the signal is delivered and the thread stops, and _then_ I issue si?
> > I guess the "when execution of the thread resumes" confused me.
> 
> Sounds like you're thinking of "queue-signal" like "kill" from the shell,
> but that's not how "queue-signal" works.  "queue-signal" instead
> passes the signal to the program immediately as if the thread had
> _already_ stopped for the signal.  GDB doesn't intercept
> the signal anymore.

How is this different from what I wrote?  The program behaves as if
the signal was delivered to it, right?

> I've added a queue-signal example.  Hopefully that makes things clearer.

It does, thanks.

> Let me know how this version looks.

LGTM.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives
  2014-10-27 19:58                 ` Eli Zaretskii
@ 2014-10-27 20:34                   ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2014-10-27 20:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 10/27/2014 07:58 PM, Eli Zaretskii wrote:

>>>> +@cindex stepping into signal handlers
>>>> +@anchor{stepping into signal handlers}
>>>
>>> I would remove this @cindex entry: it doesn't add anything useful to
>>> the previous one, and will likely point to the same page.
>>
>> I'd prefer to keep it, if you don't mind.  The queue-signal reference wants
>> to point here directly, and I can imagine the text above expanding in
>> directions not relevant for that cross reference.
> 
> Both entries use the same words, so why do we need both?  There's one
> paragraph of only a dozen of text lines between them.

I was thinking that "stepping into signal handler" was a concept
on its own.  But I can certainly live without it; removed in the
final version.

>>>> +thread resumes (@pxref{Signaling, ,Giving your Program a Signal}),
>>>> +then a stepping command steps into the signal's handler.
>>>
>>> Not sure I understand the sequence here.  First, I queue-signal, then
>>> the signal is delivered and the thread stops, and _then_ I issue si?
>>> I guess the "when execution of the thread resumes" confused me.
>>
>> Sounds like you're thinking of "queue-signal" like "kill" from the shell,
>> but that's not how "queue-signal" works.  "queue-signal" instead
>> passes the signal to the program immediately as if the thread had
>> _already_ stopped for the signal.  GDB doesn't intercept
>> the signal anymore.
> 
> How is this different from what I wrote?  The program behaves as if
> the signal was delivered to it, right?

Yes, but the "then the signal is delivered and the thread stops" part
doesn't exist.  The thread is (and must be) already stopped.

>> I've added a queue-signal example.  Hopefully that makes things clearer.
> 
> It does, thanks.
> 
>> Let me know how this version looks.
> 
> LGTM.

Great, I've pushed it in then, as below.

Thanks!

From e5f8a7cc2d376c81749b6e4a4efc034201cf683c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 27 Oct 2014 20:24:59 +0000
Subject: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal
 arrives

I noticed that "si" behaves differently when a "handle nostop" signal
arrives while the step is in progress, depending on whether the
program was stopped at a breakpoint when "si" was entered.
Specifically, in case GDB needs to step off a breakpoint, the handler
is skipped and the program stops in the next "mainline" instruction.
Otherwise, the "si" stops in the first instruction of the signal
handler.

I was surprised the testsuite doesn't catch this difference.  Turns
out gdb.base/sigstep.exp covers a bunch of cases related to stepping
and signal handlers, but does not test stepi nor nexti, only
step/next/continue.

My first reaction was that stopping in the signal handler was the
correct thing to do, as it's where the next user-visible instruction
that is executed is.  I considered then "nexti" -- a signal handler
could be reasonably considered a subroutine call to step over, it'd
seem intuitive to me that "nexti" would skip it.

But then, I realized that signals that arrive while a plain/line
"step" is in progress _also_ have their handler skipped.  A user might
well be excused for being confused by this, given:

  (gdb) help step
  Step program until it reaches a different source line.

And the signal handler's sources will be in different source lines,
after all.

I think that having to explain that "stepi" steps into handlers, (and
that "nexti" wouldn't according to my reasoning above), while "step"
does not, is a sign of an awkward interface.

E.g., if a user truly is interested in stepping into signal handlers,
then it's odd that she has to either force the signal to "handle
stop", or recall to do "stepi" whenever such a signal might be
delivered.  For that use case, it'd seem nicer to me if "step" also
stepped into handlers.

This suggests to me that we either need a global "step-into-handlers"
setting, or perhaps better, make "handle pass/nopass stop/nostop
print/noprint" have have an additional axis - "handle
stepinto/nostepinto", so that the user could configure whether
handlers for specific signals should be stepped into.

In any case, I think it's simpler (and thus better) for all step
commands to behave the same.  This commit thus makes "si/ni" skip
handlers for "handle nostop" signals that arrive while the command was
already in progress, like step/next do.

To be clear, nothing changes if the program was stopped for a signal,
and the user enters a stepping command _then_ -- GDB still steps into
the handler.  The change concerns signals that don't cause a stop and
that arrive while the step is in progress.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_signal_stop): Also skip handlers when a random
	signal arrives while handling a "stepi" or a "nexti".  Set the
	thread's 'step_after_step_resume_breakpoint' flag.

gdb/doc/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Continuing and Stepping): Add cross reference to
	info on stepping and signal handlers.
	(Signals): Explain stepping and signal handlers.  Add context
	index entry, and cross references.

gdb/testsuite/
2014-10-27  Pedro Alves  <palves@redhat.com>

	* gdb.base/sigstep.c (dummy): New global.
	(main): Issue a couple writes to the new global.
	* gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New
	procedures.
	(skip_over_handler): Use test_skip_handler.
	(top level): Call skip_over_handler for stepi and nexti too.
	(breakpoint_over_handler): Use test_skip_handler.
	(top level): Call breakpoint_over_handler for stepi and nexti too.
---
 gdb/ChangeLog                      |  6 ++++
 gdb/doc/ChangeLog                  |  7 ++++
 gdb/testsuite/ChangeLog            | 11 +++++++
 gdb/doc/gdb.texinfo                | 67 +++++++++++++++++++++++++++++++++++++-
 gdb/infrun.c                       |  7 ++--
 gdb/testsuite/gdb.base/sigstep.c   |  7 ++--
 gdb/testsuite/gdb.base/sigstep.exp | 55 +++++++++++++++++++++++--------
 7 files changed, 141 insertions(+), 19 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 32f788c..8a34118 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-27  Pedro Alves  <palves@redhat.com>
+
+	* infrun.c (handle_signal_stop): Also skip handlers when a random
+	signal arrives while handling a "stepi" or a "nexti".  Set the
+	thread's 'step_after_step_resume_breakpoint' flag.
+
 2014-10-27  Luis Machado  <lgustavo@codesourcery.com>

 	* arm-tdep.c (INSN_S_L_BIT_NUM): Document.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index f1b2329..f60fd8f 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2014-10-27  Pedro Alves  <palves@redhat.com>
+
+	* gdb.texinfo (Continuing and Stepping): Add cross reference to
+	info on stepping and signal handlers.
+	(Signals): Explain stepping and signal handlers.  Add context
+	index entry, and cross references.
+
 2014-10-20  Simon Marchi  <simon.marchi@ericsson.com>

 	* python.texi (Breakpoints In Python): Add parenthesis after
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2eea791..5cc7b0b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2014-10-27  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/sigstep.c (dummy): New global.
+	(main): Issue a couple writes to the new global.
+	* gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New
+	procedures.
+	(skip_over_handler): Use test_skip_handler.
+	(top level): Call skip_over_handler for stepi and nexti too.
+	(breakpoint_over_handler): Use test_skip_handler.
+	(top level): Call breakpoint_over_handler for stepi and nexti too.
+
 2014-10-27  Yao Qi  <yao@codesourcery.com>

 	* gdb.trace/tfile.c (adjust_function_address)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a1b8ac7..15c2908 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5079,7 +5079,9 @@ line of source code, or one machine instruction (depending on what
 particular command you use).  Either when continuing or when stepping,
 your program may stop even sooner, due to a breakpoint or a signal.  (If
 it stops due to a signal, you may want to use @code{handle}, or use
-@samp{signal 0} to resume execution.  @xref{Signals, ,Signals}.)
+@samp{signal 0} to resume execution (@pxref{Signals, ,Signals}),
+or you may step into the signal's handler (@pxref{stepping and signal
+handlers}).)

 @table @code
 @kindex continue
@@ -5573,6 +5575,66 @@ a result of the fatal signal once it saw the signal.  To prevent this,
 you can continue with @samp{signal 0}.  @xref{Signaling, ,Giving your
 Program a Signal}.

+@cindex stepping and signal handlers
+@anchor{stepping and signal handlers}
+
+@value{GDBN} optimizes for stepping the mainline code.  If a signal
+that has @code{handle nostop} and @code{handle pass} set arrives while
+a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is
+in progress, @value{GDBN} lets the signal handler run and then resumes
+stepping the mainline code once the signal handler returns.  In other
+words, @value{GDBN} steps over the signal handler.  This prevents
+signals that you've specified as not interesting (with @code{handle
+nostop}) from changing the focus of debugging unexpectedly.  Note that
+the signal handler itself may still hit a breakpoint, stop for another
+signal that has @code{handle stop} in effect, or for any other event
+that normally results in stopping the stepping command sooner.  Also
+note that @value{GDBN} still informs you that the program received a
+signal if @code{handle print} is set.
+
+@anchor{stepping into signal handlers}
+
+If you set @code{handle pass} for a signal, and your program sets up a
+handler for it, then issuing a stepping command, such as @code{step}
+or @code{stepi}, when your program is stopped due to the signal will
+step @emph{into} the signal handler (if the target supports that).
+
+Likewise, if you use the @code{queue-signal} command to queue a signal
+to be delivered to the current thread when execution of the thread
+resumes (@pxref{Signaling, ,Giving your Program a Signal}), then a
+stepping command will step into the signal handler.
+
+Here's an example, using @code{stepi} to step to the first instruction
+of @code{SIGUSR1}'s handler:
+
+@smallexample
+(@value{GDBP}) handle SIGUSR1
+Signal        Stop      Print   Pass to program Description
+SIGUSR1       Yes       Yes     Yes             User defined signal 1
+(@value{GDBP}) c
+Continuing.
+
+Program received signal SIGUSR1, User defined signal 1.
+main () sigusr1.c:28
+28        p = 0;
+(@value{GDBP}) si
+sigusr1_handler () at sigusr1.c:9
+9       @{
+@end smallexample
+
+The same, but using @code{queue-signal} instead of waiting for the
+program to receive the signal first:
+
+@smallexample
+(@value{GDBP}) n
+28        p = 0;
+(@value{GDBP}) queue-signal SIGUSR1
+(@value{GDBP}) si
+sigusr1_handler () at sigusr1.c:9
+9       @{
+(@value{GDBP})
+@end smallexample
+
 @cindex extra signal information
 @anchor{extra signal information}

@@ -16654,6 +16716,9 @@ be used to pass a signal whose handling state has been set to @code{nopass}
 @end table
 @c @end group

+@xref{stepping into signal handlers}, for information on how stepping
+commands behave when the thread has a signal queued.
+
 @node Returning
 @section Returning from a Function

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 90a3123..df053e2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4463,9 +4463,9 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  return;
 	}

-      if (ecs->event_thread->control.step_range_end != 0
-	  && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
-	  && pc_in_thread_step_range (stop_pc, ecs->event_thread)
+      if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
+	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	      || ecs->event_thread->control.step_range_end == 1)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4485,6 +4485,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "single-step range\n");

 	  insert_hp_step_resume_breakpoint_at_frame (frame);
+	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c
index aa2384a..cc69184 100644
--- a/gdb/testsuite/gdb.base/sigstep.c
+++ b/gdb/testsuite/gdb.base/sigstep.c
@@ -24,6 +24,7 @@
 #include <errno.h>

 static volatile int done;
+static volatile int dummy;

 static void
 handler (int sig)
@@ -74,8 +75,10 @@ main ()
 	      return 1;
 	    }
 	}
-      /* Wait.  */
-      while (!done);
+      /* Wait.  Issue a couple writes to a dummy volatile var to be
+	 reasonably sure our simple "get-next-pc" logic doesn't
+	 stumble on branches.  */
+      dummy = 0; dummy = 0; while (!done);
       done = 0;
     }
   return 0;
diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index 184d46e..53152b8 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -269,9 +269,38 @@ proc skip_to_handler_entry { i } {
     gdb_test "clear *handler" ".*" "$prefix; clear handler"
 }

-skip_to_handler_entry step
-skip_to_handler_entry next
-skip_to_handler_entry continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    skip_to_handler_entry $cmd
+}
+
+# Get the address of where a single-step should land.
+
+proc get_next_pc {test} {
+    global gdb_prompt
+    global hex
+
+    set next ""
+    gdb_test_multiple "x/2i \$pc" $test {
+	-re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	    set next $expect_out(1,string)
+	    pass $test
+	}
+    }
+
+    return $next
+}
+
+# Test that the command skipped over the handler.
+
+proc test_skip_handler {prefix i} {
+    if {$i == "stepi" || $i == "nexti"} {
+	set next_pc [get_next_pc "$prefix; get next PC"]
+	gdb_test "$i" "dummy = 0.*" "$prefix; performing $i"
+	gdb_test "p /x \$pc" " = $next_pc" "$prefix; advanced"
+    } else {
+	gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    }
+}

 # Try stepping when there's a signal pending but no breakpoints.
 # Should skip the handler advancing to the next line.
@@ -295,13 +324,13 @@ proc skip_over_handler { i } {

     # Make the signal pending
     sleep 1
-
-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+
+    test_skip_handler $prefix $i
 }

-skip_over_handler step
-skip_over_handler next
-skip_over_handler continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    skip_over_handler $cmd
+}

 # Try stepping when there's a signal pending, a pre-existing
 # breakpoint at the current instruction, and a breakpoint in the
@@ -385,7 +414,7 @@ breakpoint_to_handler_entry continue

 # Try stepping when there's a signal pending, and a pre-existing
 # breakpoint at the current instruction, and no breakpoint in the
-# handler.  Should advance to the next line.
+# handler.  Should advance to the next line/instruction.

 proc breakpoint_over_handler { i } {
     global gdb_prompt
@@ -409,10 +438,10 @@ proc breakpoint_over_handler { i } {
     # Make the signal pending
     sleep 1

-    gdb_test "$i" "done = 0.*" "$prefix; performing $i"
+    test_skip_handler $prefix $i
     gdb_test "clear $infinite_loop" ".*" "$prefix; clear infinite loop"
 }

-breakpoint_over_handler step
-breakpoint_over_handler next
-breakpoint_over_handler continue
+foreach cmd {"stepi" "nexti" "step" "next" "continue"} {
+    breakpoint_over_handler $cmd
+}
-- 
1.9.3


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-10-27 20:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-14 17:48 [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives Pedro Alves
2014-10-14 18:27 ` Eli Zaretskii
2014-10-14 18:49   ` Pedro Alves
2014-10-14 18:56     ` Pedro Alves
2014-10-14 19:22     ` Eli Zaretskii
2014-10-15 13:40       ` Pedro Alves
2014-10-15 14:31         ` Eli Zaretskii
2014-10-25 14:00           ` Pedro Alves
2014-10-27 17:39             ` Eli Zaretskii
2014-10-27 19:44               ` Pedro Alves
2014-10-27 19:58                 ` Eli Zaretskii
2014-10-27 20:34                   ` Pedro Alves
2014-10-15 11:12 ` Yao Qi
2014-10-15 13:44   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).