Generalize the clone function slightly.

* Allow clone where both the child function and stack are null. It's
obviously wrong to ask to call a function without a stack, but it's not
necessarily wrong to supply no stack if you're also not supplying a
function.

* Reimplement fork in terms of the clone function, rather than using the
clone system call directly.

This is intended as a step towards enabling use of pid namespaces.

Change-Id: I03c89bd1dc540d8b4ed1c8fdf6644290744b9e91

diff --git a/libc/bionic/clone.cpp b/libc/bionic/clone.cpp
index af63977..8281ac8 100644 (file)
--- a/libc/bionic/clone.cpp
+++ b/libc/bionic/clone.cpp
@@ -47,7 +47,7 @@ int clone(int (*fn)(void*), void* child_stack, int flags, void* arg, ...) {
   void* new_tls = NULL;
   int* child_tid = NULL;
 
-  if (!child_stack) {
+  if (fn != nullptr && child_stack == nullptr) {
     errno = EINVAL;
     return -1;
   }
@@ -76,7 +76,16 @@ int clone(int (*fn)(void*), void* child_stack, int flags, void* arg, ...) {
   pid_t parent_pid = self->invalidate_cached_pid();
 
   // Actually do the clone.
-  int clone_result = __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg);
+  int clone_result;
+  if (fn != nullptr) {
+    clone_result = __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg);
+  } else {
+#if defined(__x86_64__) // sys_clone's last two arguments are flipped on x86-64.
+    clone_result = syscall(__NR_clone, flags, child_stack, parent_tid, child_tid, new_tls);
+#else
+    clone_result = syscall(__NR_clone, flags, child_stack, parent_tid, new_tls, child_tid);
+#endif
+  }
 
   // We're the parent, so put our known pid back in place.
   // We leave the child without a cached pid, but:

diff --git a/libc/bionic/fork.cpp b/libc/bionic/fork.cpp
index 6cfc736..ffe94f4 100644 (file)
--- a/libc/bionic/fork.cpp
+++ b/libc/bionic/fork.cpp
@@ -31,8 +31,6 @@
 
 #include "pthread_internal.h"
 
-#define FORK_FLAGS (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD)
-
 int fork() {
   __bionic_atfork_run_prepare();
 
@@ -41,11 +39,13 @@ int fork() {
   // Remember the parent pid and invalidate the cached value while we fork.
   pid_t parent_pid = self->invalidate_cached_pid();
 
-#if defined(__x86_64__) // sys_clone's last two arguments are flipped on x86-64.
-  int result = syscall(__NR_clone, FORK_FLAGS, NULL, NULL, &(self->tid), NULL);
-#else
-  int result = syscall(__NR_clone, FORK_FLAGS, NULL, NULL, NULL, &(self->tid));
-#endif
+  int result = clone(nullptr,
+                     nullptr,
+                     (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD),
+                     nullptr,
+                     nullptr,
+                     nullptr,
+                     &(self->tid));
   if (result == 0) {
     self->set_cached_pid(gettid());
     __bionic_atfork_run_child();

diff --git a/tests/sched_test.cpp b/tests/sched_test.cpp
index 92d6c26..a4cffc0 100644 (file)
--- a/tests/sched_test.cpp
+++ b/tests/sched_test.cpp
@@ -55,7 +55,8 @@ TEST(sched, clone_errno) {
   // Check that our hand-written clone assembler sets errno correctly on failure.
   uintptr_t fake_child_stack[16];
   errno = 0;
-  ASSERT_EQ(-1, clone(NULL, &fake_child_stack[16], CLONE_THREAD, NULL));
+  // If CLONE_THREAD is set, CLONE_SIGHAND must be set too.
+  ASSERT_EQ(-1, clone(child_fn, &fake_child_stack[16], CLONE_THREAD, NULL));
   ASSERT_EQ(EINVAL, errno);
 }