diff mbox series

nptl: Fix tst-pthread-gdb-attach for ptrace_scope equal 1

Message ID 20210512174959.310818-1-adhemerval.zanella@linaro.org
State Superseded
Headers
Series nptl: Fix tst-pthread-gdb-attach for ptrace_scope equal 1 |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella May 12, 2021, 5:49 p.m. UTC 
  This is similar to the fix for elf/tst-pldd (2f9046fb059e94fe25):
it checks ptrace_scope value (values higher than 2 are too restrictive
to allow the test to run) and it rearranges the spawned processes
to make the target process the gdb child.

Checked on x86_64-linux-gnu with ptrace_scope set to 1.
---
 nptl/tst-pthread-gdb-attach.c | 69 ++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 30 deletions(-)

Comments

Florian Weimer May 12, 2021, 5:58 p.m. UTC | #1 
* Adhemerval Zanella:

> +static void
> +gdb_process (void *arg)
> +{
> +  const char *gdb_path = (const char *) arg;

The cast is unnecessary, I think.

> +  /* Create a copy of current test to check with gdb.  As the
> +     target_process is a child of this pldd_process, pldd is also able
> +     to attach to target_process if YAMA is configured to 1 =
> +     "restricted ptrace".  */
> +  struct support_subprocess target = support_subprocess (in_subprocess, NULL);

Stray references to pldd.

> +
> +  char *gdbscript;
> +  xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript));
> +  write_gdbscript (gdbscript, target.pid);
> +
> +  xdup2 (STDOUT_FILENO, STDERR_FILENO);
> +  execl (gdb_path, "gdb", "-nx", "-batch", "-x", gdbscript, NULL);
> +  if (errno == ENOENT)
> +    _exit (EXIT_UNSUPPORTED);
> +  else
> +    _exit (1);
> +}

Is the GDB script file still deleted after these changes?  (The atexit
handlers do not run due to _exit.)  Maybe create it in the parent and
use global variable?

> +  /* Run 'gdb' on test subprocess which will be created in gdb_process.
> +     The pid of the subprocess will be written to target_pid_ptr.  */
> +  struct support_capture_subprocess gdb;
> +  gdb = support_capture_subprocess (gdb_process, gdb_path);
> +  support_capture_subprocess_check (&gdb, "gdb", 0, sc_allow_stdout);
> +  support_capture_subprocess_free (&gdb);

I think this suppresses the output on timeout, so failures will become
rather hard to diagnose.

Thanks,
Florian
diff mbox series

Patch

diff --git a/nptl/tst-pthread-gdb-attach.c b/nptl/tst-pthread-gdb-attach.c
index 901a120034..d315bc9ef2 100644
--- a/nptl/tst-pthread-gdb-attach.c
+++ b/nptl/tst-pthread-gdb-attach.c
@@ -26,7 +26,10 @@ 
 #include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
+#include <support/capture_subprocess.h>
 #include <support/check.h>
+#include <support/xptrace.h>
+#include <support/subprocess.h>
 #include <support/support.h>
 #include <support/temp_file.h>
 #include <support/test-driver.h>
@@ -141,7 +144,7 @@  subprocess_thread (void *closure)
    second thread, waiting for its value to change to 2, and checks
    that the main thread also changed its value to 1.  */
 static void
-in_subprocess (void)
+in_subprocess (void *arg)
 {
   pthread_t thr = xpthread_create (NULL, subprocess_thread, NULL);
   TEST_VERIFY (xpthread_join (thr) == NULL);
@@ -149,6 +152,29 @@  in_subprocess (void)
   _exit (0);
 }
 
+static void
+gdb_process (void *arg)
+{
+  const char *gdb_path = (const char *) arg;
+
+  /* Create a copy of current test to check with gdb.  As the
+     target_process is a child of this pldd_process, pldd is also able
+     to attach to target_process if YAMA is configured to 1 =
+     "restricted ptrace".  */
+  struct support_subprocess target = support_subprocess (in_subprocess, NULL);
+
+  char *gdbscript;
+  xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript));
+  write_gdbscript (gdbscript, target.pid);
+
+  xdup2 (STDOUT_FILENO, STDERR_FILENO);
+  execl (gdb_path, "gdb", "-nx", "-batch", "-x", gdbscript, NULL);
+  if (errno == ENOENT)
+    _exit (EXIT_UNSUPPORTED);
+  else
+    _exit (1);
+}
+
 static int
 do_test (void)
 {
@@ -179,37 +205,20 @@  do_test (void)
     free (threaddb_path);
   }
 
-  pid_t tested_pid = xfork ();
-  if (tested_pid == 0)
-    in_subprocess ();
-  char *tested_pid_string = xasprintf ("%d", tested_pid);
-
-  char *gdbscript;
-  xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript));
-  write_gdbscript (gdbscript, tested_pid);
+  /* Check if our subprocess can be debugged with ptrace.  */
+  {
+    int ptrace_scope = support_ptrace_scope ();
+    if (ptrace_scope >= 2)
+      FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2");
+  }
 
-  pid_t gdb_pid = xfork ();
-  if (gdb_pid == 0)
-    {
-      xdup2 (STDOUT_FILENO, STDERR_FILENO);
-      execl (gdb_path, "gdb", "-nx", "-batch", "-x", gdbscript, NULL);
-      if (errno == ENOENT)
-        _exit (EXIT_UNSUPPORTED);
-      else
-        _exit (1);
-    }
+  /* Run 'gdb' on test subprocess which will be created in gdb_process.
+     The pid of the subprocess will be written to target_pid_ptr.  */
+  struct support_capture_subprocess gdb;
+  gdb = support_capture_subprocess (gdb_process, gdb_path);
+  support_capture_subprocess_check (&gdb, "gdb", 0, sc_allow_stdout);
+  support_capture_subprocess_free (&gdb);
 
-  int status;
-  TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid);
-  if (WIFEXITED (status) && WEXITSTATUS (status) == EXIT_UNSUPPORTED)
-    /* gdb is not installed.  */
-    return EXIT_UNSUPPORTED;
-  TEST_COMPARE (status, 0);
-  TEST_COMPARE (xwaitpid (tested_pid, &status, 0), tested_pid);
-  TEST_COMPARE (status, 0);
-
-  free (tested_pid_string);
-  free (gdbscript);
   free (gdb_path);
   return 0;
 }