[RFC] pthread_create: Name stacks in /proc/<pid>/maps

Message ID 20230927190826.1805917-1-irogers@google.com
State Superseded
Headers
Series [RFC] pthread_create: Name stacks in /proc/<pid>/maps |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed

Commit Message

Ian Rogers Sept. 27, 2023, 7:08 p.m. UTC
  Linux 4.5 removed thread stack annotations due to the complexity of
computing them:
https://github.com/torvalds/linux/commit/65376df582174ffcec9e6471bf5b0dd79ba05e4a

This untested (other than building) RFC patch uses
PR_SET_VMA_ANON_NAME to name stacks again as part of thread
creation. PR_SET_VMA_ANON_NAME was added in Linux 5.17:
https://github.com/torvalds/linux/commit/9a10064f5625d5572c3626c1516e0bebc6c9fe9b

The patch is intended to create discussion and possibly serve as an
implementation. When I try to test it current I get unrelated
failures and so guidance would be appreciated.

The naming of stacks can be useful in situations like debugging and
profiling, for example, to differentiate stack from heap memory accesses.
---
 include/sys/prctl.h   |  5 +++++
 nptl/allocatestack.c  | 24 ++++++++++++++++++++++++
 nptl/pthread_create.c |  5 +++++
 3 files changed, 34 insertions(+)
  

Comments

Adhemerval Zanella Netto Sept. 27, 2023, 7:35 p.m. UTC | #1
On 27/09/23 16:08, Ian Rogers wrote:
> Linux 4.5 removed thread stack annotations due to the complexity of
> computing them:
> https://github.com/torvalds/linux/commit/65376df582174ffcec9e6471bf5b0dd79ba05e4a
> 
> This untested (other than building) RFC patch uses
> PR_SET_VMA_ANON_NAME to name stacks again as part of thread
> creation. PR_SET_VMA_ANON_NAME was added in Linux 5.17:
> https://github.com/torvalds/linux/commit/9a10064f5625d5572c3626c1516e0bebc6c9fe9b
> 
> The patch is intended to create discussion and possibly serve as an
> implementation. When I try to test it current I get unrelated
> failures and so guidance would be appreciated.
> 
> The naming of stacks can be useful in situations like debugging and
> profiling, for example, to differentiate stack from heap memory accesses.
> ---
>  include/sys/prctl.h   |  5 +++++
>  nptl/allocatestack.c  | 24 ++++++++++++++++++++++++
>  nptl/pthread_create.c |  5 +++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> index d33f3a290e..8968a632d3 100644
> --- a/include/sys/prctl.h
> +++ b/include/sys/prctl.h
> @@ -3,6 +3,11 @@
>  
>  # ifndef _ISOMAC
>  
> +#  ifndef PR_SET_VMA
> +#   define PR_SET_VMA          0x53564d41
> +#   define PR_SET_VMA_ANON_NAME 0
> +#  endif
> +
>  extern int __prctl (int __option, ...);
>  libc_hidden_proto (__prctl)
>  
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index f9d8cdfd08..60ad5c18b0 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -23,6 +23,7 @@
>  #include <unistd.h>
>  #include <sys/mman.h>
>  #include <sys/param.h>
> +#include <sys/prctl.h>
>  #include <dl-sysdep.h>
>  #include <dl-tls.h>
>  #include <tls.h>
> @@ -33,6 +34,7 @@
>  #include <nptl-stack.h>
>  #include <libc-lock.h>
>  #include <tls-internal.h>
> +#include "intprops.h"
>  
>  /* Default alignment of stack.  */
>  #ifndef STACK_ALIGN
> @@ -577,3 +579,25 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
>  
>    return 0;
>  }
> +
> +/* Use PR_SET_VMA_ANON_NAME to name the thread's stack stack:<tid> */
> +static void
> +name_stack_maps (struct pthread *pd, bool set)
> +{
> +  void *stack = THREAD_GETMEM (pd, stackblock);
> +  size_t stacksize = THREAD_GETMEM (pd, stackblock_size);
> +
> +  if (!set)
> +    __prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, stack, stacksize, NULL);

Space after function call. There are two other instances below.

> +  else
> +    {
> +#define STACK_MAPS_PREFIX "stack:"
> +      char stack_name[sizeof(STACK_MAPS_PREFIX) +
> +                      INT_BUFSIZE_BOUND(unsigned int)];
> +
> +      __snprintf(stack_name, sizeof(stack_name), STACK_MAPS_PREFIX "%u",

Space after sizeof.

> +                 (unsigned int)THREAD_GETMEM (pd, tid));
> +      __prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, stack, stacksize, stack_name);
> +#undef STACK_MAPS_PREFIX
> +    }
> +}
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 6a41d50109..7249513a80 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -369,6 +369,9 @@ start_thread (void *arg)
>    /* Initialize pointers to locale data.  */
>    __ctype_init ();
>  
> +  /* Set up /proc/<pid>/maps entry for stack to stack:tid. */
> +  name_stack_maps (pd, true);
> +
>    /* Register rseq TLS to the kernel.  */
>    {
>      bool do_rseq = THREAD_GETMEM (pd, flags) & ATTR_FLAG_DO_RSEQ;
> @@ -571,6 +574,8 @@ start_thread (void *arg)
>      /* Free the TCB.  */
>      __nptl_free_tcb (pd);
>  
> +  /* Release name for /proc/<pid>/maps. */
> +  name_stack_maps (pd, false);
>  out:
>    /* We cannot call '_exit' here.  '_exit' will terminate the process.
>
  

Patch

diff --git a/include/sys/prctl.h b/include/sys/prctl.h
index d33f3a290e..8968a632d3 100644
--- a/include/sys/prctl.h
+++ b/include/sys/prctl.h
@@ -3,6 +3,11 @@ 
 
 # ifndef _ISOMAC
 
+#  ifndef PR_SET_VMA
+#   define PR_SET_VMA          0x53564d41
+#   define PR_SET_VMA_ANON_NAME 0
+#  endif
+
 extern int __prctl (int __option, ...);
 libc_hidden_proto (__prctl)
 
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index f9d8cdfd08..60ad5c18b0 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -23,6 +23,7 @@ 
 #include <unistd.h>
 #include <sys/mman.h>
 #include <sys/param.h>
+#include <sys/prctl.h>
 #include <dl-sysdep.h>
 #include <dl-tls.h>
 #include <tls.h>
@@ -33,6 +34,7 @@ 
 #include <nptl-stack.h>
 #include <libc-lock.h>
 #include <tls-internal.h>
+#include "intprops.h"
 
 /* Default alignment of stack.  */
 #ifndef STACK_ALIGN
@@ -577,3 +579,25 @@  allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 
   return 0;
 }
+
+/* Use PR_SET_VMA_ANON_NAME to name the thread's stack stack:<tid> */
+static void
+name_stack_maps (struct pthread *pd, bool set)
+{
+  void *stack = THREAD_GETMEM (pd, stackblock);
+  size_t stacksize = THREAD_GETMEM (pd, stackblock_size);
+
+  if (!set)
+    __prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, stack, stacksize, NULL);
+  else
+    {
+#define STACK_MAPS_PREFIX "stack:"
+      char stack_name[sizeof(STACK_MAPS_PREFIX) +
+                      INT_BUFSIZE_BOUND(unsigned int)];
+
+      __snprintf(stack_name, sizeof(stack_name), STACK_MAPS_PREFIX "%u",
+                 (unsigned int)THREAD_GETMEM (pd, tid));
+      __prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, stack, stacksize, stack_name);
+#undef STACK_MAPS_PREFIX
+    }
+}
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 6a41d50109..7249513a80 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -369,6 +369,9 @@  start_thread (void *arg)
   /* Initialize pointers to locale data.  */
   __ctype_init ();
 
+  /* Set up /proc/<pid>/maps entry for stack to stack:tid. */
+  name_stack_maps (pd, true);
+
   /* Register rseq TLS to the kernel.  */
   {
     bool do_rseq = THREAD_GETMEM (pd, flags) & ATTR_FLAG_DO_RSEQ;
@@ -571,6 +574,8 @@  start_thread (void *arg)
     /* Free the TCB.  */
     __nptl_free_tcb (pd);
 
+  /* Release name for /proc/<pid>/maps. */
+  name_stack_maps (pd, false);
 out:
   /* We cannot call '_exit' here.  '_exit' will terminate the process.