a
Do not respawn signals, take 2.

Long story short, a test I wrote for something else was failing
occasionally.  Eventually, after a long, LONG, session of head
banging, I traced it to this bit in glibc (nptl/nptl-init.c):

/* For asynchronous cancellation we use a signal.  This is the handler.  */
static void
sigcancel_handler (int sig, siginfo_t *si, void *ctx)
{
...
  /* Safety check.  It would be possible to call this function for
     other signals and send a signal from another process.  This is not
     correct and might even be a security problem.  Try to catch as
     many incorrect invocations as possible.  */
  if (sig != SIGCANCEL
#ifdef __ASSUME_CORRECT_SI_PID
      /* Kernels before 2.5.75 stored the thread ID and not the process
	 ID in si_pid so we skip this test.  */
      || si->si_pid != pid
#endif
      || si->si_code != SI_TKILL)
    return;


The "si->si_pid != pid" check is what triggered the issue.  In that
new test, a thread happens to cancel another with a SIGCANCEL (glibc
internals) as GDB is trying to stop all threads.  GDB catches the
SIGCANCEL in stop_wait_callback, sets it aside, and keeps fetching
signals until it sees the SIGSTOP.  Later, when GDB finaly re-delivers
the SIGCANCEL to the inferior, the signal's si_pid is set to GDB's pid
instead of the inferior's.

We had discussed this back in 2010, around:

http://sourceware.org/ml/gdb-patches/2010-09/msg00360.html

I took that patch, and noticed that the new sigstep-threads.exp test
failed on gdbserver, because it didn't have that new "fetch SIGTRAP if
thread was stepping" bit in the 2010 patch.

Looking further at the "[rfc] linux-nat: Never PTRACE_CONT a stepping
thread" follow up to that series,

http://sourceware.org/ml/gdb-patches/2010-09/msg00362.html

that can't work as is for software single-step targets, as on those we
can't just set "lwp->step" to make an lwp software single-step.

So I took a step back and re-analyzed the issue.

The main issue is that when infrun.c sees a spurious signal that is in
"pass" state, it needs to re-forward that signal back to the inferior.
This is the end of the

  /* For the program's own signals, act according to
     the signal handling tables.  */

  if (ecs->random_signal)

block.  And it does that with a keep_going call.  If scheduler-locking
is not in effect, that delivers the signal to the inferior, but, also
continues all other threads, even if one of them should be stepped
instead, because we were "step"ing or "next"ing, etc. some other
thread.

(This could be seen as a limitation of the target_resume interface.
There's no way to step one thread, deliver a signal to another, and
resume all others with a single call.  We can't issue multiple
target_resume calls without an intervening target_wait, due to how the
regular remote protocol is designed.  The remote target, with vCont,
does support this though, allowing multiple actions in the same
packet.)

So let's tackly the lost step issue first.  This change:

 diff --git a/gdb/infrun.c b/gdb/infrun.c
 index 53db335..3e8dbc8 100644
 --- a/gdb/infrun.c
 +++ b/gdb/infrun.c
 @@ -4363,10 +4363,8 @@ process_event_stop_test:
  	 (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;
      }
 -
 +  else
    /* Handle cases caused by hitting a breakpoint.  */
    {

(I'll reindent this before checking in)

means that instead of calling keep_going immediately when we have a
spurious signal to forward to the inferior, we reach the

>   /* In all-stop mode, if we're currently stepping but have stopped in
>   some other thread, we need to switch back to the stepped thread.  */
>  if (!non_stop)

part, which makes sure we don't PTRACE_CONT a thread that was meant to
be stepping, and do all the necessary handling of software-single
step.  All good, we unify the "spurious signal", and "breakpoint that
doesn't cause stop" cases.

However, if we do just that, then we lose the signal that we were
meant to pass back to the inferior, when ultimately we call keep_going
with the stepping thread selected as current.  The way to address this
that is not a complete overhaul is to make the target backend
responsible for forwarding a thread's last stop signal back to the
thread, unless it has been cleared or is in "no pass" state.  This
isn't actually that alien.  It is actually quite natural, as it is
largelly how 'proceed/keep_going' track the signal to be forwarded as
well.  (I started out with a new field in struct thread_info, until I
realized this.)

This way, the extra SIGTRAP handling is not necessary, and gdbserver
doesn't need to change either - only remote.c is taught to send more
actions in the vCont (which is valid, and defined) packet.  If gdb's
target_resume interface is ever made more like gdbserver's, then those
bits that handle stop_signal in both linux-nat.c and remote.c could be
factored out to common code.

The tests are pretty much unmodified.  I only did some cosmetic fixes,
and moved the "signal" call to "main" in sigstep-threads.c, as signal
handlers are per process.

No regressions on x86_64 Fedora 16, native and gdbserver.  New tests
pass.  Fixes my unsubmitted test as well.

Comments?

gdb/
2012-06-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* gdbthread.h (ALL_THREADS): New macro.
	(thread_list): Declare.
	* infrun.c (handle_inferior_event) <spurious signal>: Don't keep
	going, but instead fall through to the stepping handling.
	* linux-nat.c (resume_lwp): New parameter 'signo'.  Resume with
	the passed in signal.  Adjust debug output.
	(resume_callback): Rename to ...
	(linux_nat_resume_callback): ... this.  Pass the thread's last
	stop signal, if in "pass" state.
	(linux_nat_resume): Adjust to rename.
	(stop_wait_callback): New assertion.  Don't respawn signals;
	instead let the LWP remain with SIGNALLED set.
	(linux_nat_wait_1): Remove flushing of pending SIGSTOPs.

gdb/testsuite/
2012-06-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.threads/siginfo-threads.exp: New file.
	* gdb.threads/siginfo-threads.c: New file.
	* gdb.threads/sigstep-threads.exp: New file.
	* gdb.threads/sigstep-threads.c: New file.
a