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

Pedro Alves
2011-04-28T12:01:24Z

On Thursday 28 April 2011 09:54:39, Ulrich Weigand wrote:
> 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;
> + }
> +

(I wish there was no need to undo stuff, and do the decision
before actually inserting sss breakpoints, but I see why you
did it. We need to call gdbarch_software_single_step to
know whether sss breakpoints will be inserted, which, actually
inserts them.)

I'm trying to think whether this works okay with nested
signals. Consider that you had a signal to deliver while
stepping over a breakpoint, and you hit that new code.
Now, the signal handler runs with breakpoint inserted, and
happens to hit a breakpoint that shouldn't cause a stop and
so needs stepping over immediately (e.g., "b foo if 0"). This sets
stepping_over_breakpoint=1, and keeps going. Say an asynchronous pass/nostop
signal happens while trying to step over that breakpoint, and we
see it before the "step" finishes. While handling it, we end up in keep_going,
here, because we already had a step-resume breakpoint set, I think:

/* Note: step_resume_breakpoint may be non-NULL. This occures
when either there's a nested signal, or when there's a
pending signal enabled just as the signal handler returns
(leaving the inferior at the step-resume-breakpoint without
actually executing it). Either way continue until the
breakpoint is really hit. */
keep_going (ecs);
return;
}

eventually re-reaching resume with trap_expected set, sss breakpoints
inserted, and with a signal to deliver.
Since you'll already have one step-resume breakpoint set (back in the
main code), and you can't set another -- you'd hit the
assert in insert_step_resume_breakpoint_at_sal trying to insert
a second one.

But I'm not certain I'm guessing the flow correctly on software-step
archs.

Here's a small test that nests SIGSEGV handlers. Setting a breakpoint
at the *(int *)p lines like I mentioned above should do the trick.

#include <string.h>
#include <signal.h>
#include <unistd.h>

static void *p;

static void
handler (int sig, siginfo_t *info, void *context)
{
/* Trigger a nested SIGSEGV. */
*(int *)p = 0;

_exit (0);
}

int
main (void)
{
/* Set up the signal handler. */
struct sigaction action;
memset (&action, 0, sizeof (action));
action.sa_sigaction = handler;
action.sa_flags |= SA_SIGINFO | SA_NODEFER;
if (sigaction (SIGSEGV, &action, NULL))
{
perror ("sigaction");
return 1;
}

/* Trigger SIGSEGV. */
*(int *)p = 0;
return 0;
}

--
Pedro Alves