[v2,3/8] posix: Consolidate fork implementation

Message ID 20210311195210.3153729-3-adhemerval.zanella@linaro.org
State Superseded
Delegated to: Florian Weimer
Headers
Series [v2,1/8] posix: Consolidate register-atfork |

Commit Message

Adhemerval Zanella March 11, 2021, 7:52 p.m. UTC
  Changes from v1:
* Add the fork_system_setup system hook.
* Move the Linux __fork_generation_pointer out of _Fork to the
  NPTL fork_system_setup hook (called from fork).
* Removed comment about POSIX async-signal-safe requirement for
  fork.
---
The Linux nptl implementation is used as base for generic fork
implementation to handle the internal locks and mutexes.  The
system specific bits are moved a new internal _Fork symbol.

(This new implementation will be used to provide a async-signal-safe
_Fork now that POSIX has clarified that fork might not be
async-signal-safe [1]).

For Hurd it means hat the __nss_database_fork_prepare_parent and
__nss_database_fork_subprocess will be run in a slight different
order.

[1] https://austingroupbugs.net/view.php?id=62
---
 include/unistd.h                      |   2 +
 posix/Makefile                        |   3 +-
 posix/_Fork.c                         |  34 ++++++
 posix/fork.c                          | 112 +++++++++++++++---
 sysdeps/generic/fork.h                |   5 +
 sysdeps/mach/hurd/{fork.c => _Fork.c} |  21 +---
 sysdeps/nptl/_Fork.c                  |  60 ++++++++++
 sysdeps/nptl/fork.c                   | 162 --------------------------
 sysdeps/nptl/fork.h                   |  10 +-
 sysdeps/unix/sysv/linux/arch-fork.h   |   3 +
 10 files changed, 216 insertions(+), 196 deletions(-)
 create mode 100644 posix/_Fork.c
 rename sysdeps/mach/hurd/{fork.c => _Fork.c} (98%)
 create mode 100644 sysdeps/nptl/_Fork.c
 delete mode 100644 sysdeps/nptl/fork.c
  

Patch

diff --git a/include/unistd.h b/include/unistd.h
index 54becbc9eb..5010d75d30 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -135,6 +135,8 @@  libc_hidden_proto (__setresuid)
 libc_hidden_proto (__setresgid)
 extern __pid_t __vfork (void);
 libc_hidden_proto (__vfork)
+extern __pid_t _Fork (void);
+libc_hidden_proto (_Fork);
 extern int __ttyname_r (int __fd, char *__buf, size_t __buflen)
      attribute_hidden;
 extern int __isatty (int __fd) attribute_hidden;
diff --git a/posix/Makefile b/posix/Makefile
index be0c72f0bb..c59190a3f3 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -39,7 +39,7 @@  routines :=								      \
 	times								      \
 	wait waitpid wait3 wait4 waitid					      \
 	alarm sleep pause nanosleep					      \
-	fork vfork _exit register-atfork				      \
+	fork _Fork vfork _exit register-atfork				      \
 	execve fexecve execv execle execl execvp execlp execvpe		      \
 	getpid getppid							      \
 	getuid geteuid getgid getegid getgroups setuid setgid group_member    \
@@ -261,6 +261,7 @@  CFLAGS-execl.os = -fomit-frame-pointer
 CFLAGS-execvp.os = -fomit-frame-pointer
 CFLAGS-execlp.os = -fomit-frame-pointer
 CFLAGS-nanosleep.c += -fexceptions -fasynchronous-unwind-tables
+CFLAGS-fork.c = $(libio-mtsafe)
 
 tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
 		--none random --col --color --colour
diff --git a/posix/_Fork.c b/posix/_Fork.c
new file mode 100644
index 0000000000..4a998c04f1
--- /dev/null
+++ b/posix/_Fork.c
@@ -0,0 +1,34 @@ 
+/* _Fork implementation.  Generic version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <unistd.h>
+
+/* Clone the calling process, creating an exact copy.  Return -1 for errors,
+   0 to the new process, and the process ID of the new process to the
+   old process.
+   Different than fork, this functions is marked as async-signal-safe by
+   POSIX (by Austin Group issue 62).  */
+pid_t
+_Fork (void)
+{
+  __set_errno (ENOSYS);
+  return -1;
+}
+libc_hidden_def (_Fork)
+stub_warning (_Fork)
diff --git a/posix/fork.c b/posix/fork.c
index 05bda04ac5..cc1bdc1232 100644
--- a/posix/fork.c
+++ b/posix/fork.c
@@ -1,4 +1,5 @@ 
-/* Copyright (C) 1991-2021 Free Software Foundation, Inc.
+/* fork - create a child process.
+   Copyright (C) 1991-2021 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -15,20 +16,105 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <unistd.h>
+#include <fork.h>
+#include <libio/libioP.h>
+#include <ldsodefs.h>
+#include <malloc/malloc-internal.h>
+#include <nss/nss_database.h>
+#include <register-atfork.h>
+#include <stdio-lock.h>
+#include <sys/single_threaded.h>
+#include <unwind-link.h>
 
+static void
+fresetlockfiles (void)
+{
+  _IO_ITER i;
+
+  for (i = _IO_iter_begin(); i != _IO_iter_end(); i = _IO_iter_next(i))
+    if ((_IO_iter_file (i)->_flags & _IO_USER_LOCK) == 0)
+      _IO_lock_init (*((_IO_lock_t *) _IO_iter_file(i)->_lock));
+}
 
-/* Clone the calling process, creating an exact copy.
-   Return -1 for errors, 0 to the new process,
-   and the process ID of the new process to the old process.  */
-int
-__fork (void)
+pid_t
+__libc_fork (void)
 {
-  __set_errno (ENOSYS);
-  return -1;
+  /* Determine if we are running multiple threads.  We skip some fork
+     handlers in the single-thread case, to make fork safer to use in
+     signal handlers.  */
+  bool multiple_threads = __libc_single_threaded == 0;
+
+  __run_fork_handlers (atfork_run_prepare, multiple_threads);
+
+  struct nss_database_data nss_database_data;
+
+  /* If we are not running multiple threads, we do not have to
+     preserve lock state.  If fork runs from a signal handler, only
+     async-signal-safe functions can be used in the child.  These data
+     structures are only used by unsafe functions, so their state does
+     not matter if fork was called from a signal handler.  */
+  if (multiple_threads)
+    {
+      call_function_static_weak (__nss_database_fork_prepare_parent,
+				 &nss_database_data);
+
+      _IO_list_lock ();
+
+      /* Acquire malloc locks.  This needs to come last because fork
+	 handlers may use malloc, and the libio list lock has an
+	 indirect malloc dependency as well (via the getdelim
+	 function).  */
+      call_function_static_weak (__malloc_fork_lock_parent);
+    }
+
+  pid_t pid = _Fork ();
+
+  if (pid == 0)
+    {
+      fork_system_setup ();
+
+      /* Reset the lock state in the multi-threaded case.  */
+      if (multiple_threads)
+	{
+	  __libc_unwind_link_after_fork ();
+
+	  /* Release malloc locks.  */
+	  call_function_static_weak (__malloc_fork_unlock_child);
+
+	  /* Reset the file list.  These are recursive mutexes.  */
+	  fresetlockfiles ();
+
+	  /* Reset locks in the I/O code.  */
+	  _IO_list_resetlock ();
+
+	  call_function_static_weak (__nss_database_fork_subprocess,
+				     &nss_database_data);
+	}
+
+      /* Reset the lock the dynamic loader uses to protect its data.  */
+      __rtld_lock_initialize (GL(dl_load_lock));
+
+      /* Run the handlers registered for the child.  */
+      __run_fork_handlers (atfork_run_child, multiple_threads);
+    }
+  else
+    {
+      /* Release acquired locks in the multi-threaded case.  */
+      if (multiple_threads)
+	{
+	  /* Release malloc locks, parent process variant.  */
+	  call_function_static_weak (__malloc_fork_unlock_parent);
+
+	  /* We execute this even if the 'fork' call failed.  */
+	  _IO_list_unlock ();
+	}
+
+      /* Run the handlers registered for the parent.  */
+      __run_fork_handlers (atfork_run_parent, multiple_threads);
+    }
+
+  return pid;
 }
+weak_alias (__libc_fork, __fork)
 libc_hidden_def (__fork)
-stub_warning (fork)
-
-weak_alias (__fork, fork)
+weak_alias (__libc_fork, fork)
diff --git a/sysdeps/generic/fork.h b/sysdeps/generic/fork.h
index 6cc842a425..f7dfa3fdb7 100644
--- a/sysdeps/generic/fork.h
+++ b/sysdeps/generic/fork.h
@@ -25,3 +25,8 @@ 
    You should have received a copy of the GNU Lesser General Public
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
+
+static inline void
+fork_system_setup (void)
+{
+}
diff --git a/sysdeps/mach/hurd/fork.c b/sysdeps/mach/hurd/_Fork.c
similarity index 98%
rename from sysdeps/mach/hurd/fork.c
rename to sysdeps/mach/hurd/_Fork.c
index 1c5299e686..f6a16c4ca5 100644
--- a/sysdeps/mach/hurd/fork.c
+++ b/sysdeps/mach/hurd/_Fork.c
@@ -58,16 +58,13 @@  DEFINE_HOOK (_hurd_fork_parent_hook, (void));
    Return -1 for errors, 0 to the new process,
    and the process ID of the new process to the old process.  */
 pid_t
-__fork (void)
+_Fork (void)
 {
   jmp_buf env;
   pid_t pid;
   size_t i;
   error_t err;
   struct hurd_sigstate *volatile ss;
-  struct nss_database_data nss_database_data;
-
-  __run_fork_handlers (atfork_run_prepare, true);
 
   ss = _hurd_self_sigstate ();
   __spin_lock (&ss->critical_section_lock);
@@ -107,9 +104,6 @@  __fork (void)
       /* Run things that prepare for forking before we create the task.  */
       RUN_HOOK (_hurd_fork_prepare_hook, ());
 
-      call_function_static_weak (__nss_database_fork_prepare_parent,
-				 &nss_database_data);
-
       /* Lock things that want to be locked before we fork.  */
       {
 	void *const *p;
@@ -669,9 +663,6 @@  __fork (void)
       _hurd_malloc_fork_child ();
       call_function_static_weak (__malloc_fork_unlock_child);
 
-      call_function_static_weak (__nss_database_fork_subprocess,
-				 &nss_database_data);
-
       /* Run things that want to run in the child task to set up.  */
       RUN_HOOK (_hurd_fork_child_hook, ());
 
@@ -719,14 +710,6 @@  __fork (void)
 
   _hurd_critical_section_unlock (ss);
 
-  if (!err)
-    {
-      __run_fork_handlers (pid == 0 ? atfork_run_child : atfork_run_parent,
-			   true);
-    }
-
   return err ? __hurd_fail (err) : pid;
 }
-libc_hidden_def (__fork)
-
-weak_alias (__fork, fork)
+libc_hidden_def (_Fork)
diff --git a/sysdeps/nptl/_Fork.c b/sysdeps/nptl/_Fork.c
new file mode 100644
index 0000000000..eb6317e758
--- /dev/null
+++ b/sysdeps/nptl/_Fork.c
@@ -0,0 +1,60 @@ 
+/* _Fork implementation.  Linux version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <arch-fork.h>
+#include <nptl/pthreadP.h>
+
+/* Pointer to the fork generation counter in the thread library.  */
+extern unsigned long int *__fork_generation_pointer attribute_hidden;
+
+pid_t
+_Fork (void)
+{
+  pid_t pid = arch_fork (&THREAD_SELF->tid);
+  if (pid == 0)
+    {
+      struct pthread *self = THREAD_SELF;
+
+      /* Initialize the robust mutex list setting in the kernel which has
+	 been reset during the fork.  We do not check for errors because if
+	 it fails here, it must have failed at process startup as well and
+	 nobody could have used robust mutexes.
+	 Before we do that, we have to clear the list of robust mutexes
+	 because we do not inherit ownership of mutexes from the parent.
+	 We do not have to set self->robust_head.futex_offset since we do
+	 inherit the correct value from the parent.  We do not need to clear
+	 the pending operation because it must have been zero when fork was
+	 called.  */
+#if __PTHREAD_MUTEX_HAVE_PREV
+      self->robust_prev = &self->robust_head;
+#endif
+      self->robust_head.list = &self->robust_head;
+#ifdef SHARED
+      if (__builtin_expect (__libc_pthread_functions_init, 0))
+	PTHFCT_CALL (ptr_set_robust, (self));
+#else
+      extern __typeof (__nptl_set_robust) __nptl_set_robust
+	__attribute__((weak));
+      if (__builtin_expect (__nptl_set_robust != NULL, 0))
+	__nptl_set_robust (self);
+#endif
+    }
+  return pid;
+}
+libc_hidden_def (_Fork)
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
deleted file mode 100644
index f78267b68c..0000000000
--- a/sysdeps/nptl/fork.c
+++ /dev/null
@@ -1,162 +0,0 @@ 
-/* Copyright (C) 2002-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library 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
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <assert.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sysdep.h>
-#include <libio/libioP.h>
-#include <tls.h>
-#include <hp-timing.h>
-#include <ldsodefs.h>
-#include <stdio-lock.h>
-#include <atomic.h>
-#include <nptl/pthreadP.h>
-#include <fork.h>
-#include <arch-fork.h>
-#include <futex-internal.h>
-#include <malloc/malloc-internal.h>
-#include <nss/nss_database.h>
-#include <unwind-link.h>
-#include <sys/single_threaded.h>
-
-static void
-fresetlockfiles (void)
-{
-  _IO_ITER i;
-
-  for (i = _IO_iter_begin(); i != _IO_iter_end(); i = _IO_iter_next(i))
-    if ((_IO_iter_file (i)->_flags & _IO_USER_LOCK) == 0)
-      _IO_lock_init (*((_IO_lock_t *) _IO_iter_file(i)->_lock));
-}
-
-
-pid_t
-__libc_fork (void)
-{
-  pid_t pid;
-
-  /* Determine if we are running multiple threads.  We skip some fork
-     handlers in the single-thread case, to make fork safer to use in
-     signal handlers.  POSIX requires that fork is async-signal-safe,
-     but our current fork implementation is not.  */
-  bool multiple_threads = __libc_single_threaded == 0;
-
-  __run_fork_handlers (atfork_run_prepare, multiple_threads);
-
-  struct nss_database_data nss_database_data;
-
-  /* If we are not running multiple threads, we do not have to
-     preserve lock state.  If fork runs from a signal handler, only
-     async-signal-safe functions can be used in the child.  These data
-     structures are only used by unsafe functions, so their state does
-     not matter if fork was called from a signal handler.  */
-  if (multiple_threads)
-    {
-      call_function_static_weak (__nss_database_fork_prepare_parent,
-				 &nss_database_data);
-
-      _IO_list_lock ();
-
-      /* Acquire malloc locks.  This needs to come last because fork
-	 handlers may use malloc, and the libio list lock has an
-	 indirect malloc dependency as well (via the getdelim
-	 function).  */
-      call_function_static_weak (__malloc_fork_lock_parent);
-    }
-
-  pid = arch_fork (&THREAD_SELF->tid);
-
-  if (pid == 0)
-    {
-      struct pthread *self = THREAD_SELF;
-
-      /* See __pthread_once.  */
-      if (__fork_generation_pointer != NULL)
-	*__fork_generation_pointer += __PTHREAD_ONCE_FORK_GEN_INCR;
-
-      /* Initialize the robust mutex list setting in the kernel which has
-	 been reset during the fork.  We do not check for errors because if
-	 it fails here, it must have failed at process startup as well and
-	 nobody could have used robust mutexes.
-	 Before we do that, we have to clear the list of robust mutexes
-	 because we do not inherit ownership of mutexes from the parent.
-	 We do not have to set self->robust_head.futex_offset since we do
-	 inherit the correct value from the parent.  We do not need to clear
-	 the pending operation because it must have been zero when fork was
-	 called.  */
-#if __PTHREAD_MUTEX_HAVE_PREV
-      self->robust_prev = &self->robust_head;
-#endif
-      self->robust_head.list = &self->robust_head;
-#ifdef SHARED
-      if (__builtin_expect (__libc_pthread_functions_init, 0))
-	PTHFCT_CALL (ptr_set_robust, (self));
-#else
-      extern __typeof (__nptl_set_robust) __nptl_set_robust
-	__attribute__((weak));
-      if (__builtin_expect (__nptl_set_robust != NULL, 0))
-	__nptl_set_robust (self);
-#endif
-
-      /* Reset the lock state in the multi-threaded case.  */
-      if (multiple_threads)
-	{
-	  __libc_unwind_link_after_fork ();
-
-	  /* Release malloc locks.  */
-	  call_function_static_weak (__malloc_fork_unlock_child);
-
-	  /* Reset the file list.  These are recursive mutexes.  */
-	  fresetlockfiles ();
-
-	  /* Reset locks in the I/O code.  */
-	  _IO_list_resetlock ();
-
-	  call_function_static_weak (__nss_database_fork_subprocess,
-				     &nss_database_data);
-	}
-
-      /* Reset the lock the dynamic loader uses to protect its data.  */
-      __rtld_lock_initialize (GL(dl_load_lock));
-
-      /* Run the handlers registered for the child.  */
-      __run_fork_handlers (atfork_run_child, multiple_threads);
-    }
-  else
-    {
-      /* Release acquired locks in the multi-threaded case.  */
-      if (multiple_threads)
-	{
-	  /* Release malloc locks, parent process variant.  */
-	  call_function_static_weak (__malloc_fork_unlock_parent);
-
-	  /* We execute this even if the 'fork' call failed.  */
-	  _IO_list_unlock ();
-	}
-
-      /* Run the handlers registered for the parent.  */
-      __run_fork_handlers (atfork_run_parent, multiple_threads);
-    }
-
-  return pid;
-}
-weak_alias (__libc_fork, __fork)
-libc_hidden_def (__fork)
-weak_alias (__libc_fork, fork)
diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
index 5246754290..c5937530ee 100644
--- a/sysdeps/nptl/fork.h
+++ b/sysdeps/nptl/fork.h
@@ -16,11 +16,19 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <lowlevellock.h>
 #include <register-atfork.h>
+#include <nptl/pthreadP.h>
 
 /* The fork generation counter, defined in libpthread.  */
 extern unsigned long int __fork_generation attribute_hidden;
 
 /* Pointer to the fork generation counter in the thread library.  */
 extern unsigned long int *__fork_generation_pointer attribute_hidden;
+
+static inline void
+fork_system_setup (void)
+{
+  /* See __pthread_once.  */
+  if (__fork_generation_pointer != NULL)
+    *__fork_generation_pointer += __PTHREAD_ONCE_FORK_GEN_INCR;
+}
diff --git a/sysdeps/unix/sysv/linux/arch-fork.h b/sysdeps/unix/sysv/linux/arch-fork.h
index 35d9397ff8..b846da08f9 100644
--- a/sysdeps/unix/sysv/linux/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/arch-fork.h
@@ -19,6 +19,9 @@ 
 #ifndef __ARCH_FORK_H
 #define __ARCH_FORK_H
 
+#include <sysdep.h>
+#include <sched.h>
+#include <signal.h>
 #include <unistd.h>
 
 /* Call the clone syscall with fork semantic.  The CTID address is used