kernel-2.6.26.6-49.fc8.x86_64 also has this problem.

Looks like a bug. Created "signal-loss" testcase and added it to

http://sourceware.org/systemtap/wiki/utrace/tests

Futexes are not essential for this bug, this:

  if (child == 0)
    while (1)
      pause ();

will also work.

The bug here is that when we enter pause(), kernel doesn't notice that there is a pending SIGINT, and it never returns from the syscall, as if signal is not there. Vanilla (non-utrace) kernels return right away, report syscall exit (SIGTRAP | 0x80) and then report SIGINT being delivered to the tracee.

Will attach testcase now. Build it with "gcc -Os -DTALKATIVE signal-loss.c".

Created attachment 323908 [details]
signal-loss.c testcase

IMO the testcase is racy.

You call there PTRACE_SYSCALL without waiting till the inferior stops.  At that moment you also asynchronously do kill(2) there.  kill(2) may come arbitrarily before or after the inferior stops.

In the case you kill(2) it after the inferior stopped due to PTRACE_SYSCALL two different signals are generated at that moment (SIGINT and SIGTRAP|0x80).  You should pick up both signals by waitpid(2) otherwise one is left pending and it all gets tricky.

Not sure how the testcase should be fixed as it is racy and after removing the race there is not much to test.

Tried to finally write down some notes on the ptrace(2) functionality based on the infrormation provided by Roland McGrath:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/PTRACE?cvsroot=systemtap
It is only a draft, there may be mistakes and it definitely should address more parts of the ptrace(2) functionality, please point out if anything is missing there.

> You call there PTRACE_SYSCALL without waiting till the inferior stops.

I don't think so. As far as I can see, in the testcase ptrace(PTRACE_SYSCALL) is always called after waitpid().

> At that moment you also asynchronously do kill(2) there.  kill(2) may come arbitrarily before or after the inferior stops.

True. This is done deliberately. When you strace a program and it is being signaled by another process, you certainly can't control _when_ it will be signaled (when it is stopped or not).

> In the case you kill(2) it after the inferior stopped due to PTRACE_SYSCALL two
different signals are generated at that moment (SIGINT and SIGTRAP|0x80).  You
should pick up both signals by waitpid(2) otherwise one is left pending and it
all gets tricky.

Yes. And testcase does exactly that. It waitpid's and collects SIGINTs until it finally sees SIGTRAP|0x80. As you correctly point out, it's not determinable how many SIGINTs we will get before we see SIGTRAP|0x80. I saw from none to ~30.

What testcase is testing for is: as soon as we got SIGTRAP|0x80, the very next stop should be SIGTRAP|0x80 too, and next one should be SIGINT.

On buggy kernels, second waitpid (which has to return second SIGTRAP|0x80) does not return, it blocks.

Created attachment 324123 [details]
Testcase without looping.

Thanks for the explanation, understood now.

Proposing this testcase instead - it does not need the loop there and it does not have any undeterministic behavior (ptrace vs. kill races, despite controlled in the testcases before).

I verified that it still fails on Fedora.
But what you do with SA_RESTART needs better explanation. It took me circa 10 minutes to grok it.

  /* Send signal. It will not kill the child because it is being traced,
   * and all signals are reported to tracer first. We never pass back
   * the signal through PTRACE_SYSCALL, thus it never reaches the child.
>   * However, it must make child interrupt the pause() and report SIGINT
>   * to us.
^^^^^^^ delete this sentence, it's superfluous now
   * Still it should immediately report back to us SIGINT as would
   * happen if we would not be inside the pause(2) syscall by the first
   * PTRACE_SYSCALL above.
   */
  rc = kill (child, SIGINT);
  assert (rc == 0);



  /* Prepare an expected one-second timeout as SIGINT should get just enqueued
     when inside a syscall by PTRACE_SYSCALL.  We must clear SA_RESTARt to
     abort waitpid(2) by the SIGALRM handler invocation.  */

I did not understand the above even though I read it several times. I propose something like:

  /* At this point, waitpid should block, because tracee is stopped.
   * Sending signals to it should not wake it up.
   * Check that this is true. (We install NOP handler for SIGALRM
   * and make sure syscall restart is disabled (SA_RESTART is off).
   * waitpid will block for 1 second and SIGALRM will EINTR it). */

  alarm (1);
  memset (&act, 0, sizeof act);
  act.sa_handler = handler_nop;
  /* ACT.SA_FLAGS must have SA_RESTARt unset.  */
  rc = sigemptyset (&act.sa_mask);           <<-- superfluous
  assert (rc == 0);                          <<--
  rc = sigaction (SIGALRM, &act, &oldact);
  assert (rc == 0);
  assert (oldact.sa_handler == handler_fail);



More readable alternative is to setup SIGALRM handler with !SA_RESTART in main (to avoid clutter in testcase code), have this in the handler:

static volatile int timeout_is_a_known_bug;
static volatile int timeout_is_ok;

static void
handler_fail (int signo)
{
  if (signo == SIGALRM) {
    if (timeout_is_ok) return;
    if (timeout_is_a_known_bug) exit (1);
  }
...

Then you can replace entire sigaction dance with just 

  alarm (1)
  timeout_is_ok = 1;
  ...
  ...
  timeout_is_ok = 0;

(In reply to comment #7)
>   memset (&act, 0, sizeof act);
>   act.sa_handler = handler_nop;
>   /* ACT.SA_FLAGS must have SA_RESTARt unset.  */
>   rc = sigemptyset (&act.sa_mask);           <<-- superfluous
>   assert (rc == 0);                          <<--

This is not superfluous as it would violate POSIX if it would be missing there:
http://www.opengroup.org/onlinepubs/009695399/functions/sigemptyset.html
# For such reasons, either sigemptyset() or sigfillset() must be called prior
# to any other use of the signal set, even if such use is read-only

Valid sigset_t may require to have some size field set etc., it is just
a coincidence the current GNU/Linux sigemptyset() implementation just clears it.


The other comment/code changes I find OK.  Also the Comment 5 in the code is obsolete now.  I primarily wanted to remove the PTRACE_SYSCALL vs. kill(2) race there (no matter if it had an effect on the actual result or not).

Created attachment 324191 [details]
Testcase without looping #2

I updated testcase in the utrace_tests cvs. Thanks Jan.

Damn if I know what utrace does, just a wild guess...

The problem is, if we send SIGINT to TASK_TRACED task, the
signal is queued but TIF_SIGPENDING is not set (this is OK).

Then, we are waking the tracee by PTRACE_SYSCALL, it completes
the report_syscall logic, and re-enters sys_pause due to
-ERESTARTNOHAND. And this is not OK: it sleeps in TASK_INTERRUPTIBLE
with SIGINT pending and without signal_pending().

The subsequent kill(INT) can't help due to legacy_queue() check
in send_signal() (we can't queue INT twice).

int main(void)
{
        long res;
        int status;

        pid_t pid = fork();
        if (!pid) {
                pause();
                exit(0);
        }

        res = ptrace(PTRACE_ATTACH, pid, NULL, NULL);
        assert(res == 0);

        while (1) {
                res = waitpid(-1, &status, __WALL);
                assert(WIFSTOPPED(status));
                printf("waitpid: %ld -> %x %d\n", res, status, WSTOPSIG(status));

                res = ptrace(PTRACE_SYSCALL, pid, NULL, NULL);
                assert(res == 0);

                sleep(1);
                kill(pid, SIGINT);
        }
}

this program hangs after it recieves the SIGTRAP notification,
output:

   waitpid: 5137 -> 137f 19
   waitpid: 5137 -> 57f 5

I think this confirms the theory.

I'll send the patch in a minute. utrace_report_syscall_entry()
has a lenghty comment (which I can't parse), but the code
looks buggy: afaics we should recalc_sigpending() in any case.

Created attachment 325158 [details]
[PATCH] fix utrace_report_syscall_entry()->recalc_sigpending() logic

This message is a reminder that Fedora 9 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 9.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '9'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 9's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 9 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Fedora 9 changed to end-of-life (EOL) status on 2009-07-10. Fedora 9 is 
no longer maintained, which means that it will not receive any further 
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of 
Fedora please feel free to reopen this bug against that version.

Thank you for reporting this bug and we are sorry it could not be fixed.