[PATCH v2 3/7] nptl: Decorate thread stack on pthread_create

[DJ Delorie]

One spelling fix.
One question but doesn't block

LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> diff --git a/elf/Makefile b/elf/Makefile
> +  tst-decorate-maps \

Ok.

> +
> +$(objpfx)tst-decorate-maps: $(shared-thread-library)

Ok.

> diff --git a/elf/tst-decorate-maps.c b/elf/tst-decorate-maps.c
> +/* Check the VMA name decoration.
> +   Copyright (C) 2023 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 <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/test-driver.h>
> +#include <support/xstdio.h>
> +#include <support/xthread.h>
> +#include <support/xunistd.h>
> +#include <sys/mman.h>

Ok.

> +#ifndef MAP_STACK
> +# define MAP_STACK 0
> +#endif
> +
> +static pthread_barrier_t b;
> +
> +static void *
> +tf (void *closure)
> +{
> +  /* Wait the thread startup, so thread stack is allocated.  */
> +  xpthread_barrier_wait (&b);
> +
> +  /* Wait the test to read the process mapiping.  */

Spelling: "mapping"

> +  xpthread_barrier_wait (&b);
> +
> +  return NULL;
> +}

Ok.

> +struct proc_maps_t
> +{
> +  int n_def_threads;
> +  int n_user_threads;
> +};

Ok.

> +static struct proc_maps_t
> +read_proc_maps (void)
> +{
> +  if (test_verbose)
> +    printf ("=== print process %jd memory mapping ===\n",
> +	    (intmax_t) getpid ());
> +  struct proc_maps_t r = { 0 };

Misleading, but OK.  Should be { 0, 0 }.

> +
> +  FILE *f = xfopen ("/proc/self/maps", "r");
> +  char *line = NULL;
> +  size_t line_len = 0;
> +  while (xgetline (&line, &line_len, f))
> +    {
> +      if (test_verbose)
> +	printf ("%s", line);
> +      if (strstr (line, "[anon: glibc: pthread stack:") != NULL)
> +	r.n_def_threads++;
> +      else if (strstr (line, "[anon: glibc: pthread user stack:") != NULL)
> +	r.n_user_threads++;
> +    }
> +  free (line);
> +  xfclose (f);
> +
> +  if (test_verbose)
> +    printf ("===\n");
> +  return r;
> +}

Ok.

> +static void
> +do_test_threads (bool set_guard)
> +{
> +  enum
> +    {
> +      num_def_threads  = 8,
> +      num_user_threads = 2,
> +      num_threads = num_def_threads + num_user_threads,
> +    };

10 threads total, ok.

> +  xpthread_barrier_init (&b, NULL, num_threads + 1);

Ok.

> +  pthread_t thr[num_threads];
> +  {
> +    int i = 0;
> +    for (; i < num_threads - num_user_threads; i++)
> +      {
> +	pthread_attr_t attr;
> +	xpthread_attr_init (&attr);
> +	/* The guard page is not annotated.  */
> +	if (!set_guard)
> +	  xpthread_attr_setguardsize (&attr, 0);
> +	thr[i] = xpthread_create (&attr, tf, NULL);
> +	xpthread_attr_destroy (&attr);
> +      }

Ok.

> +    for (; i < num_threads; i++)
> +      {
> +	pthread_attr_t attr;
> +	xpthread_attr_init (&attr);
> +	size_t stacksize = support_small_thread_stack_size ();
> +	void *stack = xmmap (0,
> +			     stacksize,
> +			     PROT_READ | PROT_WRITE,
> +			     MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK,
> +			     -1);
> +	xpthread_attr_setstack (&attr, stack, stacksize);
> +	if (!set_guard)
> +	  xpthread_attr_setguardsize (&attr, 0);
> +	thr[i] = xpthread_create (&attr, tf, NULL);
> +	xpthread_attr_destroy (&attr);
> +      }
> +  }

Ok.

> +  /* Wait all threads to finshed statup and stack allocation.  */
> +  xpthread_barrier_wait (&b);
> +
> +  {
> +    struct proc_maps_t r = read_proc_maps ();
> +    TEST_COMPARE (r.n_def_threads, num_def_threads);
> +    TEST_COMPARE (r.n_user_threads, num_user_threads);
> +  }

Ok.

> +  /* Let the threads finish.  */
> +  xpthread_barrier_wait (&b);
> +
> +  for (int i = 0; i < num_threads; i++)
> +    xpthread_join (thr[i]);
> +
> +  {
> +    struct proc_maps_t r = read_proc_maps ();
> +    TEST_COMPARE (r.n_def_threads, 0);
> +    TEST_COMPARE (r.n_user_threads, 0);
> +  }
> +}

Ok.

> +static int
> +do_test (void)
> +{
> +  support_need_proc ("Reads /proc/self/maps to get stack names.");
> +
> +  if (!support_set_vma_name ())
> +    FAIL_UNSUPPORTED ("kernel does not support PR_SET_VMA_ANON_NAME");
> +
> +  do_test_threads (false);
> +  do_test_threads (true);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> +#include <intprops.h>
> +#include <setvmaname.h>

Ok.

> +/* Maximum supported name from initial kernel support, not exported
> +   by user API.  */
> +#define ANON_VMA_NAME_MAX_LEN 80
> +
> +#define SET_STACK_NAME(__prefix, __stack, __stacksize, __tid)		\
> +  ({									\
> +     char __stack_name[sizeof (__prefix) +				\
> +		       INT_BUFSIZE_BOUND (unsigned int)];		\
> +     _Static_assert (sizeof __stack_name <= ANON_VMA_NAME_MAX_LEN,	\
> +		     "VMA name size larger than maximum supported");	\
> +     __snprintf (__stack_name, sizeof (__stack_name), __prefix "%u",	\
> +		 (unsigned int) __tid);					\
> +     __set_vma_name (__stack, __stacksize, __stack_name);		\
> +   })

Ok.

> +/* Add or remove an associated name to the PD VMA stack.  */
> +static void
> +name_stack_maps (struct pthread *pd, bool set)
> +{
> +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK)
> +  void *stack = pd->stackblock + pd->guardsize;
> +#else
> +  void *stack = pd->stackblock;

Do we have any stack-grows-up systems to test this on?

> +#endif
> +  size_t stacksize = pd->stackblock_size - pd->guardsize;
> +
> +  if (!set)
> +    __set_vma_name (stack, stacksize, NULL);
> +  else
> +    {
> +      unsigned int tid = pd->tid;
> +      if (pd->user_stack)
> +	SET_STACK_NAME (" glibc: pthread user stack: ", stack, stacksize, tid);
> +      else
> +	SET_STACK_NAME (" glibc: pthread stack: ", stack, stacksize, tid);
> +    }
> +}

Ok.

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
>    /* Initialize pointers to locale data.  */
>    __ctype_init ();
>  
> +  /* Name the thread stack if kernel supports it.  */
> +  name_stack_maps (pd, true);
> +

Ok.

> @@ -571,6 +574,9 @@ start_thread (void *arg)
>      /* Free the TCB.  */
>      __nptl_free_tcb (pd);
>  
> +  /* Remove the associated name from the thread stack.  */
> +  name_stack_maps (pd, false);
> +
>  out:
>    /* We cannot call '_exit' here.  '_exit' will terminate the process.

Ok.