diff mbox series

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

Message ID 20210512185312.938871-1-adhemerval.zanella@linaro.org
State Committed
Headers show
Series [v2] nptl: Fix tst-pthread-gdb-attach for ptrace_scope equal 1 | expand

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, 6:53 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 | 59 +++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 17 deletions(-)

Comments

Adhemerval Zanella May 12, 2021, 6:59 p.m. UTC | #1
On 12/05/2021 15:53, Adhemerval Zanella wrote:
> @@ -141,7 +146,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)

This change is not necessary, I will remove it.
Adhemerval Zanella May 14, 2021, 12:48 p.m. UTC | #2
On 12/05/2021 15:53, Adhemerval Zanella wrote:
> 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.

Florian, are this patch ok? The nptl/tst-pthread-gdb-attach failures are
annoying and make is hard to check for regressions.

> ---
>  nptl/tst-pthread-gdb-attach.c | 59 +++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/nptl/tst-pthread-gdb-attach.c b/nptl/tst-pthread-gdb-attach.c
> index 901a120034..3c6eed07f6 100644
> --- a/nptl/tst-pthread-gdb-attach.c
> +++ b/nptl/tst-pthread-gdb-attach.c
> @@ -23,16 +23,21 @@
>  #include <elf.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <signal.h>
>  #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>
>  #include <support/xstdio.h>
>  #include <support/xthread.h>
>  #include <support/xunistd.h>
> +#include <sys/mman.h>
>  #include <unistd.h>
>  
>  /* Starts out as zero, changed to 1 or 2 by the debugger, depending on
> @@ -141,7 +146,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 +154,26 @@ in_subprocess (void)
>    _exit (0);
>  }
>  
> +static void
> +gdb_process (const char *gdb_path, const char *gdbscript, pid_t *tested_pid)
> +{
> +  /* Create a copy of current test to check with gdb.  As the
> +     target_process is a child of this gdb_process, gdb 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);
> +
> +  write_gdbscript (gdbscript, target.pid);
> +  *tested_pid = 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,25 +204,25 @@ 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);
> +  /* 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");
> +  }
>  
>    char *gdbscript;
>    xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript));
> -  write_gdbscript (gdbscript, tested_pid);
> +
> +  /* Run 'gdb' on test subprocess which will be created in gdb_process.
> +     The pid of the subprocess will be written to 'tested_pid'.  */
> +  pid_t *tested_pid = (pid_t *) xmmap (NULL, sizeof (pid_t),
> +                                       PROT_READ | PROT_WRITE,
> +                                       MAP_SHARED | MAP_ANONYMOUS, -1);
>  
>    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);
> -    }
> +    gdb_process (gdb_path, gdbscript, tested_pid);
>  
>    int status;
>    TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid);
> @@ -205,10 +230,10 @@ do_test (void)
>      /* 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);
> +  kill (*tested_pid, SIGKILL);
> +
> +  xmunmap (tested_pid, sizeof (pid_t));
>    free (gdbscript);
>    free (gdb_path);
>    return 0;
>
Florian Weimer May 14, 2021, 3:15 p.m. UTC | #3
* Adhemerval Zanella via Libc-alpha:

> +  /* Run 'gdb' on test subprocess which will be created in gdb_process.
> +     The pid of the subprocess will be written to 'tested_pid'.  */
> +  pid_t *tested_pid = (pid_t *) xmmap (NULL, sizeof (pid_t),
> +                                       PROT_READ | PROT_WRITE,
> +                                       MAP_SHARED | MAP_ANONYMOUS, -1);


We have support_shared_allocate for this.  Rest looks okay.

Thanks,
Florian
diff mbox series

Patch

diff --git a/nptl/tst-pthread-gdb-attach.c b/nptl/tst-pthread-gdb-attach.c
index 901a120034..3c6eed07f6 100644
--- a/nptl/tst-pthread-gdb-attach.c
+++ b/nptl/tst-pthread-gdb-attach.c
@@ -23,16 +23,21 @@ 
 #include <elf.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <signal.h>
 #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>
 #include <support/xstdio.h>
 #include <support/xthread.h>
 #include <support/xunistd.h>
+#include <sys/mman.h>
 #include <unistd.h>
 
 /* Starts out as zero, changed to 1 or 2 by the debugger, depending on
@@ -141,7 +146,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 +154,26 @@  in_subprocess (void)
   _exit (0);
 }
 
+static void
+gdb_process (const char *gdb_path, const char *gdbscript, pid_t *tested_pid)
+{
+  /* Create a copy of current test to check with gdb.  As the
+     target_process is a child of this gdb_process, gdb 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);
+
+  write_gdbscript (gdbscript, target.pid);
+  *tested_pid = 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,25 +204,25 @@  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);
+  /* 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");
+  }
 
   char *gdbscript;
   xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript));
-  write_gdbscript (gdbscript, tested_pid);
+
+  /* Run 'gdb' on test subprocess which will be created in gdb_process.
+     The pid of the subprocess will be written to 'tested_pid'.  */
+  pid_t *tested_pid = (pid_t *) xmmap (NULL, sizeof (pid_t),
+                                       PROT_READ | PROT_WRITE,
+                                       MAP_SHARED | MAP_ANONYMOUS, -1);
 
   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);
-    }
+    gdb_process (gdb_path, gdbscript, tested_pid);
 
   int status;
   TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid);
@@ -205,10 +230,10 @@  do_test (void)
     /* 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);
+  kill (*tested_pid, SIGKILL);
+
+  xmunmap (tested_pid, sizeof (pid_t));
   free (gdbscript);
   free (gdb_path);
   return 0;