mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: oleg@redhat.com, vda.linux@googlemail.com,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, indan@nul.nu
Subject: Re: PTRACE_SEIZE should not stop  [Re: [PATCH 02/11] ptrace: implement PTRACE_SEIZE]
Date: Sun, 15 May 2011 21:48:29 +0200	[thread overview]
Message-ID: <20110515194829.GA27023@host1.jankratochvil.net> (raw)
In-Reply-To: <20110515172505.GL23665@htj.dyndns.org>

Hi Tejun,

On Sun, 15 May 2011 19:25:05 +0200, Tejun Heo wrote:
> On Sun, May 15, 2011 at 07:15:12PM +0200, Jan Kratochvil wrote:
> > On Sun, 15 May 2011 18:26:30 +0200, Tejun Heo wrote:
> > > the code to SEIZE and establish initial state would be simple.
> > 
> > In normal case yes; but one needs to handle all the corner cases when the
> > first signal is not INTERRUPT; which one usually does not handle as during
> > development (=in normal cases) it is always INTERRUPT.
[...]
> Maybe this is best solved with a test case which can reliably trigger
> different initial traps sites?

How to trigger it reliably?  One can just try it in a loop but it takes minutes
and depends on hardware specifics making it I guess even unreproducible in
various configurations.  We were allocating various machines for hours in the
farm but it may be unreproducible anyway.

http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/attach-into-signal.c?cvsroot=systemtap
  // Even DEFAULT_LOOPS of 400 is not enough to catch it reliably.
  // With TESTTIME=60 or more it should be close to 100%,
  // but takes long time (~10 minutes).
http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/attach-sigcont-wait.c?cvsroot=systemtap
/* Failure occurs either immediately or in about 20 runs.
   But sometimes not.  */
etc.


> > > You can tell them apart from userland and it doesn't matter which order or
> > > how many times INTERRUPT occurs.
> > 
> > I must know in which order they come to know when the tracee is still stopped
> > and I collect the signals to be displayed to the user and at which moment
> > there are no more signals in the queue and I start waiting on the debuggee
> > which started running.
> > 
> > Otherwise I can workaround it by various waitpid(NOHANG)s but it is better if
> > the ordering and when INTERRUPT is / is not reported is well defined.
> 
> Hmmm... you should be able to tell that without resorting to WNOHANG
> or depending on order of traps.  That's the goal anyway.  I'm a bit
> confused tho.  What do you mean by "the tracee is still stopped"?
> Tracee is always stopped (or rather trapped) after reporting a trap.

When debugging races in multithreaded applications a thread may get multiple
signals at once.  GDB in the default all-stop mode (the other is non-stop mode)
stops all the other threads when it sees the first event on some thread.

#include <pthread.h>
#include <assert.h>
#include <asm/unistd.h>
#include <unistd.h>
#include <signal.h>
#define tkill(tid, sig) syscall (__NR_tkill, (tid), (sig))
#define gettid() syscall (__NR_gettid)
static volatile pid_t tid;
static void *
start (void *arg)
{
  int i = (intptr_t) arg;
  while (!tid);
  sleep (1);
  tkill (tid, i & 1 ? SIGUSR1 : SIGUSR2);
  pause ();
  return arg;
}
int main (void)
{
  pthread_t thread;
  int i;
  for (i = 0; i < 10; i++)
    pthread_create (&thread, NULL, start, (void *) (intptr_t) i);
  tid = gettid ();	/* line 25 */
  sleep (1);
  return pause ();
}

gdb -nx ./threadsigs -ex 'tb 25' -ex r -ex 'set debug lin-lwp 1' -ex c
GNU gdb (GDB) 7.3.50.20110514-cvs
This GDB was configured as "x86_64-unknown-linux-gnu".
Program received signal SIGUSR1, User defined signal 1.
(gdb) info threads 
  Id   Target Id         Frame 
  11   Thread 0x7ffff3019700 (LWP 31784) "threadsigs" 0x00007ffff7bcecfd in pause () at ../sysdeps/unix/syscall-template.S:82
... [ everything in pause () ]
  2    Thread 0x7ffff7822700 (LWP 31775) "threadsigs" 0x00007ffff7bcecfd in pause () at ../sysdeps/unix/syscall-template.S:82
* 1    Thread 0x7ffff7fe6720 (LWP 31772) "threadsigs" 0x00007ffff78d0ced in nanosleep () at ../sysdeps/unix/syscall-template.S:82
(gdb) _

$ grep SigCgt /proc/31772/task/31772/status
SigCgt:	0000000180000000

OK, so the threads managed to deliver both SIGUSR1 and SIGUSR2 but GDB has
reported only SIGUSR1 to the user.

# The debugee does not handle SIGUSR1 so it would crash on its delivery:
(gdb) handle SIGUSR1 nopass
Signal        Stop	Print	Pass to program	Description
SIGUSR1       Yes	Yes	No		User defined signal 1
(gdb) continue 
Program received signal SIGUSR1, User defined signal 1.

OK, GDB has waitpid()ed SIGUSR1 already and still some thread has delivered
afterwards before GDB has managed to stop that thread.

(gdb) continue 
Program received signal SIGUSR2, User defined signal 2.

Only now the user has found SIGUSR2 has also been delivered.  The main thread
(receiving the signals) has not run yet been resumed at all.  It would be nice
if GDB could display all the signals the inferior has received as the other
threads are stopped already after the signals were sent (in pause ()) - this
gives user a skewed picture of different state in time for each thread.

I would prefer if GDB would print all the signals at once on a single stop:

Program received signal SIGUSR1, User defined signal 1.
Program received signal SIGUSR2, User defined signal 2.
(gdb) _

(This is not a simple change for GDB as it has many operations bound to
receiving single signal.)

Currently when GDB receives SIGUSR1 it has to do PTRACE_CONT before waitpid()
and receiving SIGUSR2.  The time it does PTRACE_CONT it does not know if then
waitpid() returns immediately or if the application will run for another hour.

There are similar problems GDB wanting to do something-like-INTERRUPT sends now
SIGSTOP and then it wants to remove that SIGSTOP from the inferior's queue as
it would confuse both user and the debuggee if left there.  Fortunately this
paragraph's pain will no longer be needed with PTRACE_INTERRUPT.

For example if you guarantee that after PTRACE_INTERRUPT the INTERRUPT even
will always get delivered as the last one after all the other signals GDB could
safely operate on all the delivered signals without a risk of accidentally
resuming the debuggee before explicitly instructed to do so by the user.

This is not a real plan how it should be done - but I hope it gives a picture
debuggers are interested the processing all the already delivered signals.
GDB should probably check the SigCgt /proc field (it already does in some
cases) for the informational display of delivered threads.


Thanks,
Jan

  reply	other threads:[~2011-05-15 19:49 UTC|newest]

Thread overview: 115+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-08 15:48 [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification Tejun Heo
2011-05-08 15:48 ` [PATCH 01/11] job control: rename signal->group_stop and flags to jobctl and rearrange flags Tejun Heo
2011-05-08 15:48 ` [PATCH 02/11] ptrace: implement PTRACE_SEIZE Tejun Heo
2011-05-09 16:18   ` Oleg Nesterov
2011-05-10  9:46     ` Tejun Heo
2011-05-10 13:20       ` Oleg Nesterov
2011-05-10 13:47         ` Tejun Heo
2011-05-10 18:19           ` Oleg Nesterov
2011-05-15 15:56   ` PTRACE_SEIZE should not stop [Re: [PATCH 02/11] ptrace: implement PTRACE_SEIZE] Jan Kratochvil
2011-05-15 16:26     ` Tejun Heo
2011-05-15 17:15       ` Jan Kratochvil
2011-05-15 17:25         ` Tejun Heo
2011-05-15 19:48           ` Jan Kratochvil [this message]
2011-05-16  8:31             ` Tejun Heo
2011-05-16 12:26               ` Jan Kratochvil
2011-05-16 12:42                 ` Tejun Heo
2011-05-16 13:03                   ` Jan Kratochvil
2011-05-16 13:51                     ` Tejun Heo
2011-05-16 13:21               ` Jan Kratochvil
2011-05-16 13:45                 ` Tejun Heo
2011-05-16 13:48                   ` Jan Kratochvil
2011-05-16 13:54                     ` Tejun Heo
2011-05-08 15:48 ` [PATCH 03/11] ptrace: ptrace_check_attach(): rename @kill to @ignore_state and add comments Tejun Heo
2011-05-08 15:48 ` [PATCH 04/11] ptrace: implement PTRACE_INTERRUPT Tejun Heo
2011-05-08 21:58   ` Denys Vlasenko
2011-05-09 10:09     ` Tejun Heo
2011-05-09 10:55       ` Denys Vlasenko
2011-05-09 16:58   ` Oleg Nesterov
2011-05-10  9:50     ` Tejun Heo
2011-05-10 14:06       ` Oleg Nesterov
2011-05-10 14:20         ` Tejun Heo
2011-05-10 18:08           ` Oleg Nesterov
2011-05-11  8:29             ` Tejun Heo
2011-05-12 17:06               ` Oleg Nesterov
2011-05-12 17:21                 ` Tejun Heo
2011-05-10 21:59         ` Denys Vlasenko
2011-05-11  9:19           ` Tejun Heo
2011-05-11 12:23             ` Denys Vlasenko
2011-05-11 13:22               ` Tejun Heo
2011-05-11 16:20                 ` Bryan Donlan
2011-05-11 19:24                   ` Tejun Heo
2011-05-15 16:10             ` PTRACE_DETACH without stop [Re: [PATCH 04/11] ptrace: implement PTRACE_INTERRUPT] Jan Kratochvil
2011-05-15 16:35               ` Tejun Heo
2011-05-15 17:39                 ` Jan Kratochvil
2011-05-16  9:01                   ` Tejun Heo
2011-05-16 12:08                     ` Jan Kratochvil
2011-05-16 12:24                       ` Tejun Heo
2011-05-08 15:48 ` [PATCH 05/11] ptrace: restructure ptrace_getsiginfo() Tejun Heo
2011-05-08 15:49 ` [PATCH 06/11] ptrace: make group stop state visible via PTRACE_GETSIGINFO Tejun Heo
2011-05-10 16:55   ` Oleg Nesterov
2011-05-10 17:11     ` Oleg Nesterov
2011-05-11  8:08     ` Tejun Heo
2011-05-12 16:47       ` Oleg Nesterov
2011-05-12 17:15         ` Tejun Heo
2011-05-08 15:49 ` [PATCH 07/11] ptrace: add JOBCTL_TRAPPED Tejun Heo
2011-05-08 15:49 ` [PATCH 08/11] ptrace: move fallback JOBCTL_TRAPPING clearing to get_signal_to_deliver() Tejun Heo
2011-05-11 15:48   ` Oleg Nesterov
2011-05-11 19:17     ` Tejun Heo
2011-05-12 15:40       ` Oleg Nesterov
2011-05-08 15:49 ` [PATCH 09/11] job control: reorganize wait_task_stopped() Tejun Heo
2011-05-11 15:48   ` Oleg Nesterov
2011-05-11 19:29     ` Tejun Heo
2011-05-12 15:42       ` Oleg Nesterov
2011-05-12 16:02         ` Tejun Heo
2011-05-12 17:25           ` Oleg Nesterov
2011-05-12 17:32             ` Tejun Heo
2011-05-12 17:33               ` Tejun Heo
2011-05-12 18:33               ` Oleg Nesterov
2011-05-13  8:46                 ` Tejun Heo
2011-05-13 17:21                   ` Oleg Nesterov
2011-05-14 10:56                     ` Tejun Heo
2011-05-15 14:40               ` waitpid(WNOHANG) should report SIGCHLD-notified signals [Re: [PATCH 09/11] job control: reorganize wait_task_stopped()] Jan Kratochvil
2011-05-15 16:47                 ` Tejun Heo
2011-05-15 17:01                   ` Tejun Heo
2011-05-15 17:47                   ` Jan Kratochvil
2011-05-16  9:13                     ` Tejun Heo
2011-05-16 12:11                       ` Jan Kratochvil
2011-05-16 12:27                         ` Tejun Heo
2011-05-16 12:39                           ` Jan Kratochvil
2011-05-16 12:46                             ` Tejun Heo
2011-05-08 15:49 ` [PATCH 10/11] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach() Tejun Heo
2011-05-11 16:49   ` Oleg Nesterov
2011-05-11 17:00     ` Oleg Nesterov
2011-05-11 19:45       ` Tejun Heo
2011-05-11 19:53     ` Tejun Heo
2011-05-12 10:23       ` Tejun Heo
2011-05-12 16:06         ` Oleg Nesterov
2011-05-12 15:59       ` Oleg Nesterov
2011-05-12 16:07         ` Tejun Heo
2011-05-12 18:20           ` Oleg Nesterov
2011-05-13  9:13             ` Tejun Heo
2011-05-13 18:34               ` Oleg Nesterov
2011-05-08 15:49 ` [PATCH 11/11] ptrace: implement group stop notification for ptracer Tejun Heo
2011-05-08 22:42   ` Denys Vlasenko
2011-05-09 10:10     ` Tejun Heo
2011-05-10 22:37   ` Denys Vlasenko
2011-05-11  9:05     ` Tejun Heo
2011-05-11 12:01       ` Denys Vlasenko
2011-05-11 13:13         ` Tejun Heo
2011-05-11 19:58   ` Oleg Nesterov
2011-05-11 20:18     ` Tejun Heo
2011-05-11 20:21       ` Tejun Heo
2011-05-12 10:24         ` Tejun Heo
2011-05-15 14:02   ` getter PTRACE_GETSIGINFO should not modify anything [Re: [PATCH 11/11] ptrace: implement group stop notification for ptracer] Jan Kratochvil
2011-05-15 14:28     ` Tejun Heo
2011-05-15 17:17       ` Jan Kratochvil
2011-05-15 17:28         ` Tejun Heo
2011-05-15 20:06           ` Jan Kratochvil
2011-05-16  8:43             ` Tejun Heo
2011-05-16 12:17               ` Jan Kratochvil
2011-05-16 12:56                 ` Tejun Heo
2011-05-16 13:00                   ` Ingo Molnar
2011-05-08 22:27 ` [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification Denys Vlasenko
2011-05-09  9:48   ` Tejun Heo
2011-05-15 13:55   ` ptrace-testsuite status [Re: [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification] Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110515194829.GA27023@host1.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=indan@nul.nu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®