[PATCH] Handle vfork in thread with follow-fork-mode child

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Handle vfork in thread with follow-fork-mode child

Tom de Vries
Hi,

When debugging the test-case vfork-follow-child.c (which does a vfork in a
thread) with follow-fork-mode child set, we run into this assertion:
...
src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
  void x86_linux_update_debug_registers(lwp_info*): \
  Assertion `lwp_is_stopped (lwp)' failed.
...

The assert is caused by the following: the event that the vfork child exits,
is handled by handle_vfork_child_exec_or_exit, which calls target_detach to
detach from the vfork parent.  During target_detach we call
linux_nat_target::detach, which:
- stops all the threads
- waits for all the threads to be stopped
- detaches all the threads.
However, during the second step we run into this code in stop_wait_callback:
...
  /* If this is a vfork parent, bail out, it is not going to report
     any SIGSTOP until the vfork is done with.  */
  if (inf->vfork_child != NULL)
    return 0;
...
and we don't wait for the threads to be stopped, which resulting in this
assert in x86_linux_update_debug_registers triggering during the third step:
...
  gdb_assert (lwp_is_stopped (lwp));
...

Fix this by resetting the vfork parent's vfork_child field before calling
target_detach in handle_vfork_child_exec_or_exit.

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

OK for trunk?

Thanks,
- Tom

[gdb] Handle vfork in thread with follow-fork-mode child

gdb/ChangeLog:

2019-04-16  Tom de Vries  <[hidden email]>

        PR gdb/24454
        * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
        avfork_child field before calling target_detach.

gdb/testsuite/ChangeLog:

2019-04-16  Tom de Vries  <[hidden email]>

        PR gdb/24454
        * gdb.threads/vfork-follow-child.c: New test.
        * gdb.threads/vfork-follow-child.exp: New file.

---
 gdb/infrun.c                                     | 13 ++++++++++++-
 gdb/testsuite/gdb.threads/vfork-follow-child.c   | 19 +++++++++++++++++++
 gdb/testsuite/gdb.threads/vfork-follow-child.exp | 21 +++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 37713b24fe..b088138250 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -923,6 +923,7 @@ handle_vfork_child_exec_or_exit (int exec)
   struct thread_info *tp;
   struct program_space *pspace;
   struct address_space *aspace;
+  struct inferior *to_detach;
 
   /* follow-fork child, detach-on-fork on.  */
 
@@ -982,7 +983,17 @@ handle_vfork_child_exec_or_exit (int exec)
  }
     }
 
-  target_detach (inf->vfork_parent, 0);
+  /* Now that the vfork child has terminated, make sure during detach
+     that we no longer consider the vfork parent to be a vfork parent,
+     but just a regular process that we're detaching from.  If not, on
+     linux we would avoid waiting for threads to stop in
+     linux-nat.c:stop_wait_callback, while that was only necessary when
+     the vfork child was still active.  */
+  to_detach = inf->vfork_parent;
+  inf->vfork_parent->vfork_child = NULL;
+  inf->vfork_parent = NULL;
+
+  target_detach (to_detach, 0);
 
   /* Put it back.  */
   inf->pspace = pspace;
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child.c b/gdb/testsuite/gdb.threads/vfork-follow-child.c
new file mode 100644
index 0000000000..42c76edd01
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child.c
@@ -0,0 +1,19 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+
+static void *
+f (void *arg)
+{
+  vfork ();
+  return NULL;
+}
+
+int
+main (void)
+{
+  pthread_t tid;
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child.exp b/gdb/testsuite/gdb.threads/vfork-follow-child.exp
new file mode 100644
index 0000000000..26896975af
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child.exp
@@ -0,0 +1,21 @@
+if { ! [istarget "*-*-linux*"] } {
+    return 0
+}
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable {debug}] != "" } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if ![runto_main] then {
+   fail "can't run to main"
+   return 0
+}
+
+gdb_test "set follow-fork-mode child"
+
+gdb_test "continue" "" "continue"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Handle vfork in thread with follow-fork-mode child

Pedro Alves-7
On 4/16/19 4:06 PM, Tom de Vries wrote:
> Hi,

Hi!

Comments below.  As I was reviewing this, I kept experimenting,
so I ended up addressing my own comments myself.  See updated patch
at the bottom.

>
> When debugging the test-case vfork-follow-child.c (which does a vfork in a
> thread) with follow-fork-mode child set, we run into this assertion:
> ...
> src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
>   void x86_linux_update_debug_registers(lwp_info*): \
>   Assertion `lwp_is_stopped (lwp)' failed.
> ...
>
> The assert is caused by the following: the event that the vfork child exits,
> is handled by handle_vfork_child_exec_or_exit, which calls target_detach to
> detach from the vfork parent.  During target_detach we call
> linux_nat_target::detach, which:
> - stops all the threads
> - waits for all the threads to be stopped
> - detaches all the threads.
> However, during the second step we run into this code in stop_wait_callback:
> ...
>   /* If this is a vfork parent, bail out, it is not going to report
>      any SIGSTOP until the vfork is done with.  */
>   if (inf->vfork_child != NULL)
>     return 0;
> ...
> and we don't wait for the threads to be stopped, which resulting in this

"which results"

> assert in x86_linux_update_debug_registers triggering during the third step:
> ...
>   gdb_assert (lwp_is_stopped (lwp));
> ...
>
> Fix this by resetting the vfork parent's vfork_child field before calling
> target_detach in handle_vfork_child_exec_or_exit.
>
> Tested on x86_64-linux, using native and native-gdbserver.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> [gdb] Handle vfork in thread with follow-fork-mode child
>
> gdb/ChangeLog:
>
> 2019-04-16  Tom de Vries  <[hidden email]>
>
> PR gdb/24454
> * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
> avfork_child field before calling target_detach.

Typo "avfork_child".

>
> gdb/testsuite/ChangeLog:
>
> 2019-04-16  Tom de Vries  <[hidden email]>
>
> PR gdb/24454
> * gdb.threads/vfork-follow-child.c: New test.
> * gdb.threads/vfork-follow-child.exp: New file.
>
> ---
>  gdb/infrun.c                                     | 13 ++++++++++++-
>  gdb/testsuite/gdb.threads/vfork-follow-child.c   | 19 +++++++++++++++++++
>  gdb/testsuite/gdb.threads/vfork-follow-child.exp | 21 +++++++++++++++++++++
>  3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 37713b24fe..b088138250 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -923,6 +923,7 @@ handle_vfork_child_exec_or_exit (int exec)
>    struct thread_info *tp;
>    struct program_space *pspace;
>    struct address_space *aspace;
> +  struct inferior *to_detach;
>  
>    /* follow-fork child, detach-on-fork on.  */
>  
> @@ -982,7 +983,17 @@ handle_vfork_child_exec_or_exit (int exec)
>   }
>      }
>  
> -  target_detach (inf->vfork_parent, 0);
> +  /* Now that the vfork child has terminated, make sure during detach

This path is also reached if the vfork child execs, so the reference to
"terminated" above would better be exec.  But also, the other paths in
the function already clear vfork_parent/vfork_child, so I think it's better
to refactor things a bit so that all paths share the code.

> +     that we no longer consider the vfork parent to be a vfork parent,
> +     but just a regular process that we're detaching from.  If not, on
> +     linux we would avoid waiting for threads to stop in
> +     linux-nat.c:stop_wait_callback, while that was only necessary when
> +     the vfork child was still active.  */

I'd rather avoid talking about this deep linux-nat.c implementation detail
here.  There are other target backends, and, linux-nat.c can change as well,
and it'd be easy to leave this comment stale.

> +  to_detach = inf->vfork_parent;
> +  inf->vfork_parent->vfork_child = NULL;
> +  inf->vfork_parent = NULL;
> +
> +  target_detach (to_detach, 0);
>  
>    /* Put it back.  */
>    inf->pspace = pspace;
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child.c b/gdb/testsuite/gdb.threads/vfork-follow-child.c
> new file mode 100644
> index 0000000000..42c76edd01
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child.c
> @@ -0,0 +1,19 @@

Missing copyright headers in all the new test files.

> +#include <stdio.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +static void *
> +f (void *arg)
> +{
> +  vfork ();

vfork children must exit with '_exit', not by returning!

> +  return NULL;
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t tid;
> +  pthread_create (&tid, NULL, f, NULL);
> +  pthread_join (tid, NULL);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child.exp b/gdb/testsuite/gdb.threads/vfork-follow-child.exp
> new file mode 100644
> index 0000000000..26896975af
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child.exp
> @@ -0,0 +1,21 @@

Copyright header.

> +if { ! [istarget "*-*-linux*"] } {
> +    return 0
> +}

I understand that you probably copied this from elsewhere, but I'd
rather just remove it.  Other ports support follow fork/vfork, and
nobody ever remembers to remove these restrictions in test cases.
That's why typically blacklisting is better than whitelisting.
In any case, I think a new supports_follow_fork procedure or
some such, and then use that in all relevant testcases would be
much better than sprinkling around istarget checks.

> +
> +standard_testfile
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> + executable {debug}] != "" } {
> +    return -1
> +}
> +
> +clean_restart ${binfile}

We can use prepare_for_testing.

> +
> +if ![runto_main] then {
> +   fail "can't run to main"
> +   return 0
> +}
> +
> +gdb_test "set follow-fork-mode child"

Here it's better to use gdb_test_no_output.  As you have it,
any output, except a crash, would pass.

> +gdb_test "continue" "" "continue"

Here, I understand that you're only caring for not crashing
gdb, but I think it's good practice to make the test's regex
tighter.  That'll will help with better coverage, helping
future development.

Also, as the name suggests, handle_vfork_child_exec_or_exit
handles both execs and exists similarly, but the new testcase
only handled exit.  I've added a new variant of the testcase
for execs as well.

I've also done some minor tweaks to the commit log.

Here's the updated patch.  WDYT?

From a66460444e1d5a27f82d676b1f2d1b030ff7454e Mon Sep 17 00:00:00 2001
From: Tom de Vries <[hidden email]>
Date: Wed, 17 Apr 2019 17:37:09 +0100
Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child

When debugging any of the testcases added by this commit, which do a
vfork in a thread, with follow-fork-mode child set, we run into this
assertion:

...
src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
  void x86_linux_update_debug_registers(lwp_info*): \
  Assertion `lwp_is_stopped (lwp)' failed.
...

The assert is caused by the following: the vfork-child exit or exec
event is handled by handle_vfork_child_exec_or_exit, which calls
target_detach to detach from the vfork parent.  During target_detach
we call linux_nat_target::detach, which:

#1 - stops all the threads
#2 - waits for all the threads to be stopped
#3 - detaches all the threads

However, during the second step we run into this code in
stop_wait_callback:

...
  /* If this is a vfork parent, bail out, it is not going to report
     any SIGSTOP until the vfork is done with.  */
  if (inf->vfork_child != NULL)
    return 0;
...

and we don't wait for the threads to be stopped, which results in this
assert in x86_linux_update_debug_registers triggering during the third
step:

...
  gdb_assert (lwp_is_stopped (lwp));
...

The fix is to reset the vfork parent's vfork_child field before
calling target_detach in handle_vfork_child_exec_or_exit.  There's
already similar code for the other paths handled by
handle_vfork_child_exec_or_exit, so this commit refactors the code a
bit so that all paths share the same code.

The new tests cover both a vfork child exiting, and a vfork child
execing, since both cases would trigger the assertion.

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

gdb/ChangeLog:
yyyy-mm-dd  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
        vfork_child field before calling target_detach.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * gdb.threads/vfork-follow-child-exec.c: New file.
        * gdb.threads/vfork-follow-child-exec.exp: New file.
        * gdb.threads/vfork-follow-child-exit.c: New file.
        * gdb.threads/vfork-follow-child-exit.exp: New file.
---
 gdb/infrun.c                                       | 27 ++++-----
 .../gdb.threads/vfork-follow-child-exec.c          | 66 ++++++++++++++++++++++
 .../gdb.threads/vfork-follow-child-exec.exp        | 42 ++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.c          | 52 +++++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.exp        | 39 +++++++++++++
 5 files changed, 211 insertions(+), 15 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 37713b24fee..09049ca756a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -915,10 +915,14 @@ handle_vfork_child_exec_or_exit (int exec)
       int resume_parent = -1;
 
       /* This exec or exit marks the end of the shared memory region
- between the parent and the child.  If the user wanted to
- detach from the parent, now is the time.  */
+ between the parent and the child.  Break the bonds.  */
+      inferior *vfork_parent = inf->vfork_parent;
+      inf->vfork_parent->vfork_child = NULL;
+      inf->vfork_parent = NULL;
 
-      if (inf->vfork_parent->pending_detach)
+      /* If the user wanted to detach from the parent, now is the
+ time.  */
+      if (vfork_parent->pending_detach)
  {
   struct thread_info *tp;
   struct program_space *pspace;
@@ -926,7 +930,7 @@ handle_vfork_child_exec_or_exit (int exec)
 
   /* follow-fork child, detach-on-fork on.  */
 
-  inf->vfork_parent->pending_detach = 0;
+  vfork_parent->pending_detach = 0;
 
   gdb::optional<scoped_restore_exited_inferior>
     maybe_restore_inferior;
@@ -941,7 +945,7 @@ handle_vfork_child_exec_or_exit (int exec)
     maybe_restore_thread.emplace ();
 
   /* We're letting loose of the parent.  */
-  tp = any_live_thread_of_inferior (inf->vfork_parent);
+  tp = any_live_thread_of_inferior (vfork_parent);
   switch_to_thread (tp);
 
   /* We're about to detach from the parent, which implicitly
@@ -964,7 +968,7 @@ handle_vfork_child_exec_or_exit (int exec)
   if (print_inferior_events)
     {
       std::string pidstr
- = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
+ = target_pid_to_str (ptid_t (vfork_parent->pid));
 
       target_terminal::ours_for_output ();
 
@@ -982,7 +986,7 @@ handle_vfork_child_exec_or_exit (int exec)
  }
     }
 
-  target_detach (inf->vfork_parent, 0);
+  target_detach (vfork_parent, 0);
 
   /* Put it back.  */
   inf->pspace = pspace;
@@ -998,9 +1002,6 @@ handle_vfork_child_exec_or_exit (int exec)
   set_current_program_space (inf->pspace);
 
   resume_parent = inf->vfork_parent->pid;
-
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
  }
       else
  {
@@ -1034,13 +1035,9 @@ handle_vfork_child_exec_or_exit (int exec)
   inf->pspace = pspace;
   inf->aspace = pspace->aspace;
 
-  resume_parent = inf->vfork_parent->pid;
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
+  resume_parent = vfork_parent->pid;
  }
 
-      inf->vfork_parent = NULL;
-
       gdb_assert (current_program_space == inf->pspace);
 
       if (non_stop && resume_parent != -1)
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
new file mode 100644
index 00000000000..80632d1772f
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
@@ -0,0 +1,66 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <string.h>
+#include <stdlib.h>
+
+static char *program_name;
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      execl (program_name, program_name, "1", NULL);
+      perror ("exec");
+      abort ();
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t tid;
+
+  if (argc > 1)
+    {
+      /* Getting here via execl.  */
+      return 0;
+    }
+
+  program_name = argv[0];
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
new file mode 100644
index 00000000000..18c58d5743f
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
@@ -0,0 +1,42 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that execs, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+if ![runto_main] then {
+   fail "can't run to main"
+   return 0
+}
+
+delete_breakpoints
+
+gdb_test_no_output "set follow-fork-mode child"
+
+gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exec.*" \
+ ".*Inferior 1 .* detached.*" \
+ ".*is executing new program: .*" \
+         ".*Inferior 2 .*exited normally.*"] \
+    "continue"
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
new file mode 100644
index 00000000000..6ae254cce96
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
@@ -0,0 +1,52 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      _exit (0);
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (void)
+{
+  pthread_t tid;
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
new file mode 100644
index 00000000000..9773afcbc11
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
@@ -0,0 +1,39 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that exits, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+if ![runto_main] then {
+   fail "can't run to main"
+   return 0
+}
+
+gdb_test_no_output "set follow-fork-mode child"
+
+gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exit.*" \
+ ".*Inferior 1 .* detached.*" \
+         ".*Inferior 2 .*exited normally.*"] \
+    "continue"
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Handle vfork in thread with follow-fork-mode child

Tom de Vries
On 17-04-19 19:45, Pedro Alves wrote:
> On 4/16/19 4:06 PM, Tom de Vries wrote:
>> Hi,
>
> Hi!
>
> Comments below.  As I was reviewing this, I kept experimenting,
> so I ended up addressing my own comments myself.  See updated patch
> at the bottom.
>

Hi Pedro,

thanks for the comments and updated patch.

>> @@ -982,7 +983,17 @@ handle_vfork_child_exec_or_exit (int exec)
>>   }
>>      }
>>  
>> -  target_detach (inf->vfork_parent, 0);
>> +  /* Now that the vfork child has terminated, make sure during detach
>
> This path is also reached if the vfork child execs, so the reference to
> "terminated" above would better be exec.  But also, the other paths in
> the function already clear vfork_parent/vfork_child, so I think it's better
> to refactor things a bit so that all paths share the code.
>

> +      inf->vfork_parent = NULL;

I was checking this and found dereferences of inf->vfork_parent after it
was set to NULL here:
...
      else if (exec)
        {
          ...
          resume_parent = inf->vfork_parent->pid;
...
and here:
...
      else
        {
          ...
          clone_program_space (pspace, inf->vfork_parent->pspace);
...

To confirm, I did another testrun with aborts at the start of the
blocks, and I found no regressions. So, either this is dead code, or we
need test-cases that trigger these paths.

Thanks,
- Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Handle vfork in thread with follow-fork-mode child

Pedro Alves-7
On 4/18/19 9:02 AM, Tom de Vries wrote:

> On 17-04-19 19:45, Pedro Alves wrote:
>
>> +      inf->vfork_parent = NULL;
>
> I was checking this and found dereferences of inf->vfork_parent after it
> was set to NULL here:
> ...
>       else if (exec)
>         {
>           ...
>           resume_parent = inf->vfork_parent->pid;
> ...
> and here:
> ...
>       else
>         {
>           ...
>           clone_program_space (pspace, inf->vfork_parent->pspace);
> ...
>
> To confirm, I did another testrun with aborts at the start of the
> blocks, and I found no regressions. So, either this is dead code, or we
> need test-cases that trigger these paths.

Indeed, I noticed this yesterday, fixed it & wrote the extra testing, but
then had to leave for the day.  Here is the updated patch with extra
testing, which involved moving the body of the testcases to a procedure,
and then testing with both "set detach-on-fork" "on" and "off".

WDYT?

From 575fecd185d07cd0d2f9d9aed5325e7b09b675e0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <[hidden email]>
Date: Thu, 18 Apr 2019 09:57:45 +0100
Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child

When debugging any of the testcases added by this commit, which do a
vfork in a thread with "set follow-fork-mode child" + "set
detach-on-fork on", we run into this assertion:

...
src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
  void x86_linux_update_debug_registers(lwp_info*): \
  Assertion `lwp_is_stopped (lwp)' failed.
...

The assert is caused by the following: the vfork-child exit or exec
event is handled by handle_vfork_child_exec_or_exit, which calls
target_detach to detach from the vfork parent.  During target_detach
we call linux_nat_target::detach, which:

#1 - stops all the threads
#2 - waits for all the threads to be stopped
#3 - detaches all the threads

However, during the second step we run into this code in
stop_wait_callback:

...
  /* If this is a vfork parent, bail out, it is not going to report
     any SIGSTOP until the vfork is done with.  */
  if (inf->vfork_child != NULL)
    return 0;
...

and we don't wait for the threads to be stopped, which results in this
assert in x86_linux_update_debug_registers triggering during the third
step:

...
  gdb_assert (lwp_is_stopped (lwp));
...

The fix is to reset the vfork parent's vfork_child field before
calling target_detach in handle_vfork_child_exec_or_exit.  There's
already similar code for the other paths handled by
handle_vfork_child_exec_or_exit, so this commit refactors the code a
bit so that all paths share the same code.

The new tests cover both a vfork child exiting, and a vfork child
execing, since both cases would trigger the assertion.

The new testcases also exercise following the vfork children with "set
detach-on-fork off", since it doesn't seem to be tested anywhere.

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

gdb/ChangeLog:
yyyy-mm-dd  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
        vfork_child field before calling target_detach.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * gdb.threads/vfork-follow-child-exec.c: New file.
        * gdb.threads/vfork-follow-child-exec.exp: New file.
        * gdb.threads/vfork-follow-child-exit.c: New file.
        * gdb.threads/vfork-follow-child-exit.exp: New file.
---
 gdb/infrun.c                                       | 31 +++++-----
 .../gdb.threads/vfork-follow-child-exec.c          | 66 ++++++++++++++++++++++
 .../gdb.threads/vfork-follow-child-exec.exp        | 64 +++++++++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.c          | 52 +++++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.exp        | 60 ++++++++++++++++++++
 5 files changed, 256 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
 create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 37713b24fee..37df561de0b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -915,10 +915,14 @@ handle_vfork_child_exec_or_exit (int exec)
       int resume_parent = -1;
 
       /* This exec or exit marks the end of the shared memory region
- between the parent and the child.  If the user wanted to
- detach from the parent, now is the time.  */
+ between the parent and the child.  Break the bonds.  */
+      inferior *vfork_parent = inf->vfork_parent;
+      inf->vfork_parent->vfork_child = NULL;
+      inf->vfork_parent = NULL;
 
-      if (inf->vfork_parent->pending_detach)
+      /* If the user wanted to detach from the parent, now is the
+ time.  */
+      if (vfork_parent->pending_detach)
  {
   struct thread_info *tp;
   struct program_space *pspace;
@@ -926,7 +930,7 @@ handle_vfork_child_exec_or_exit (int exec)
 
   /* follow-fork child, detach-on-fork on.  */
 
-  inf->vfork_parent->pending_detach = 0;
+  vfork_parent->pending_detach = 0;
 
   gdb::optional<scoped_restore_exited_inferior>
     maybe_restore_inferior;
@@ -941,7 +945,7 @@ handle_vfork_child_exec_or_exit (int exec)
     maybe_restore_thread.emplace ();
 
   /* We're letting loose of the parent.  */
-  tp = any_live_thread_of_inferior (inf->vfork_parent);
+  tp = any_live_thread_of_inferior (vfork_parent);
   switch_to_thread (tp);
 
   /* We're about to detach from the parent, which implicitly
@@ -964,7 +968,7 @@ handle_vfork_child_exec_or_exit (int exec)
   if (print_inferior_events)
     {
       std::string pidstr
- = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
+ = target_pid_to_str (ptid_t (vfork_parent->pid));
 
       target_terminal::ours_for_output ();
 
@@ -982,7 +986,7 @@ handle_vfork_child_exec_or_exit (int exec)
  }
     }
 
-  target_detach (inf->vfork_parent, 0);
+  target_detach (vfork_parent, 0);
 
   /* Put it back.  */
   inf->pspace = pspace;
@@ -997,10 +1001,7 @@ handle_vfork_child_exec_or_exit (int exec)
   inf->removable = 1;
   set_current_program_space (inf->pspace);
 
-  resume_parent = inf->vfork_parent->pid;
-
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
+  resume_parent = vfork_parent->pid;
  }
       else
  {
@@ -1030,17 +1031,13 @@ handle_vfork_child_exec_or_exit (int exec)
   set_current_program_space (pspace);
   inf->removable = 1;
   inf->symfile_flags = SYMFILE_NO_READ;
-  clone_program_space (pspace, inf->vfork_parent->pspace);
+  clone_program_space (pspace, vfork_parent->pspace);
   inf->pspace = pspace;
   inf->aspace = pspace->aspace;
 
-  resume_parent = inf->vfork_parent->pid;
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
+  resume_parent = vfork_parent->pid;
  }
 
-      inf->vfork_parent = NULL;
-
       gdb_assert (current_program_space == inf->pspace);
 
       if (non_stop && resume_parent != -1)
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
new file mode 100644
index 00000000000..80632d1772f
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
@@ -0,0 +1,66 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <string.h>
+#include <stdlib.h>
+
+static char *program_name;
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      execl (program_name, program_name, "1", NULL);
+      perror ("exec");
+      abort ();
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t tid;
+
+  if (argc > 1)
+    {
+      /* Getting here via execl.  */
+      return 0;
+    }
+
+  program_name = argv[0];
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
new file mode 100644
index 00000000000..5a28715fa0d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
@@ -0,0 +1,64 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that execs, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+# DETACH indicates whether "set detach-on-fork" is enabled.  It is
+# either "on" or "off".
+
+proc test_vfork {detach} {
+    global binfile
+
+    clean_restart $binfile
+
+    if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+    }
+
+    delete_breakpoints
+
+    gdb_test_no_output "set follow-fork-mode child"
+    gdb_test_no_output "set detach-on-fork $detach"
+
+    if {$detach == "off"} {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".* is executing new program: .*" \
+ ".*Inferior 2 .* exited normally.*"]
+    } else {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exec.*" \
+ ".*Inferior 1 .* detached.*" \
+ ".*is executing new program: .*" \
+ ".*Inferior 2 .*exited normally.*"]
+    }
+}
+
+foreach_with_prefix detach-on-fork {"off" "on"} {
+    test_vfork ${detach-on-fork}
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
new file mode 100644
index 00000000000..6ae254cce96
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
@@ -0,0 +1,52 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      _exit (0);
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (void)
+{
+  pthread_t tid;
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
new file mode 100644
index 00000000000..f07215d41c6
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
@@ -0,0 +1,60 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that exits, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+# DETACH indicates whether "set detach-on-fork" is enabled.  It is
+# either "on" or "off".
+
+proc test_vfork {detach} {
+    global binfile
+
+    clean_restart $binfile
+
+    if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+    }
+
+    gdb_test_no_output "set follow-fork-mode child"
+    gdb_test_no_output "set detach-on-fork $detach"
+
+    if {$detach == "off"} {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Inferior 2 .*exited normally.*"]
+    } else {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exit.*" \
+ ".*Inferior 1 .* detached.*" \
+ ".*Inferior 2 .*exited normally.*"]
+    }
+}
+
+foreach_with_prefix detach-on-fork {"off" "on"} {
+    test_vfork ${detach-on-fork}
+}
--
2.14.5


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Handle vfork in thread with follow-fork-mode child

Tom de Vries
On 18-04-19 11:12, Pedro Alves wrote:

> On 4/18/19 9:02 AM, Tom de Vries wrote:
>> On 17-04-19 19:45, Pedro Alves wrote:
>>
>>> +      inf->vfork_parent = NULL;
>>
>> I was checking this and found dereferences of inf->vfork_parent after it
>> was set to NULL here:
>> ...
>>       else if (exec)
>>         {
>>           ...
>>           resume_parent = inf->vfork_parent->pid;
>> ...
>> and here:
>> ...
>>       else
>>         {
>>           ...
>>           clone_program_space (pspace, inf->vfork_parent->pspace);
>> ...
>>
>> To confirm, I did another testrun with aborts at the start of the
>> blocks, and I found no regressions. So, either this is dead code, or we
>> need test-cases that trigger these paths.
>
> Indeed, I noticed this yesterday, fixed it & wrote the extra testing, but
> then had to leave for the day.  Here is the updated patch with extra
> testing, which involved moving the body of the testcases to a procedure,
> and then testing with both "set detach-on-fork" "on" and "off".
>
> WDYT?
>

I've tested this both with native and native-gdbserver and found no
regressions.

I've also verified that both blocks mentioned above are triggered by the
new test-cases.

LGTM.

Thanks,
- Tom

> From 575fecd185d07cd0d2f9d9aed5325e7b09b675e0 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <[hidden email]>
> Date: Thu, 18 Apr 2019 09:57:45 +0100
> Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child
>
> When debugging any of the testcases added by this commit, which do a
> vfork in a thread with "set follow-fork-mode child" + "set
> detach-on-fork on", we run into this assertion:
>
> ...
> src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
>   void x86_linux_update_debug_registers(lwp_info*): \
>   Assertion `lwp_is_stopped (lwp)' failed.
> ...
>
> The assert is caused by the following: the vfork-child exit or exec
> event is handled by handle_vfork_child_exec_or_exit, which calls
> target_detach to detach from the vfork parent.  During target_detach
> we call linux_nat_target::detach, which:
>
> #1 - stops all the threads
> #2 - waits for all the threads to be stopped
> #3 - detaches all the threads
>
> However, during the second step we run into this code in
> stop_wait_callback:
>
> ...
>   /* If this is a vfork parent, bail out, it is not going to report
>      any SIGSTOP until the vfork is done with.  */
>   if (inf->vfork_child != NULL)
>     return 0;
> ...
>
> and we don't wait for the threads to be stopped, which results in this
> assert in x86_linux_update_debug_registers triggering during the third
> step:
>
> ...
>   gdb_assert (lwp_is_stopped (lwp));
> ...
>
> The fix is to reset the vfork parent's vfork_child field before
> calling target_detach in handle_vfork_child_exec_or_exit.  There's
> already similar code for the other paths handled by
> handle_vfork_child_exec_or_exit, so this commit refactors the code a
> bit so that all paths share the same code.
>
> The new tests cover both a vfork child exiting, and a vfork child
> execing, since both cases would trigger the assertion.
>
> The new testcases also exercise following the vfork children with "set
> detach-on-fork off", since it doesn't seem to be tested anywhere.
>
> Tested on x86_64-linux, using native and native-gdbserver.
>
> gdb/ChangeLog:
> yyyy-mm-dd  Tom de Vries  <[hidden email]>
>    Pedro Alves  <[hidden email]>
>
> PR gdb/24454
> * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
> vfork_child field before calling target_detach.
>
> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Tom de Vries  <[hidden email]>
>    Pedro Alves  <[hidden email]>
>
> PR gdb/24454
> * gdb.threads/vfork-follow-child-exec.c: New file.
> * gdb.threads/vfork-follow-child-exec.exp: New file.
> * gdb.threads/vfork-follow-child-exit.c: New file.
> * gdb.threads/vfork-follow-child-exit.exp: New file.
> ---
>  gdb/infrun.c                                       | 31 +++++-----
>  .../gdb.threads/vfork-follow-child-exec.c          | 66 ++++++++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exec.exp        | 64 +++++++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exit.c          | 52 +++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exit.exp        | 60 ++++++++++++++++++++
>  5 files changed, 256 insertions(+), 17 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
>  create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
>  create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
>  create mode 100644 gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 37713b24fee..37df561de0b 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -915,10 +915,14 @@ handle_vfork_child_exec_or_exit (int exec)
>        int resume_parent = -1;
>  
>        /* This exec or exit marks the end of the shared memory region
> - between the parent and the child.  If the user wanted to
> - detach from the parent, now is the time.  */
> + between the parent and the child.  Break the bonds.  */
> +      inferior *vfork_parent = inf->vfork_parent;
> +      inf->vfork_parent->vfork_child = NULL;
> +      inf->vfork_parent = NULL;
>  
> -      if (inf->vfork_parent->pending_detach)
> +      /* If the user wanted to detach from the parent, now is the
> + time.  */
> +      if (vfork_parent->pending_detach)
>   {
>    struct thread_info *tp;
>    struct program_space *pspace;
> @@ -926,7 +930,7 @@ handle_vfork_child_exec_or_exit (int exec)
>  
>    /* follow-fork child, detach-on-fork on.  */
>  
> -  inf->vfork_parent->pending_detach = 0;
> +  vfork_parent->pending_detach = 0;
>  
>    gdb::optional<scoped_restore_exited_inferior>
>      maybe_restore_inferior;
> @@ -941,7 +945,7 @@ handle_vfork_child_exec_or_exit (int exec)
>      maybe_restore_thread.emplace ();
>  
>    /* We're letting loose of the parent.  */
> -  tp = any_live_thread_of_inferior (inf->vfork_parent);
> +  tp = any_live_thread_of_inferior (vfork_parent);
>    switch_to_thread (tp);
>  
>    /* We're about to detach from the parent, which implicitly
> @@ -964,7 +968,7 @@ handle_vfork_child_exec_or_exit (int exec)
>    if (print_inferior_events)
>      {
>        std::string pidstr
> - = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
> + = target_pid_to_str (ptid_t (vfork_parent->pid));
>  
>        target_terminal::ours_for_output ();
>  
> @@ -982,7 +986,7 @@ handle_vfork_child_exec_or_exit (int exec)
>   }
>      }
>  
> -  target_detach (inf->vfork_parent, 0);
> +  target_detach (vfork_parent, 0);
>  
>    /* Put it back.  */
>    inf->pspace = pspace;
> @@ -997,10 +1001,7 @@ handle_vfork_child_exec_or_exit (int exec)
>    inf->removable = 1;
>    set_current_program_space (inf->pspace);
>  
> -  resume_parent = inf->vfork_parent->pid;
> -
> -  /* Break the bonds.  */
> -  inf->vfork_parent->vfork_child = NULL;
> +  resume_parent = vfork_parent->pid;
>   }
>        else
>   {
> @@ -1030,17 +1031,13 @@ handle_vfork_child_exec_or_exit (int exec)
>    set_current_program_space (pspace);
>    inf->removable = 1;
>    inf->symfile_flags = SYMFILE_NO_READ;
> -  clone_program_space (pspace, inf->vfork_parent->pspace);
> +  clone_program_space (pspace, vfork_parent->pspace);
>    inf->pspace = pspace;
>    inf->aspace = pspace->aspace;
>  
> -  resume_parent = inf->vfork_parent->pid;
> -  /* Break the bonds.  */
> -  inf->vfork_parent->vfork_child = NULL;
> +  resume_parent = vfork_parent->pid;
>   }
>  
> -      inf->vfork_parent = NULL;
> -
>        gdb_assert (current_program_space == inf->pspace);
>  
>        if (non_stop && resume_parent != -1)
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
> new file mode 100644
> index 00000000000..80632d1772f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
> @@ -0,0 +1,66 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static char *program_name;
> +
> +static void *
> +f (void *arg)
> +{
> +  int res = vfork ();
> +
> +  if (res == -1)
> +    {
> +      perror ("vfork");
> +      return NULL;
> +    }
> +  else if (res == 0)
> +    {
> +      /* Child.  */
> +      execl (program_name, program_name, "1", NULL);
> +      perror ("exec");
> +      abort ();
> +    }
> +  else
> +    {
> +      /* Parent.  */
> +      return NULL;
> +    }
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  pthread_t tid;
> +
> +  if (argc > 1)
> +    {
> +      /* Getting here via execl.  */
> +      return 0;
> +    }
> +
> +  program_name = argv[0];
> +
> +  pthread_create (&tid, NULL, f, NULL);
> +  pthread_join (tid, NULL);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
> new file mode 100644
> index 00000000000..5a28715fa0d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
> @@ -0,0 +1,64 @@
> +# Copyright (C) 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test following a vfork child that execs, when the vfork parent is a
> +# threaded program, and it's a non-main thread that vforks.
> +
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
> +    return -1
> +}
> +
> +# DETACH indicates whether "set detach-on-fork" is enabled.  It is
> +# either "on" or "off".
> +
> +proc test_vfork {detach} {
> +    global binfile
> +
> +    clean_restart $binfile
> +
> +    if ![runto_main] then {
> + fail "can't run to main"
> + return 0
> +    }
> +
> +    delete_breakpoints
> +
> +    gdb_test_no_output "set follow-fork-mode child"
> +    gdb_test_no_output "set detach-on-fork $detach"
> +
> +    if {$detach == "off"} {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".* is executing new program: .*" \
> + ".*Inferior 2 .* exited normally.*"]
> +    } else {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Detaching vfork parent process .* after child exec.*" \
> + ".*Inferior 1 .* detached.*" \
> + ".*is executing new program: .*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    }
> +}
> +
> +foreach_with_prefix detach-on-fork {"off" "on"} {
> +    test_vfork ${detach-on-fork}
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
> new file mode 100644
> index 00000000000..6ae254cce96
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
> @@ -0,0 +1,52 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +static void *
> +f (void *arg)
> +{
> +  int res = vfork ();
> +
> +  if (res == -1)
> +    {
> +      perror ("vfork");
> +      return NULL;
> +    }
> +  else if (res == 0)
> +    {
> +      /* Child.  */
> +      _exit (0);
> +    }
> +  else
> +    {
> +      /* Parent.  */
> +      return NULL;
> +    }
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t tid;
> +
> +  pthread_create (&tid, NULL, f, NULL);
> +  pthread_join (tid, NULL);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
> new file mode 100644
> index 00000000000..f07215d41c6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
> @@ -0,0 +1,60 @@
> +# Copyright (C) 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test following a vfork child that exits, when the vfork parent is a
> +# threaded program, and it's a non-main thread that vforks.
> +
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
> +    return -1
> +}
> +
> +# DETACH indicates whether "set detach-on-fork" is enabled.  It is
> +# either "on" or "off".
> +
> +proc test_vfork {detach} {
> +    global binfile
> +
> +    clean_restart $binfile
> +
> +    if ![runto_main] then {
> + fail "can't run to main"
> + return 0
> +    }
> +
> +    gdb_test_no_output "set follow-fork-mode child"
> +    gdb_test_no_output "set detach-on-fork $detach"
> +
> +    if {$detach == "off"} {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    } else {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Detaching vfork parent process .* after child exit.*" \
> + ".*Inferior 1 .* detached.*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    }
> +}
> +
> +foreach_with_prefix detach-on-fork {"off" "on"} {
> +    test_vfork ${detach-on-fork}
> +}
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Handle vfork in thread with follow-fork-mode child

Pedro Alves-7
On 4/18/19 4:48 PM, Tom de Vries wrote:

> I've tested this both with native and native-gdbserver and found no
> regressions.
>
> I've also verified that both blocks mentioned above are triggered by the
> new test-cases.
>
> LGTM.
>

Great, I've pushed it in, ...

>> From 575fecd185d07cd0d2f9d9aed5325e7b09b675e0 Mon Sep 17 00:00:00 2001
>> From: Pedro Alves <[hidden email]>
>> Date: Thu, 18 Apr 2019 09:57:45 +0100
>> Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child

... after fixing the authorship back to you.  Looks like squashing in
my fixes made stgit/git reset the author and I hadn't noticed.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

[8.3 backport] Handle vfork in thread with follow-fork-mode child

Tom de Vries
On 18-04-19 18:17, Pedro Alves wrote:

> On 4/18/19 4:48 PM, Tom de Vries wrote:
>
>> I've tested this both with native and native-gdbserver and found no
>> regressions.
>>
>> I've also verified that both blocks mentioned above are triggered by the
>> new test-cases.
>>
>> LGTM.
>>
>
> Great, I've pushed it in, ...
>
>>> From 575fecd185d07cd0d2f9d9aed5325e7b09b675e0 Mon Sep 17 00:00:00 2001
>>> From: Pedro Alves <[hidden email]>
>>> Date: Thu, 18 Apr 2019 09:57:45 +0100
>>> Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child
>
> ... after fixing the authorship back to you.  Looks like squashing in
> my fixes made stgit/git reset the author and I hadn't noticed.
>
Is this assertion fix ok for 8.3.1?

The patch doesn't apply cleanly, but the merge conflict is trivial
(showing here the relevant diff between the patch on master and the
backport one):
...
$ diff -u master.patch bp.patch
-@@ -964,7 +968,7 @@ handle_vfork_child_exec_or_exit (int exec)
+@@ -963,7 +967,7 @@ handle_vfork_child_exec_or_exit (int exec)
          if (print_inferior_events)
            {
-             std::string pidstr
+             const char *pidstr
 -              = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
 +              = target_pid_to_str (ptid_t (vfork_parent->pid));

@@ -164,7 +162,7 @@

        if (non_stop && resume_parent != -1)
...

Build on x86_64-linux, and ran test-cases belonging to the patch.

Thanks,
- Tom

[gdb] Handle vfork in thread with follow-fork-mode child

[ Backport of master commit b73715df01. ]

When debugging any of the testcases added by this commit, which do a
vfork in a thread with "set follow-fork-mode child" + "set
detach-on-fork on", we run into this assertion:

...
src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
  void x86_linux_update_debug_registers(lwp_info*): \
  Assertion `lwp_is_stopped (lwp)' failed.
...

The assert is caused by the following: the vfork-child exit or exec
event is handled by handle_vfork_child_exec_or_exit, which calls
target_detach to detach from the vfork parent.  During target_detach
we call linux_nat_target::detach, which:

However, during the second step we run into this code in
stop_wait_callback:

...
  /* If this is a vfork parent, bail out, it is not going to report
     any SIGSTOP until the vfork is done with.  */
  if (inf->vfork_child != NULL)
    return 0;
...

and we don't wait for the threads to be stopped, which results in this
assert in x86_linux_update_debug_registers triggering during the third
step:

...
  gdb_assert (lwp_is_stopped (lwp));
...

The fix is to reset the vfork parent's vfork_child field before
calling target_detach in handle_vfork_child_exec_or_exit.  There's
already similar code for the other paths handled by
handle_vfork_child_exec_or_exit, so this commit refactors the code a
bit so that all paths share the same code.

The new tests cover both a vfork child exiting, and a vfork child
execing, since both cases would trigger the assertion.

The new testcases also exercise following the vfork children with "set
detach-on-fork off", since it doesn't seem to be tested anywhere.

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

gdb/ChangeLog:
2019-04-18  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
        vfork_child field before calling target_detach.

gdb/testsuite/ChangeLog:
2019-04-18  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR gdb/24454
        * gdb.threads/vfork-follow-child-exec.c: New file.
        * gdb.threads/vfork-follow-child-exec.exp: New file.
        * gdb.threads/vfork-follow-child-exit.c: New file.
        * gdb.threads/vfork-follow-child-exit.exp: New file.

---
 gdb/infrun.c                                       | 31 +++++-----
 gdb/testsuite/ChangeLog                            |  9 +++
 .../gdb.threads/vfork-follow-child-exec.c          | 66 ++++++++++++++++++++++
 .../gdb.threads/vfork-follow-child-exec.exp        | 64 +++++++++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.c          | 52 +++++++++++++++++
 .../gdb.threads/vfork-follow-child-exit.exp        | 60 ++++++++++++++++++++
 6 files changed, 265 insertions(+), 17 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1efb8d5dc7..9d20036fcf 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -914,10 +914,14 @@ handle_vfork_child_exec_or_exit (int exec)
       int resume_parent = -1;
 
       /* This exec or exit marks the end of the shared memory region
- between the parent and the child.  If the user wanted to
- detach from the parent, now is the time.  */
+ between the parent and the child.  Break the bonds.  */
+      inferior *vfork_parent = inf->vfork_parent;
+      inf->vfork_parent->vfork_child = NULL;
+      inf->vfork_parent = NULL;
 
-      if (inf->vfork_parent->pending_detach)
+      /* If the user wanted to detach from the parent, now is the
+ time.  */
+      if (vfork_parent->pending_detach)
  {
   struct thread_info *tp;
   struct program_space *pspace;
@@ -925,7 +929,7 @@ handle_vfork_child_exec_or_exit (int exec)
 
   /* follow-fork child, detach-on-fork on.  */
 
-  inf->vfork_parent->pending_detach = 0;
+  vfork_parent->pending_detach = 0;
 
   gdb::optional<scoped_restore_exited_inferior>
     maybe_restore_inferior;
@@ -940,7 +944,7 @@ handle_vfork_child_exec_or_exit (int exec)
     maybe_restore_thread.emplace ();
 
   /* We're letting loose of the parent.  */
-  tp = any_live_thread_of_inferior (inf->vfork_parent);
+  tp = any_live_thread_of_inferior (vfork_parent);
   switch_to_thread (tp);
 
   /* We're about to detach from the parent, which implicitly
@@ -963,7 +967,7 @@ handle_vfork_child_exec_or_exit (int exec)
   if (print_inferior_events)
     {
       const char *pidstr
- = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
+ = target_pid_to_str (ptid_t (vfork_parent->pid));
 
       target_terminal::ours_for_output ();
 
@@ -981,7 +985,7 @@ handle_vfork_child_exec_or_exit (int exec)
  }
     }
 
-  target_detach (inf->vfork_parent, 0);
+  target_detach (vfork_parent, 0);
 
   /* Put it back.  */
   inf->pspace = pspace;
@@ -996,10 +1000,7 @@ handle_vfork_child_exec_or_exit (int exec)
   inf->removable = 1;
   set_current_program_space (inf->pspace);
 
-  resume_parent = inf->vfork_parent->pid;
-
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
+  resume_parent = vfork_parent->pid;
  }
       else
  {
@@ -1029,17 +1030,13 @@ handle_vfork_child_exec_or_exit (int exec)
   set_current_program_space (pspace);
   inf->removable = 1;
   inf->symfile_flags = SYMFILE_NO_READ;
-  clone_program_space (pspace, inf->vfork_parent->pspace);
+  clone_program_space (pspace, vfork_parent->pspace);
   inf->pspace = pspace;
   inf->aspace = pspace->aspace;
 
-  resume_parent = inf->vfork_parent->pid;
-  /* Break the bonds.  */
-  inf->vfork_parent->vfork_child = NULL;
+  resume_parent = vfork_parent->pid;
  }
 
-      inf->vfork_parent = NULL;
-
       gdb_assert (current_program_space == inf->pspace);
 
       if (non_stop && resume_parent != -1)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ea1f78cc4b..5ef34148d9 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2019-04-18  Tom de Vries  <[hidden email]>
+    Pedro Alves  <[hidden email]>
+
+ PR gdb/24454
+ * gdb.threads/vfork-follow-child-exec.c: New file.
+ * gdb.threads/vfork-follow-child-exec.exp: New file.
+ * gdb.threads/vfork-follow-child-exit.c: New file.
+ * gdb.threads/vfork-follow-child-exit.exp: New file.
+
 2019-04-30  Tom Tromey  <[hidden email]>
 
  PR c++/24470:
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
new file mode 100644
index 0000000000..80632d1772
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
@@ -0,0 +1,66 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <string.h>
+#include <stdlib.h>
+
+static char *program_name;
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      execl (program_name, program_name, "1", NULL);
+      perror ("exec");
+      abort ();
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t tid;
+
+  if (argc > 1)
+    {
+      /* Getting here via execl.  */
+      return 0;
+    }
+
+  program_name = argv[0];
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
new file mode 100644
index 0000000000..5a28715fa0
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
@@ -0,0 +1,64 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that execs, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+# DETACH indicates whether "set detach-on-fork" is enabled.  It is
+# either "on" or "off".
+
+proc test_vfork {detach} {
+    global binfile
+
+    clean_restart $binfile
+
+    if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+    }
+
+    delete_breakpoints
+
+    gdb_test_no_output "set follow-fork-mode child"
+    gdb_test_no_output "set detach-on-fork $detach"
+
+    if {$detach == "off"} {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".* is executing new program: .*" \
+ ".*Inferior 2 .* exited normally.*"]
+    } else {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exec.*" \
+ ".*Inferior 1 .* detached.*" \
+ ".*is executing new program: .*" \
+ ".*Inferior 2 .*exited normally.*"]
+    }
+}
+
+foreach_with_prefix detach-on-fork {"off" "on"} {
+    test_vfork ${detach-on-fork}
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
new file mode 100644
index 0000000000..6ae254cce9
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
@@ -0,0 +1,52 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+
+static void *
+f (void *arg)
+{
+  int res = vfork ();
+
+  if (res == -1)
+    {
+      perror ("vfork");
+      return NULL;
+    }
+  else if (res == 0)
+    {
+      /* Child.  */
+      _exit (0);
+    }
+  else
+    {
+      /* Parent.  */
+      return NULL;
+    }
+}
+
+int
+main (void)
+{
+  pthread_t tid;
+
+  pthread_create (&tid, NULL, f, NULL);
+  pthread_join (tid, NULL);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
new file mode 100644
index 0000000000..f07215d41c
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
@@ -0,0 +1,60 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test following a vfork child that exits, when the vfork parent is a
+# threaded program, and it's a non-main thread that vforks.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
+    return -1
+}
+
+# DETACH indicates whether "set detach-on-fork" is enabled.  It is
+# either "on" or "off".
+
+proc test_vfork {detach} {
+    global binfile
+
+    clean_restart $binfile
+
+    if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+    }
+
+    gdb_test_no_output "set follow-fork-mode child"
+    gdb_test_no_output "set detach-on-fork $detach"
+
+    if {$detach == "off"} {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Inferior 2 .*exited normally.*"]
+    } else {
+ gdb_test "continue" \
+    [multi_line \
+ "Attaching after .* vfork to child .*" \
+ ".*New inferior 2 .*" \
+ ".*Detaching vfork parent process .* after child exit.*" \
+ ".*Inferior 1 .* detached.*" \
+ ".*Inferior 2 .*exited normally.*"]
+    }
+}
+
+foreach_with_prefix detach-on-fork {"off" "on"} {
+    test_vfork ${detach-on-fork}
+}
Reply | Threaded
Open this post in threaded view
|

[PING][8.3 backport] Handle vfork in thread with follow-fork-mode child

Tom de Vries
On 23-07-19 17:04, Tom de Vries wrote:

> On 18-04-19 18:17, Pedro Alves wrote:
>> On 4/18/19 4:48 PM, Tom de Vries wrote:
>>
>>> I've tested this both with native and native-gdbserver and found no
>>> regressions.
>>>
>>> I've also verified that both blocks mentioned above are triggered by the
>>> new test-cases.
>>>
>>> LGTM.
>>>
>> Great, I've pushed it in, ...
>>
>>>> From 575fecd185d07cd0d2f9d9aed5325e7b09b675e0 Mon Sep 17 00:00:00 2001
>>>> From: Pedro Alves <[hidden email]>
>>>> Date: Thu, 18 Apr 2019 09:57:45 +0100
>>>> Subject: [PATCH] [gdb] Handle vfork in thread with follow-fork-mode child
>> ... after fixing the authorship back to you.  Looks like squashing in
>> my fixes made stgit/git reset the author and I hadn't noticed.
>>
> Is this assertion fix ok for 8.3.1?
>

Ping.

Thanks,
- Tom

> The patch doesn't apply cleanly, but the merge conflict is trivial
> (showing here the relevant diff between the patch on master and the
> backport one):
> ...
> $ diff -u master.patch bp.patch
> -@@ -964,7 +968,7 @@ handle_vfork_child_exec_or_exit (int exec)
> +@@ -963,7 +967,7 @@ handle_vfork_child_exec_or_exit (int exec)
>           if (print_inferior_events)
>             {
> -             std::string pidstr
> +             const char *pidstr
>  -              = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
>  +              = target_pid_to_str (ptid_t (vfork_parent->pid));
>
> @@ -164,7 +162,7 @@
>
>         if (non_stop && resume_parent != -1)
> ...
>
> Build on x86_64-linux, and ran test-cases belonging to the patch.
>
> Thanks,
> - Tom
>
>
> 0001-gdb-Handle-vfork-in-thread-with-follow-fork-mode-child.patch
>
> [gdb] Handle vfork in thread with follow-fork-mode child
>
> [ Backport of master commit b73715df01. ]
>
> When debugging any of the testcases added by this commit, which do a
> vfork in a thread with "set follow-fork-mode child" + "set
> detach-on-fork on", we run into this assertion:
>
> ...
> src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
>   void x86_linux_update_debug_registers(lwp_info*): \
>   Assertion `lwp_is_stopped (lwp)' failed.
> ...
>
> The assert is caused by the following: the vfork-child exit or exec
> event is handled by handle_vfork_child_exec_or_exit, which calls
> target_detach to detach from the vfork parent.  During target_detach
> we call linux_nat_target::detach, which:
>
> However, during the second step we run into this code in
> stop_wait_callback:
>
> ...
>   /* If this is a vfork parent, bail out, it is not going to report
>      any SIGSTOP until the vfork is done with.  */
>   if (inf->vfork_child != NULL)
>     return 0;
> ...
>
> and we don't wait for the threads to be stopped, which results in this
> assert in x86_linux_update_debug_registers triggering during the third
> step:
>
> ...
>   gdb_assert (lwp_is_stopped (lwp));
> ...
>
> The fix is to reset the vfork parent's vfork_child field before
> calling target_detach in handle_vfork_child_exec_or_exit.  There's
> already similar code for the other paths handled by
> handle_vfork_child_exec_or_exit, so this commit refactors the code a
> bit so that all paths share the same code.
>
> The new tests cover both a vfork child exiting, and a vfork child
> execing, since both cases would trigger the assertion.
>
> The new testcases also exercise following the vfork children with "set
> detach-on-fork off", since it doesn't seem to be tested anywhere.
>
> Tested on x86_64-linux, using native and native-gdbserver.
>
> gdb/ChangeLog:
> 2019-04-18  Tom de Vries  <[hidden email]>
>    Pedro Alves  <[hidden email]>
>
> PR gdb/24454
> * infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
> vfork_child field before calling target_detach.
>
> gdb/testsuite/ChangeLog:
> 2019-04-18  Tom de Vries  <[hidden email]>
>    Pedro Alves  <[hidden email]>
>
> PR gdb/24454
> * gdb.threads/vfork-follow-child-exec.c: New file.
> * gdb.threads/vfork-follow-child-exec.exp: New file.
> * gdb.threads/vfork-follow-child-exit.c: New file.
> * gdb.threads/vfork-follow-child-exit.exp: New file.
>
> ---
>  gdb/infrun.c                                       | 31 +++++-----
>  gdb/testsuite/ChangeLog                            |  9 +++
>  .../gdb.threads/vfork-follow-child-exec.c          | 66 ++++++++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exec.exp        | 64 +++++++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exit.c          | 52 +++++++++++++++++
>  .../gdb.threads/vfork-follow-child-exit.exp        | 60 ++++++++++++++++++++
>  6 files changed, 265 insertions(+), 17 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 1efb8d5dc7..9d20036fcf 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -914,10 +914,14 @@ handle_vfork_child_exec_or_exit (int exec)
>        int resume_parent = -1;
>  
>        /* This exec or exit marks the end of the shared memory region
> - between the parent and the child.  If the user wanted to
> - detach from the parent, now is the time.  */
> + between the parent and the child.  Break the bonds.  */
> +      inferior *vfork_parent = inf->vfork_parent;
> +      inf->vfork_parent->vfork_child = NULL;
> +      inf->vfork_parent = NULL;
>  
> -      if (inf->vfork_parent->pending_detach)
> +      /* If the user wanted to detach from the parent, now is the
> + time.  */
> +      if (vfork_parent->pending_detach)
>   {
>    struct thread_info *tp;
>    struct program_space *pspace;
> @@ -925,7 +929,7 @@ handle_vfork_child_exec_or_exit (int exec)
>  
>    /* follow-fork child, detach-on-fork on.  */
>  
> -  inf->vfork_parent->pending_detach = 0;
> +  vfork_parent->pending_detach = 0;
>  
>    gdb::optional<scoped_restore_exited_inferior>
>      maybe_restore_inferior;
> @@ -940,7 +944,7 @@ handle_vfork_child_exec_or_exit (int exec)
>      maybe_restore_thread.emplace ();
>  
>    /* We're letting loose of the parent.  */
> -  tp = any_live_thread_of_inferior (inf->vfork_parent);
> +  tp = any_live_thread_of_inferior (vfork_parent);
>    switch_to_thread (tp);
>  
>    /* We're about to detach from the parent, which implicitly
> @@ -963,7 +967,7 @@ handle_vfork_child_exec_or_exit (int exec)
>    if (print_inferior_events)
>      {
>        const char *pidstr
> - = target_pid_to_str (ptid_t (inf->vfork_parent->pid));
> + = target_pid_to_str (ptid_t (vfork_parent->pid));
>  
>        target_terminal::ours_for_output ();
>  
> @@ -981,7 +985,7 @@ handle_vfork_child_exec_or_exit (int exec)
>   }
>      }
>  
> -  target_detach (inf->vfork_parent, 0);
> +  target_detach (vfork_parent, 0);
>  
>    /* Put it back.  */
>    inf->pspace = pspace;
> @@ -996,10 +1000,7 @@ handle_vfork_child_exec_or_exit (int exec)
>    inf->removable = 1;
>    set_current_program_space (inf->pspace);
>  
> -  resume_parent = inf->vfork_parent->pid;
> -
> -  /* Break the bonds.  */
> -  inf->vfork_parent->vfork_child = NULL;
> +  resume_parent = vfork_parent->pid;
>   }
>        else
>   {
> @@ -1029,17 +1030,13 @@ handle_vfork_child_exec_or_exit (int exec)
>    set_current_program_space (pspace);
>    inf->removable = 1;
>    inf->symfile_flags = SYMFILE_NO_READ;
> -  clone_program_space (pspace, inf->vfork_parent->pspace);
> +  clone_program_space (pspace, vfork_parent->pspace);
>    inf->pspace = pspace;
>    inf->aspace = pspace->aspace;
>  
> -  resume_parent = inf->vfork_parent->pid;
> -  /* Break the bonds.  */
> -  inf->vfork_parent->vfork_child = NULL;
> +  resume_parent = vfork_parent->pid;
>   }
>  
> -      inf->vfork_parent = NULL;
> -
>        gdb_assert (current_program_space == inf->pspace);
>  
>        if (non_stop && resume_parent != -1)
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index ea1f78cc4b..5ef34148d9 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,12 @@
> +2019-04-18  Tom de Vries  <[hidden email]>
> +    Pedro Alves  <[hidden email]>
> +
> + PR gdb/24454
> + * gdb.threads/vfork-follow-child-exec.c: New file.
> + * gdb.threads/vfork-follow-child-exec.exp: New file.
> + * gdb.threads/vfork-follow-child-exit.c: New file.
> + * gdb.threads/vfork-follow-child-exit.exp: New file.
> +
>  2019-04-30  Tom Tromey  <[hidden email]>
>  
>   PR c++/24470:
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
> new file mode 100644
> index 0000000000..80632d1772
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.c
> @@ -0,0 +1,66 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static char *program_name;
> +
> +static void *
> +f (void *arg)
> +{
> +  int res = vfork ();
> +
> +  if (res == -1)
> +    {
> +      perror ("vfork");
> +      return NULL;
> +    }
> +  else if (res == 0)
> +    {
> +      /* Child.  */
> +      execl (program_name, program_name, "1", NULL);
> +      perror ("exec");
> +      abort ();
> +    }
> +  else
> +    {
> +      /* Parent.  */
> +      return NULL;
> +    }
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  pthread_t tid;
> +
> +  if (argc > 1)
> +    {
> +      /* Getting here via execl.  */
> +      return 0;
> +    }
> +
> +  program_name = argv[0];
> +
> +  pthread_create (&tid, NULL, f, NULL);
> +  pthread_join (tid, NULL);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
> new file mode 100644
> index 0000000000..5a28715fa0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exec.exp
> @@ -0,0 +1,64 @@
> +# Copyright (C) 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test following a vfork child that execs, when the vfork parent is a
> +# threaded program, and it's a non-main thread that vforks.
> +
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
> +    return -1
> +}
> +
> +# DETACH indicates whether "set detach-on-fork" is enabled.  It is
> +# either "on" or "off".
> +
> +proc test_vfork {detach} {
> +    global binfile
> +
> +    clean_restart $binfile
> +
> +    if ![runto_main] then {
> + fail "can't run to main"
> + return 0
> +    }
> +
> +    delete_breakpoints
> +
> +    gdb_test_no_output "set follow-fork-mode child"
> +    gdb_test_no_output "set detach-on-fork $detach"
> +
> +    if {$detach == "off"} {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".* is executing new program: .*" \
> + ".*Inferior 2 .* exited normally.*"]
> +    } else {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Detaching vfork parent process .* after child exec.*" \
> + ".*Inferior 1 .* detached.*" \
> + ".*is executing new program: .*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    }
> +}
> +
> +foreach_with_prefix detach-on-fork {"off" "on"} {
> +    test_vfork ${detach-on-fork}
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
> new file mode 100644
> index 0000000000..6ae254cce9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.c
> @@ -0,0 +1,52 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +static void *
> +f (void *arg)
> +{
> +  int res = vfork ();
> +
> +  if (res == -1)
> +    {
> +      perror ("vfork");
> +      return NULL;
> +    }
> +  else if (res == 0)
> +    {
> +      /* Child.  */
> +      _exit (0);
> +    }
> +  else
> +    {
> +      /* Parent.  */
> +      return NULL;
> +    }
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t tid;
> +
> +  pthread_create (&tid, NULL, f, NULL);
> +  pthread_join (tid, NULL);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
> new file mode 100644
> index 0000000000..f07215d41c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/vfork-follow-child-exit.exp
> @@ -0,0 +1,60 @@
> +# Copyright (C) 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test following a vfork child that exits, when the vfork parent is a
> +# threaded program, and it's a non-main thread that vforks.
> +
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}]} {
> +    return -1
> +}
> +
> +# DETACH indicates whether "set detach-on-fork" is enabled.  It is
> +# either "on" or "off".
> +
> +proc test_vfork {detach} {
> +    global binfile
> +
> +    clean_restart $binfile
> +
> +    if ![runto_main] then {
> + fail "can't run to main"
> + return 0
> +    }
> +
> +    gdb_test_no_output "set follow-fork-mode child"
> +    gdb_test_no_output "set detach-on-fork $detach"
> +
> +    if {$detach == "off"} {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    } else {
> + gdb_test "continue" \
> +    [multi_line \
> + "Attaching after .* vfork to child .*" \
> + ".*New inferior 2 .*" \
> + ".*Detaching vfork parent process .* after child exit.*" \
> + ".*Inferior 1 .* detached.*" \
> + ".*Inferior 2 .*exited normally.*"]
> +    }
> +}
> +
> +foreach_with_prefix detach-on-fork {"off" "on"} {
> +    test_vfork ${detach-on-fork}
> +}
>
Reply | Threaded
Open this post in threaded view
|

Re: [8.3 backport] Handle vfork in thread with follow-fork-mode child

Pedro Alves-7
In reply to this post by Tom de Vries
On 7/23/19 4:04 PM, Tom de Vries wrote:

> Is this assertion fix ok for 8.3.1?

OK.

Thanks,
Pedro Alves