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

[Pedro Alves <pedro@codesourcery.com>]

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