[patch] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken

Ulrich Weigand
2011-04-28T08:55:05Z

Pedro Alves wrote:
> On Wednesday 27 April 2011 20:11:53, Ulrich Weigand wrote:
> > The simple way to fix the inconsistency would be to just add a
> > step = 0;
> > line to the above if. Of course this changes the behaviour for
> > hardware single-step platforms: they now would not step into
> > the signal handler in this case (just like software single-step
> > platforms don't).
> >
> > On the one hand, this adds consistency: both types of platforms
> > behave the same.
>
> IIUC, with that change alone, the behavior on hw-step archs
> would change depending on there being a breakpoint at PC or
> not. Without a breakpoint, you'd step into the handler, with
> a breakpoint, you'd not.

Good point. That does not seem desirable.

> I'd prefer to keep the possibility to step into a handler.
> I find this very useful when you don't actually know what
> handler is installed. Much easier than grepping for "signal" and
> trying to guess what is installed (which may even have been done
> by some external dependency / library).

OK. The patch below should restore the prior behavior on
hardware single-step targets, and strictly improve the situation
on software single-step targets (even though of course it still
is not quite the same as with hardware single-step). I've also
attempted to better document the situation in a comment ...

Does this seem OK to you?

Bye,
Ulrich

ChangeLog:

* infrun.c (proceed): Revert previous change.
(resume): Instead, handle the case of signal delivery while stepping
off a breakpoint location here, and only if software single-stepping
is used.

Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.476
diff -u -p -r1.476 infrun.c
--- gdb/infrun.c 27 Apr 2011 17:08:41 -0000 1.476
+++ gdb/infrun.c 28 Apr 2011 08:29:36 -0000
@@ -1703,6 +1703,42 @@ a command like `return' or `jump' to con
else if (step)
step = maybe_software_singlestep (gdbarch, pc);

+ /* Currently, our software single-step implementation leads to different
+ results than hardware single-stepping in one situation: when stepping
+ into delivering a signal which has an associated signal handler,
+ hardware single-step will stop at the first instruction of the handler,
+ while software single-step will simply skip execution of the handler.
+
+ For now, this difference in behavior is accepted since there is no
+ easy way to actually implement single-stepping into a signal handler
+ without kernel support.
+
+ However, there is one scenario where this difference leads to follow-on
+ problems: if we're stepping off a breakpoint by removing all breakpoints
+ and then single-stepping. In this case, the software single-step
+ behavior means that even if there is a *breakpoint* in the signal
+ handler, GDB still would not stop.
+
+ Fortunately, we can at least fix this particular issue. We detect
+ here the case where we are about to deliver a signal while software
+ single-stepping with breakpoints removed. In this situation, we
+ revert the decisions to remove all breakpoints and insert single-
+ step breakpoints, and instead we install a step-resume breakpoint
+ at the current address, deliver the signal without stepping, and
+ once we arrive back at the step-resume breakpoint, actually step
+ over the breakpoint we originally wanted to step over. */
+ if (singlestep_breakpoints_inserted_p
+ && tp->control.trap_expected && sig != TARGET_SIGNAL_0)
+ {
+ remove_single_step_breakpoints ();
+ singlestep_breakpoints_inserted_p = 0;
+ tp->control.trap_expected = 0;
+
+ insert_step_resume_breakpoint_at_frame (get_current_frame ());
+ insert_breakpoints ();
+ tp->step_after_step_resume_breakpoint = 1;
+ }
+
if (should_resume)
{
ptid_t resume_ptid;
@@ -2064,6 +2100,24 @@ proceed (CORE_ADDR addr, enum target_sig
/* prepare_to_proceed may change the current thread. */
tp = inferior_thread ();

+ if (oneproc)
+ {
+ tp->control.trap_expected = 1;
+ /* If displaced stepping is enabled, we can step over the
+ breakpoint without hitting it, so leave all breakpoints
+ inserted. Otherwise we need to disable all breakpoints, step
+ one instruction, and then re-add them when that step is
+ finished. */
+ if (!use_displaced_stepping (gdbarch))
+ remove_breakpoints ();
+ }
+
+ /* We can insert breakpoints if we're not trying to step over one,
+ or if we are stepping over one but we're using displaced stepping
+ to do so. */
+ if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
+ insert_breakpoints ();
+
if (!non_stop)
{
/* Pass the last stop signal to the thread we're resuming,
@@ -2133,42 +2187,6 @@ proceed (CORE_ADDR addr, enum target_sig
/* Reset to normal state. */
init_infwait_state ();

- /* Stepping over a breakpoint while at the same time delivering a signal
- has a problem: we cannot use displaced stepping, but we also cannot
- use software single-stepping, because we do not know where execution
- will continue if a signal handler is installed.
-
- On the other hand, if there is a signal handler we'd have to step
- over it anyway. So what we do instead is to install a step-resume
- handler at the current address right away, deliver the signal without
- stepping, and once we arrive back at the step-resume breakpoint, step
- once more over the original breakpoint we wanted to step over. */
- if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
- && execution_direction != EXEC_REVERSE)
- {
- insert_step_resume_breakpoint_at_frame (get_current_frame ());
- tp->step_after_step_resume_breakpoint = 1;
- oneproc = 0;
- }
-
- if (oneproc)
- {
- tp->control.trap_expected = 1;
- /* If displaced stepping is enabled, we can step over the
- breakpoint without hitting it, so leave all breakpoints
- inserted. Otherwise we need to disable all breakpoints, step
- one instruction, and then re-add them when that step is
- finished. */
- if (!use_displaced_stepping (gdbarch))
- remove_breakpoints ();
- }
-
- /* We can insert breakpoints if we're not trying to step over one,
- or if we are stepping over one but we're using displaced stepping
- to do so. */
- if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
- insert_breakpoints ();
-
/* Resume inferior. */
resume (oneproc || step || bpstat_should_step (), tp->suspend.stop_signal);