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, 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(-)
  

> 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.
  

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
  

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
  

> 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.
  

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)
  

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
  

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
  

> 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.