Linux: Use kill_lwp/tkill instead of kill when killing a, process

Message ID 53C54C7C.3070907@redhat.com
State Committed
Headers

Commit Message

Pedro Alves July 15, 2014, 3:45 p.m. UTC
  Another thing I noticed by inspection while fixing the
recent gdbserver kill crash.

Anyone know a reason we use plain "kill" here, instead
of tkill like everywhere else?

Passes testing on x86_64 Fedora 20 for me, at least...

--------------
From 08a773214245e9d6deb5060a6025c169fa8cdc4d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 11 Jul 2014 11:36:20 +0100
Subject: [PATCH] Linux: Use kill_lwp/tkill instead of kill when killing a
 process

Since we use tkill everywhere, using kill to try to kill each lwp
individually looks suspiciously odd.  We should really be using tgkill
everywhere, but at least while we don't get there this makes us
consistent.

gdb/gdbserver/
2014-07-15  Pedro Alves  <palves@redhat.com>

	* linux-low.c (linux_kill_one_lwp): Use kill_lwp, not kill.

gdb/
2014-07-15  Pedro Alves  <palves@redhat.com>

	* linux-nat.c (kill_callback): Use kill_lwp, not kill.
---
 gdb/gdbserver/linux-low.c | 2 +-
 gdb/linux-nat.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Jan Kratochvil July 15, 2014, 7:27 p.m. UTC | #1
On Tue, 15 Jul 2014 17:45:00 +0200, Pedro Alves wrote:
> Another thing I noticed by inspection while fixing the
> recent gdbserver kill crash.
> 
> Anyone know a reason we use plain "kill" here, instead
> of tkill like everywhere else?

I do not.
Just it is the general GDB rule of "If it ain't broken, don't fix it".
There are various old and patched kernels out there.


Jan
  
Stan Shebs July 15, 2014, 8:52 p.m. UTC | #2
On 7/15/14, 12:27 PM, Jan Kratochvil wrote:
> On Tue, 15 Jul 2014 17:45:00 +0200, Pedro Alves wrote:
>> Another thing I noticed by inspection while fixing the
>> recent gdbserver kill crash.
>>
>> Anyone know a reason we use plain "kill" here, instead
>> of tkill like everywhere else?
> 
> I do not.

Heh, since it ultimately stems from a patch you did for Archer:

https://sourceware.org/ml/archer/2011-q1/msg00102.html

But going by context, maybe kill_lwp was just overlooked at
the time, and then propagated verbatim thereafter.

Stan
stan@codesourcery.com
  
Pedro Alves July 16, 2014, 7:02 p.m. UTC | #3
On 07/15/2014 08:27 PM, Jan Kratochvil wrote:

> I do not.
> Just it is the general GDB rule of "If it ain't broken, don't fix it".
> There are various old and patched kernels out there.

Well, it's broken, because it's not using tgkill, which we should be
using everywhere instead of tkill/kill.  Leaving the place that sends
SIGKILL using plain "kill" is kind of self-defeating.

If there are NPTL kernels that require "kill" for some odd reason,
then we should add some kind of detection for that, so to not punish
newer kernels.  LinuxThreads will always fallback to "kill".

On 07/15/2014 09:52 PM, Stan Shebs wrote:
> But going by context, maybe kill_lwp was just overlooked at
> the time, and then propagated verbatim thereafter.

Yes, that's my theory.

I'm pushing this in then.
  

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 0f4dbe2..521d9a2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -895,7 +895,7 @@  linux_kill_one_lwp (struct lwp_info *lwp)
      everywhere.  */
 
   errno = 0;
-  kill (pid, SIGKILL);
+  kill_lwp (pid, SIGKILL);
   if (debug_threads)
     {
       int save_errno = errno;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index c738abf..b50a88e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3704,7 +3704,7 @@  kill_callback (struct lwp_info *lp, void *data)
   /* PTRACE_KILL may resume the inferior.  Send SIGKILL first.  */
 
   errno = 0;
-  kill (ptid_get_lwp (lp->ptid), SIGKILL);
+  kill_lwp (ptid_get_lwp (lp->ptid), SIGKILL);
   if (debug_linux_nat)
     {
       int save_errno = errno;