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: Mon, 16 May 2011 14:26:42 +0200	[thread overview]
Message-ID: <20110516122642.GD10469@host1.jankratochvil.net> (raw)
In-Reply-To: <20110516083113.GN23665@htj.dyndns.org>

Hi Tejun,

On Mon, 16 May 2011 10:31:13 +0200, Tejun Heo wrote:
> On Sun, May 15, 2011 at 09:48:29PM +0200, Jan Kratochvil wrote:
> > # 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.
> 
> I can't understand the above sentence.  A thread can't deliver signal
> without going through tracer while ptraced.  Can you elaborate a bit
> more?

I tried to explain why GDB will see SIGUSR1 twice.  Despite it is not
a realtime signal and therefore the signal is "flag", it does not queue/count.
You know better than me why GDB sees SIGUSR1 twice.


> > (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.
> 
> There's no distinction between main or sub threads in terms of signal
> delivery unless signal itself is specifically directed to a thread.

This sample code uses only tkill to avoid any mess with which TID will get
which signal.


> > 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.
> 
> Isn't that the signal pending mask?

Yes but how do you query siginfo_t (GDB $_siginfo) of a pending signal to make
it accessible to the user?  You also need to mask out blocked signals and
properly order them like kernel does - which is not guaranteed by POSIX.
You need to reimplement part of the kernel functionality and if you implement
it a bit differently it will break transparency of the debugging.


> > 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) _
> 
> Ditto.
> 
> > (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.
> 
> Signal delivery is sequential in nature and delivering a signal which
> has user specified signal handler involves roundtrip to userland.  I'm
> not following what you're suggesting.
> 
> > 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.
> 
> Okay, I'm a bit confused, so let's clear things up a bit.
> 
> * Signal is sent to a group of threads of a specific thread.  Note
>   that SIGCONT wakes up stopped process at this point.

Normally yes but this sample code uses tkill to avoid it.


> * On the receipient, the signal becomes pending.  The mask of pending
>   signals is visible through /proc.

But not their siginfo_t, not which are blocked, their ordering etc.


> * Signal is delievered when the receipient processes those pending
>   signals.  This, of course, happens one signal after another.
>   Depending on signal and configuration, signal may be ignored, kill,
>   stop the process or trigger signal handler which involves roundtrip
>   to userland.
>   
> * ptrace is notified of and can alter signal delivery.
> 
> Given the different modes of signal deliveries, I don't think
> prioritizing signal delivery to other traps makes sense.
> 
> Hmmm... but I think what you want can be achieved with simply calling
> PTRACE_INTERRUPT on each signal delivery trap.  The tracee will
> deliver the signal and then immediately take INTERRUPT trap.  ie.
> 
> * Check if there are pending signals which can be delivered by this
>   thread.  Note that different threads may have different pending and
>   blocked masks so there isn't a single thread which can do
>   everything.
> 
> * If there are signals to deliver,

This is the question if the debugger can reliably detect.  Maybe it can.


>   CONT it and it will take the signal
>   trap (eventually).  During signal trap, do PTRACE_INTERRUPT and then
>   let the tracee deliver the signal.  Tracee will deliver the signal
>   and take STOP trap.
> 
> Is the above enough for your use case?

If there is enough documentation - or one reads the soures - one can
reimplement the signal delivery login in userland to expect what will kernel
do.  TBH I do not think it is the right API but you are right it is
workaroundable in userland.


Thanks,
Jan

  reply	other threads:[~2011-05-16 12:27 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
2011-05-16  8:31             ` Tejun Heo
2011-05-16 12:26               ` Jan Kratochvil [this message]
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=20110516122642.GD10469@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®