public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA/Linux] Ask kernel to kill inferior when GDB terminates
@ 2014-11-14 16:54 Joel Brobecker
  2014-11-14 17:09 ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-11-14 16:54 UTC (permalink / raw)
  To: gdb-patches

Hello,

This patch enhances GDB on GNU/Linux systems to ask the kernel
to kill the inferior if GDB terminates without doing it itself.
This would typically happen when GDB encounters a problem and
crashes, or when it gets killed by an external process. This can
be observed by starting a program under GDB, and then killing
GDB with signal 9. After GDB is killed, the inferior still remains
in "interruptible sleep (waiting for an event to complete)" state.

gdb/ChangeLog:

        * nat/linux-ptrace.h (PTRACE_O_EXITKILL): Define if not
        already defined.
        * nat/linux-ptrace.c (linux_test_for_exitkill): New advance
        declaration.  New function.
        (linux_check_ptrace_features): Call linux_test_for_exitkill.

Tested on various x86_64-linux and x86-linux distributions
(RHES5, RHES6, RHES7) and my x86_64-linux machine, which runs
Ubuntu 14.04. On older versions of the kernel, the problem
persists, since the feature is not available, while the inferior
now automatically gets killed on newer versions of the kernel.

OK to commit?

PS: On a side note, the use of advance declarations for those
"linux_test_for_[...]" functions isn't really necessary, if we
were to move the body of those functions ahaad of
linux_check_ptrace_features. For now, I went with consistency,
but I am more than happy providing a followup patch that does
the move.

Thanks,
-- 
Joel

---
 gdb/nat/linux-ptrace.c | 17 +++++++++++++++++
 gdb/nat/linux-ptrace.h |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 8bc3f16..15da35d 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -307,6 +307,7 @@ linux_child_function (gdb_byte *child_stack)
 
 static void linux_test_for_tracesysgood (int child_pid);
 static void linux_test_for_tracefork (int child_pid);
+static void linux_test_for_exitkill (int child_pid);
 
 /* Determine ptrace features available on this target.  */
 
@@ -338,6 +339,8 @@ linux_check_ptrace_features (void)
 
   linux_test_for_tracefork (child_pid);
 
+  linux_test_for_exitkill (child_pid);
+
   /* Clean things up and kill any pending children.  */
   do
     {
@@ -449,6 +452,20 @@ linux_test_for_tracefork (int child_pid)
 	     "(%d, status 0x%x)"), ret, status);
 }
 
+/* Determine if PTRACE_O_EXITKILL can be used.  */
+
+static void
+linux_test_for_exitkill (int child_pid)
+{
+  int ret;
+
+  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+		(PTRACE_TYPE_ARG4) PTRACE_O_EXITKILL);
+
+  if (ret == 0)
+    current_ptrace_options |= PTRACE_O_EXITKILL;
+}
+
 /* Enable reporting of all currently supported ptrace events.  */
 
 void
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 31a77cd..7afc99e 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -69,6 +69,11 @@ struct buffer;
 
 #endif /* PTRACE_EVENT_FORK */
 
+#ifndef PTRACE_O_EXITKILL
+/* Only defined in Linux Kernel 3.8 or later.  */
+#define PTRACE_O_EXITKILL	0x00100000
+#endif
+
 #if (defined __bfin__ || defined __frv__ || defined __sh__) \
     && !defined PTRACE_GETFDPIC
 #define PTRACE_GETFDPIC		31
-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-11-14 16:54 [RFA/Linux] Ask kernel to kill inferior when GDB terminates Joel Brobecker
@ 2014-11-14 17:09 ` Pedro Alves
  2014-11-14 17:33   ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-11-14 17:09 UTC (permalink / raw)
  To: Joel Brobecker, gdb-patches

On 11/14/2014 04:53 PM, Joel Brobecker wrote:
> Hello,
> 
> This patch enhances GDB on GNU/Linux systems to ask the kernel
> to kill the inferior if GDB terminates without doing it itself.
> This would typically happen when GDB encounters a problem and
> crashes, or when it gets killed by an external process. This can
> be observed by starting a program under GDB, and then killing
> GDB with signal 9. After GDB is killed, the inferior still remains
> in "interruptible sleep (waiting for an event to complete)" state.

I could see this making some sense when GDB has spawned the process,
but it seems harsh when GDB has attached to the process instead
of spawning it?

Note that Windows has had a similar feature for
ages (DebugSetProcessKillOnExit), and how windows-nat.c calls
DebugSetProcessKillOnExit(false) when GDB attaches to a process.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-11-14 17:09 ` Pedro Alves
@ 2014-11-14 17:33   ` Joel Brobecker
  2014-11-19  9:25     ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-11-14 17:33 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> I could see this making some sense when GDB has spawned the process,
> but it seems harsh when GDB has attached to the process instead
> of spawning it?

Hmmm, good point. I'd like to verify what happens in that case, and
whether the process remains stopped or if execution resumes in that
case.

> Note that Windows has had a similar feature for
> ages (DebugSetProcessKillOnExit), and how windows-nat.c calls
> DebugSetProcessKillOnExit(false) when GDB attaches to a process.

Thanks, Pedro.

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-11-14 17:33   ` Joel Brobecker
@ 2014-11-19  9:25     ` Joel Brobecker
  2014-11-19  9:58       ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-11-19  9:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> > I could see this making some sense when GDB has spawned the process,
> > but it seems harsh when GDB has attached to the process instead
> > of spawning it?
> 
> Hmmm, good point. I'd like to verify what happens in that case, and
> whether the process remains stopped or if execution resumes in that
> case.

I took a little bit of time to experiment today. The experimentation
was conducted with Linux 3.13.0-39-generic from x86_64 Ubuntu.
In both cases, when GDB runs the program or attaches to a program,
killing GDB while the inferior is still running detaches GDB from
the inferior, which means it keeps running.

I will adapt my patch when I have a moment.

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-11-19  9:25     ` Joel Brobecker
@ 2014-11-19  9:58       ` Pedro Alves
  2014-12-13 16:28         ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-11-19  9:58 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/19/2014 09:25 AM, Joel Brobecker wrote:
>>> I could see this making some sense when GDB has spawned the process,
>>> but it seems harsh when GDB has attached to the process instead
>>> of spawning it?
>>
>> Hmmm, good point. I'd like to verify what happens in that case, and
>> whether the process remains stopped or if execution resumes in that
>> case.
> 
> I took a little bit of time to experiment today. The experimentation
> was conducted with Linux 3.13.0-39-generic from x86_64 Ubuntu.
> In both cases, when GDB runs the program or attaches to a program,
> killing GDB while the inferior is still running detaches GDB from
> the inferior, which means it keeps running.

Sorry, I didn't realize you meant to verify what happens without
the patch.  Yeah, it's because the ptracer always detaches that
the option was introduced, not that long ago.

Note that with PTRACE_TRACEME there's still a time window between
starting to trace a task and setting its ptrace options, where
GDB could crash, and thus we can still miss killing the tracee.  To
completely close that, we'd have to switch to use the newer PTRACE_SEIZE
instead, which takes the ptrace options as argument, but that'd be
a story of its own.

> I will adapt my patch when I have a moment.

Thank you.

Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-11-19  9:58       ` Pedro Alves
@ 2014-12-13 16:28         ` Joel Brobecker
  2014-12-15 13:22           ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-12-13 16:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> >>> I could see this making some sense when GDB has spawned the process,
> >>> but it seems harsh when GDB has attached to the process instead
> >>> of spawning it?
[...]
> > I will adapt my patch when I have a moment.

Just took a look, and it's not clear to me what the best way to do that
might me. For native GDB, it'd be fairly easy, if I could just add
an "attaching_p" parameter to "linux_enable_event_reporting", then
I could just mask PTRACE_O_EXITKILL out if nonzero. In linux-nat.c,
it's fairly easy to provide that info. However, things are not so
clear in the gdbserver case, because linux_enable_event_reporting
is performed during the event handling conditional on
child->must_set_ptrace_flags:

  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
    {
      linux_enable_event_reporting (lwpid);
      child->must_set_ptrace_flags = 0;
    }

I think the only clean way to do this would be to add another
field in struct lwp_info such as "attaching_p" which we would
set to 1 in the attaching case only.

Does this seem reasonable?

Thanks,
-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-13 16:28         ` Joel Brobecker
@ 2014-12-15 13:22           ` Pedro Alves
  2014-12-15 20:43             ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-12-15 13:22 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 12/13/2014 04:27 PM, Joel Brobecker wrote:
>>>>> I could see this making some sense when GDB has spawned the process,
>>>>> but it seems harsh when GDB has attached to the process instead
>>>>> of spawning it?
> [...]
>>> I will adapt my patch when I have a moment.
> 
> Just took a look, and it's not clear to me what the best way to do that
> might me. For native GDB, it'd be fairly easy, if I could just add
> an "attaching_p" parameter to "linux_enable_event_reporting", then
> I could just mask PTRACE_O_EXITKILL out if nonzero. In linux-nat.c,
> it's fairly easy to provide that info. However, things are not so
> clear in the gdbserver case, because linux_enable_event_reporting
> is performed during the event handling conditional on
> child->must_set_ptrace_flags:
> 
>   if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
>     {
>       linux_enable_event_reporting (lwpid);
>       child->must_set_ptrace_flags = 0;
>     }
> 
> I think the only clean way to do this would be to add another
> field in struct lwp_info such as "attaching_p" which we would
> set to 1 in the attaching case only.

I think we can get by without adding a new field.  The process_info
structure has the "process->attached" field already.  We could
check that here.  Alternatively, must_set_ptrace_flags could be a
tri-state instead of a boolean, which would be cheaper.

> 
> Does this seem reasonable?
> 
> Thanks,
> 


Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-15 13:22           ` Pedro Alves
@ 2014-12-15 20:43             ` Joel Brobecker
  2014-12-15 21:17               ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-12-15 20:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

> I think we can get by without adding a new field.  The process_info
> structure has the "process->attached" field already.  We could
> check that here.

Indeed! Attached is an updated patch which uses that flag.

gdb/ChangeLog:

        * nat/linux-ptrace.h (PTRACE_O_EXITKILL): Define if not
        already defined.
        (linux_enable_event_reporting): Add parameter "attached".
        * nat/linux-ptrace.c (linux_test_for_exitkill): New advance
        declaration.  New function.
        (linux_check_ptrace_features): Add new paramter "attached".
        Call linux_test_for_exitkill if !ATTACHED.
        (linux_enable_event_reporting): Add new parameter "attached".
        Update call to linux_check_ptrace_features.
        * linux-nat.c (linux_init_ptrace): Add parameter "attached".
        Use it.  Update function description.
        (linux_child_post_attach, linux_child_post_startup_inferior):
        Update call to linux_enable_event_reporting.

gdb/gdbserver/ChangeLog:

        * linux-low.c (linux_low_filter_event): Update call to
        linux_enable_event_reporting following the addition of
        a new parameter to that function.

Tested on x86_64-linux, native and native-gdbserver.

I also verified by hand that the inferior gets killed when killing
GDB in the "run" case, while the inferior remains in the "attach"
case. Same for GDBserver.

OK to push?

Thank you!
-- 
Joel

[-- Attachment #2: 0001-Linux-Ask-kernel-to-kill-inferior-when-GDB-terminate.patch --]
[-- Type: text/x-diff, Size: 6829 bytes --]

From af12de3224fce611ea55325015db4822c7e96ff2 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 11 Nov 2014 10:07:21 +0400
Subject: [PATCH] [Linux] Ask kernel to kill inferior when GDB terminates

This patch enhances GDB on GNU/Linux systems in the situation where
we are debugging an inferior that was created from GDB (as opposed
to attached to), by asking the kernel to kill the inferior if GDB
terminates without doing it itself.

This would typically happen when GDB encounters a problem and
crashes, or when it gets killed by an external process. This can
be observed by starting a program under GDB, and then killing
GDB with signal 9. After GDB is killed, the inferior still remains
in "interruptible sleep (waiting for an event to complete)" state.

This patch also fixes GDBserver similarly.

This fix is conditional on the kernel supporting the PTRACE_O_EXITKILL
feature.  On older kernels, the behavior remains unchanged.

gdb/ChangeLog:

        * nat/linux-ptrace.h (PTRACE_O_EXITKILL): Define if not
        already defined.
        (linux_enable_event_reporting): Add parameter "attached".
        * nat/linux-ptrace.c (linux_test_for_exitkill): New advance
        declaration.  New function.
        (linux_check_ptrace_features): Add new paramter "attached".
        Call linux_test_for_exitkill if !ATTACHED.
        (linux_enable_event_reporting): Add new parameter "attached".
        Update call to linux_check_ptrace_features.
        * linux-nat.c (linux_init_ptrace): Add parameter "attached".
        Use it.  Update function description.
        (linux_child_post_attach, linux_child_post_startup_inferior):
        Update call to linux_enable_event_reporting.

gdb/gdbserver/ChangeLog:

        * linux-low.c (linux_low_filter_event): Update call to
        linux_enable_event_reporting following the addition of
        a new parameter to that function.

Tested on x86_64-linux, native and native-gdbserver.

I also verified by hand that the inferior gets killed when killing
GDB in the "run" case, while the inferior remains in the "attach"
case. Same for GDBserver.
---
 gdb/gdbserver/linux-low.c |  4 +++-
 gdb/linux-nat.c           | 12 +++++++-----
 gdb/nat/linux-ptrace.c    | 30 +++++++++++++++++++++++++-----
 gdb/nat/linux-ptrace.h    |  7 ++++++-
 4 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 5ea9200..65f72a2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1878,7 +1878,9 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
 
   if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
     {
-      linux_enable_event_reporting (lwpid);
+      struct process_info *proc = find_process_pid (pid_of (thread));
+
+      linux_enable_event_reporting (lwpid, proc->attached);
       child->must_set_ptrace_flags = 0;
     }
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 29133f9..845d566 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -321,25 +321,27 @@ pull_pid_from_list (struct simple_pid_list **listp, int pid, int *statusp)
 }
 
 /* Initialize ptrace warnings and check for supported ptrace
-   features given PID.  */
+   features given PID.
+
+   ATTACHED should be nonzero iff we attached to the inferior.  */
 
 static void
-linux_init_ptrace (pid_t pid)
+linux_init_ptrace (pid_t pid, int attached)
 {
-  linux_enable_event_reporting (pid);
+  linux_enable_event_reporting (pid, attached);
   linux_ptrace_init_warnings ();
 }
 
 static void
 linux_child_post_attach (struct target_ops *self, int pid)
 {
-  linux_init_ptrace (pid);
+  linux_init_ptrace (pid, 1);
 }
 
 static void
 linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
 {
-  linux_init_ptrace (ptid_get_pid (ptid));
+  linux_init_ptrace (ptid_get_pid (ptid), 0);
 }
 
 /* Return the number of known LWPs in the tgid given by PID.  */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 8bc3f16..8daa977 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -307,11 +307,13 @@ linux_child_function (gdb_byte *child_stack)
 
 static void linux_test_for_tracesysgood (int child_pid);
 static void linux_test_for_tracefork (int child_pid);
+static void linux_test_for_exitkill (int child_pid);
 
-/* Determine ptrace features available on this target.  */
+/* Determine ptrace features available on this target.
+   ATTACHED should be nonzero iff we've attached to the inferior.  */
 
 static void
-linux_check_ptrace_features (void)
+linux_check_ptrace_features (int attached)
 {
   int child_pid, ret, status;
 
@@ -338,6 +340,9 @@ linux_check_ptrace_features (void)
 
   linux_test_for_tracefork (child_pid);
 
+  if (!attached)
+    linux_test_for_exitkill (child_pid);
+
   /* Clean things up and kill any pending children.  */
   do
     {
@@ -449,15 +454,30 @@ linux_test_for_tracefork (int child_pid)
 	     "(%d, status 0x%x)"), ret, status);
 }
 
-/* Enable reporting of all currently supported ptrace events.  */
+/* Determine if PTRACE_O_EXITKILL can be used.  */
+
+static void
+linux_test_for_exitkill (int child_pid)
+{
+  int ret;
+
+  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+		(PTRACE_TYPE_ARG4) PTRACE_O_EXITKILL);
+
+  if (ret == 0)
+    current_ptrace_options |= PTRACE_O_EXITKILL;
+}
+
+/* Enable reporting of all currently supported ptrace events.
+   ATTACHED should be nonzero if we have attached to the inferior.  */
 
 void
-linux_enable_event_reporting (pid_t pid)
+linux_enable_event_reporting (pid_t pid, int attached)
 {
   /* Check if we have initialized the ptrace features for this
      target.  If not, do it now.  */
   if (current_ptrace_options == -1)
-    linux_check_ptrace_features ();
+    linux_check_ptrace_features (attached);
 
   /* Set the options.  */
   ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0,
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 31a77cd..588d38a 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -69,6 +69,11 @@ struct buffer;
 
 #endif /* PTRACE_EVENT_FORK */
 
+#ifndef PTRACE_O_EXITKILL
+/* Only defined in Linux Kernel 3.8 or later.  */
+#define PTRACE_O_EXITKILL	0x00100000
+#endif
+
 #if (defined __bfin__ || defined __frv__ || defined __sh__) \
     && !defined PTRACE_GETFDPIC
 #define PTRACE_GETFDPIC		31
@@ -85,7 +90,7 @@ struct buffer;
 
 extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer);
 extern void linux_ptrace_init_warnings (void);
-extern void linux_enable_event_reporting (pid_t pid);
+extern void linux_enable_event_reporting (pid_t pid, int attached);
 extern void linux_disable_event_reporting (pid_t pid);
 extern int linux_supports_tracefork (void);
 extern int linux_supports_traceclone (void);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-15 20:43             ` Joel Brobecker
@ 2014-12-15 21:17               ` Pedro Alves
  2014-12-16  0:39                 ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-12-15 21:17 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 12/15/2014 08:43 PM, Joel Brobecker wrote:

> GDB with signal 9. After GDB is killed, the inferior still remains
> in "interruptible sleep (waiting for an event to complete)" state.

This makes it sound like "interruptible sleep" is some kind of wedged
state waiting for a debugger.  But, this simply means your process is just
running as normal, and your test's case, normal means idling.  Most likely,
your test program had a "sleep" call in it.  IOW, it'd be in sleep state
even if had been started outside gdb.  Another test program could go
to "T (stopped)", "R (running)", etc.

> -/* Determine ptrace features available on this target.  */
> +/* Determine ptrace features available on this target.
> +   ATTACHED should be nonzero iff we've attached to the inferior.  */
>
>  static void
> -linux_check_ptrace_features (void)
> +linux_check_ptrace_features (int attached)
>  {
>    int child_pid, ret, status;
>
> @@ -338,6 +340,9 @@ linux_check_ptrace_features (void)
>
>    linux_test_for_tracefork (child_pid);
>
> +  if (!attached)
> +    linux_test_for_exitkill (child_pid);

This test must be called unconditionally.  current_ptrace_options is
only initialized once:

 void
 linux_enable_event_reporting (pid_t pid)
 {
   /* Check if we have initialized the ptrace features for this
      target.  If not, do it now.  */
   if (current_ptrace_options == -1)
     linux_check_ptrace_features ();

so if the first process gdbserver debugs is an "attach" process,
but the second one is not, you'll miss setting PTRACE_O_EXITKILL on
the second process.

Instead, always call linux_test_for_exitkill, and then in
linux_enable_event_reporting mask out PTRACE_O_EXITKILL
from current_ptrace_options if 'attached' is false.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-15 21:17               ` Pedro Alves
@ 2014-12-16  0:39                 ` Joel Brobecker
  2014-12-16  9:46                   ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2014-12-16  0:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

> This makes it sound like "interruptible sleep" is some kind of wedged
> state waiting for a debugger.  But, this simply means your process is just
> running as normal, and your test's case, normal means idling.  Most likely,
> your test program had a "sleep" call in it.  IOW, it'd be in sleep state
> even if had been started outside gdb.  Another test program could go
> to "T (stopped)", "R (running)", etc.

You're right. Rev log updated.

> This test must be called unconditionally.  current_ptrace_options is
> only initialized once:
> 
>  void
>  linux_enable_event_reporting (pid_t pid)
>  {
>    /* Check if we have initialized the ptrace features for this
>       target.  If not, do it now.  */
>    if (current_ptrace_options == -1)
>      linux_check_ptrace_features ();
> 
> so if the first process gdbserver debugs is an "attach" process,
> but the second one is not, you'll miss setting PTRACE_O_EXITKILL on
> the second process.
> 
> Instead, always call linux_test_for_exitkill, and then in
> linux_enable_event_reporting mask out PTRACE_O_EXITKILL
> from current_ptrace_options if 'attached' is false.

Right again.

Here is an attached patch. Re-tested on x86_64-linux as previously
done.

gdb/ChangeLog:

        * nat/linux-ptrace.h (PTRACE_O_EXITKILL): Define if not
        already defined.
        (linux_enable_event_reporting): Add parameter "attached".
        * nat/linux-ptrace.c (linux_test_for_exitkill): New advance
        declaration.  New function.
        (linux_check_ptrace_features): Add linux_test_for_exitkill call.
        (linux_enable_event_reporting): Add new parameter "attached".
        Do not call ptrace with the PTRACE_O_EXITKILL if ATTACHED is
        nonzero.
        * linux-nat.c (linux_init_ptrace): Add parameter "attached".
        Use it.  Update function description.
        (linux_child_post_attach, linux_child_post_startup_inferior):
        Update call to linux_enable_event_reporting.

gdb/gdbserver/ChangeLog:

        * linux-low.c (linux_low_filter_event): Update call to
        linux_enable_event_reporting following the addition of
        a new parameter to that function.

Thanks, Pedro.
-- 
Joel

[-- Attachment #2: 0001-Linux-Ask-kernel-to-kill-inferior-when-GDB-terminate.patch --]
[-- Type: text/x-diff, Size: 6885 bytes --]

From 4ad92cbfc3dfdfffe46f20e2b6e8df7a793c9ea7 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 11 Nov 2014 10:07:21 +0400
Subject: [PATCH] [Linux] Ask kernel to kill inferior when GDB terminates

This patch enhances GDB on GNU/Linux systems in the situation where
we are debugging an inferior that was created from GDB (as opposed
to attached to), by asking the kernel to kill the inferior if GDB
terminates without doing it itself.

This would typically happen when GDB encounters a problem and
crashes, or when it gets killed by an external process. This can
be observed by starting a program under GDB, and then killing
GDB with signal 9. After GDB is killed, the inferior still remains.

This patch also fixes GDBserver similarly.

This fix is conditional on the kernel supporting the PTRACE_O_EXITKILL
feature.  On older kernels, the behavior remains unchanged.

gdb/ChangeLog:

        * nat/linux-ptrace.h (PTRACE_O_EXITKILL): Define if not
        already defined.
        (linux_enable_event_reporting): Add parameter "attached".
        * nat/linux-ptrace.c (linux_test_for_exitkill): New advance
        declaration.  New function.
        (linux_check_ptrace_features): Add linux_test_for_exitkill call.
        (linux_enable_event_reporting): Add new parameter "attached".
        Do not call ptrace with the PTRACE_O_EXITKILL if ATTACHED is
        nonzero.
        * linux-nat.c (linux_init_ptrace): Add parameter "attached".
        Use it.  Update function description.
        (linux_child_post_attach, linux_child_post_startup_inferior):
        Update call to linux_enable_event_reporting.

gdb/gdbserver/ChangeLog:

        * linux-low.c (linux_low_filter_event): Update call to
        linux_enable_event_reporting following the addition of
        a new parameter to that function.

Tested on x86_64-linux, native and native-gdbserver.

I also verified by hand that the inferior gets killed when killing
GDB in the "run" case, while the inferior remains in the "attach"
case. Same for GDBserver.
---
 gdb/gdbserver/linux-low.c |  4 +++-
 gdb/linux-nat.c           | 12 +++++++-----
 gdb/nat/linux-ptrace.c    | 34 +++++++++++++++++++++++++++++++---
 gdb/nat/linux-ptrace.h    |  7 ++++++-
 4 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 5ea9200..65f72a2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1878,7 +1878,9 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
 
   if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
     {
-      linux_enable_event_reporting (lwpid);
+      struct process_info *proc = find_process_pid (pid_of (thread));
+
+      linux_enable_event_reporting (lwpid, proc->attached);
       child->must_set_ptrace_flags = 0;
     }
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 29133f9..845d566 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -321,25 +321,27 @@ pull_pid_from_list (struct simple_pid_list **listp, int pid, int *statusp)
 }
 
 /* Initialize ptrace warnings and check for supported ptrace
-   features given PID.  */
+   features given PID.
+
+   ATTACHED should be nonzero iff we attached to the inferior.  */
 
 static void
-linux_init_ptrace (pid_t pid)
+linux_init_ptrace (pid_t pid, int attached)
 {
-  linux_enable_event_reporting (pid);
+  linux_enable_event_reporting (pid, attached);
   linux_ptrace_init_warnings ();
 }
 
 static void
 linux_child_post_attach (struct target_ops *self, int pid)
 {
-  linux_init_ptrace (pid);
+  linux_init_ptrace (pid, 1);
 }
 
 static void
 linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
 {
-  linux_init_ptrace (ptid_get_pid (ptid));
+  linux_init_ptrace (ptid_get_pid (ptid), 0);
 }
 
 /* Return the number of known LWPs in the tgid given by PID.  */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 8bc3f16..a0e0c32 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -307,6 +307,7 @@ linux_child_function (gdb_byte *child_stack)
 
 static void linux_test_for_tracesysgood (int child_pid);
 static void linux_test_for_tracefork (int child_pid);
+static void linux_test_for_exitkill (int child_pid);
 
 /* Determine ptrace features available on this target.  */
 
@@ -338,6 +339,8 @@ linux_check_ptrace_features (void)
 
   linux_test_for_tracefork (child_pid);
 
+  linux_test_for_exitkill (child_pid);
+
   /* Clean things up and kill any pending children.  */
   do
     {
@@ -449,19 +452,44 @@ linux_test_for_tracefork (int child_pid)
 	     "(%d, status 0x%x)"), ret, status);
 }
 
-/* Enable reporting of all currently supported ptrace events.  */
+/* Determine if PTRACE_O_EXITKILL can be used.  */
+
+static void
+linux_test_for_exitkill (int child_pid)
+{
+  int ret;
+
+  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+		(PTRACE_TYPE_ARG4) PTRACE_O_EXITKILL);
+
+  if (ret == 0)
+    current_ptrace_options |= PTRACE_O_EXITKILL;
+}
+
+/* Enable reporting of all currently supported ptrace events.
+   ATTACHED should be nonzero if we have attached to the inferior.  */
 
 void
-linux_enable_event_reporting (pid_t pid)
+linux_enable_event_reporting (pid_t pid, int attached)
 {
+  int ptrace_options;
+
   /* Check if we have initialized the ptrace features for this
      target.  If not, do it now.  */
   if (current_ptrace_options == -1)
     linux_check_ptrace_features ();
 
+  ptrace_options = current_ptrace_options;
+  if (attached)
+    {
+      /* When attached to our inferior, we do not want the inferior
+	 to die with us if we terminate unexpectedly.  */
+      ptrace_options &= ~PTRACE_O_EXITKILL;
+    }
+
   /* Set the options.  */
   ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0,
-	  (PTRACE_TYPE_ARG4) (uintptr_t) current_ptrace_options);
+	  (PTRACE_TYPE_ARG4) (uintptr_t) ptrace_options);
 }
 
 /* Disable reporting of all currently supported ptrace events.  */
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 31a77cd..588d38a 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -69,6 +69,11 @@ struct buffer;
 
 #endif /* PTRACE_EVENT_FORK */
 
+#ifndef PTRACE_O_EXITKILL
+/* Only defined in Linux Kernel 3.8 or later.  */
+#define PTRACE_O_EXITKILL	0x00100000
+#endif
+
 #if (defined __bfin__ || defined __frv__ || defined __sh__) \
     && !defined PTRACE_GETFDPIC
 #define PTRACE_GETFDPIC		31
@@ -85,7 +90,7 @@ struct buffer;
 
 extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer);
 extern void linux_ptrace_init_warnings (void);
-extern void linux_enable_event_reporting (pid_t pid);
+extern void linux_enable_event_reporting (pid_t pid, int attached);
 extern void linux_disable_event_reporting (pid_t pid);
 extern int linux_supports_tracefork (void);
 extern int linux_supports_traceclone (void);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-16  0:39                 ` Joel Brobecker
@ 2014-12-16  9:46                   ` Pedro Alves
  2014-12-16 12:58                     ` pushed: " Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-12-16  9:46 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 12/16/2014 12:39 AM, Joel Brobecker wrote:

> Here is an attached patch. Re-tested on x86_64-linux as previously
> done.

Looks good.  Thanks Joel.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* pushed: [RFA/Linux] Ask kernel to kill inferior when GDB terminates
  2014-12-16  9:46                   ` Pedro Alves
@ 2014-12-16 12:58                     ` Joel Brobecker
  0 siblings, 0 replies; 12+ messages in thread
From: Joel Brobecker @ 2014-12-16 12:58 UTC (permalink / raw)
  To: gdb-patches

> > Here is an attached patch. Re-tested on x86_64-linux as previously
> > done.
> 
> Looks good.  Thanks Joel.

Thanks, Pedro. Patch now pushed, with the small ChangeLog fix you
suggested off list.

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-12-16 12:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-14 16:54 [RFA/Linux] Ask kernel to kill inferior when GDB terminates Joel Brobecker
2014-11-14 17:09 ` Pedro Alves
2014-11-14 17:33   ` Joel Brobecker
2014-11-19  9:25     ` Joel Brobecker
2014-11-19  9:58       ` Pedro Alves
2014-12-13 16:28         ` Joel Brobecker
2014-12-15 13:22           ` Pedro Alves
2014-12-15 20:43             ` Joel Brobecker
2014-12-15 21:17               ` Pedro Alves
2014-12-16  0:39                 ` Joel Brobecker
2014-12-16  9:46                   ` Pedro Alves
2014-12-16 12:58                     ` pushed: " Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).