Clean up some uses of fork() in tests.

- Fix a few cases where async-signal-unsafe code is executed in a forked
  process pre-execve.

- Ensure that the return value of fork() is always checked.

PiperOrigin-RevId: 228949310
Change-Id: I3096cb7d7394b8d9ab81b0e0245f2060713ef589

1 files changed, 47 insertions, 18 deletions

diff --git a/test/syscalls/linux/ptrace.cc b/test/syscalls/linux/ptrace.cc
index d3b3b8b02..f063cc92b 100644
--- a/test/syscalls/linux/ptrace.cc
+++ b/test/syscalls/linux/ptrace.cc
@@ -34,6 +34,11 @@
 #include "test/util/test_util.h"
 #include "test/util/thread_util.h"
 
+DEFINE_bool(ptrace_test_execve_child, false,
+            "If true, run the "
+            "PtraceExecveTest_Execve_GetRegs_PeekUser_SIGKILL_TraceClone_"
+            "TraceExit child workload.");
+
 namespace gvisor {
 namespace testing {
 
@@ -457,26 +462,18 @@ class PtraceExecveTest : public ::testing::TestWithParam<bool> {
 };
 
 TEST_P(PtraceExecveTest, Execve_GetRegs_PeekUser_SIGKILL_TraceClone_TraceExit) {
+  ExecveArray const owned_child_argv = {"/proc/self/exe",
+                                        "--ptrace_test_execve_child"};
+  char* const* const child_argv = owned_child_argv.get();
+
   pid_t const child_pid = fork();
   if (child_pid == 0) {
-    // In child process.
-
-    // Enable tracing, then raise SIGSTOP and expect our parent to suppress it.
-    TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0);
-    MaybeSave();
-    RaiseSignal(SIGSTOP);
-    MaybeSave();
-
-    // Call execve in a non-leader thread.
-    ExecveArray const owned_child_argv = {"/proc/self/exe"};
-    char* const* const child_argv = owned_child_argv.get();
-    ScopedThread t([&] {
-      execve(child_argv[0], child_argv, /* envp = */ nullptr);
-      TEST_CHECK_MSG(false, "Survived execve? (thread)");
-    });
-    t.Join();
-    TEST_CHECK_MSG(false, "Survived execve? (main)");
-    _exit(1);
+    // In child process. The test relies on calling execve() in a non-leader
+    // thread; pthread_create() isn't async-signal-safe, so the safest way to
+    // do this is to execve() first, then enable tracing and run the expected
+    // child process behavior in the new subprocess.
+    execve(child_argv[0], child_argv, /* envp = */ nullptr);
+    TEST_PCHECK_MSG(false, "Survived execve to test child");
   }
   // In parent process.
   ASSERT_THAT(child_pid, SyscallSucceeds());
@@ -595,6 +592,28 @@ TEST_P(PtraceExecveTest, Execve_GetRegs_PeekUser_SIGKILL_TraceClone_TraceExit) {
       << " status " << status;
 }
 
+[[noreturn]] void RunExecveChild() {
+  // Enable tracing, then raise SIGSTOP and expect our parent to suppress it.
+  TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0);
+  MaybeSave();
+  RaiseSignal(SIGSTOP);
+  MaybeSave();
+
+  // Call execve() in a non-leader thread. As long as execve() succeeds, what
+  // exactly we execve() shouldn't really matter, since the tracer should kill
+  // us after execve() completes.
+  ScopedThread t([&] {
+    ExecveArray const owned_child_argv = {"/proc/self/exe",
+                                          "--this_flag_shouldnt_exist"};
+    char* const* const child_argv = owned_child_argv.get();
+    execve(child_argv[0], child_argv, /* envp = */ nullptr);
+    TEST_PCHECK_MSG(false, "Survived execve? (thread)");
+  });
+  t.Join();
+  TEST_CHECK_MSG(false, "Survived execve? (main)");
+  _exit(1);
+}
+
 INSTANTIATE_TEST_CASE_P(TraceExec, PtraceExecveTest, ::testing::Bool());
 
 // This test has expectations on when syscall-enter/exit-stops occur that are
@@ -946,3 +965,13 @@ TEST(PtraceTest, ERESTART_NoRandomSave) {
 
 }  // namespace testing
 }  // namespace gvisor
+
+int main(int argc, char** argv) {
+  gvisor::testing::TestInit(&argc, &argv);
+
+  if (FLAGS_ptrace_test_execve_child) {
+    gvisor::testing::RunExecveChild();
+  }
+
+  return RUN_ALL_TESTS();
+}