This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/4] Add a signal-safe malloc replacement
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Andrew Hunter <ahh at google dot com>
- Cc: libc-alpha at sourceware dot org, ppluzhnikov at google dot com, carlos at redhat dot com
- Date: Wed, 11 Dec 2013 11:03:09 +0100
- Subject: Re: [PATCH 2/4] Add a signal-safe malloc replacement
- Authentication-results: sourceware.org; auth=none
- References: <1386273671-13010-1-git-send-email-ahh at google dot com> <1386722143-10513-1-git-send-email-ahh at google dot com> <1386722143-10513-2-git-send-email-ahh at google dot com>
On Tue, Dec 10, 2013 at 04:35:41PM -0800, Andrew Hunter wrote:
> This is patch 2/4 of the effort to make TLS access async-signal-safe.
>
> Add a drop-in replacement for the malloc API that guarantees signal
> safety. The current implementation is extremely simple and not
> particularly efficient; it can be replaced later as needed.
>
> Use this whenever we need to dynamically allocate memory in TLS paths,
> ensuring that we can safely do these allocations from signal handlers.
ok with this.
> ---
> elf/dl-misc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
> elf/dl-tls.c | 31 +++++------
> sysdeps/generic/ldsodefs.h | 6 +++
> 3 files changed, 154 insertions(+), 15 deletions(-)
>
> diff --git a/elf/dl-misc.c b/elf/dl-misc.c
> index 5fc13a4..cec65d0 100644
> --- a/elf/dl-misc.c
> +++ b/elf/dl-misc.c
> @@ -19,6 +19,7 @@
> #include <assert.h>
> #include <fcntl.h>
> #include <ldsodefs.h>
> +#include <libc-symbols.h>
> #include <limits.h>
> #include <link.h>
> #include <stdarg.h>
> @@ -364,3 +365,134 @@ _dl_higher_prime_number (unsigned long int n)
>
> return *low;
> }
> +
> +/* To support accessing TLS variables from signal handlers, we need an
> + async signal safe memory allocator. These routines are never
> + themselves invoked reentrantly (all calls to them are surrounded by
> + signal masks) but may be invoked concurrently from many threads.
> + The current implementation is not particularly performant nor space
> + efficient, but it will be used rarely (and only in binaries that use
> + dlopen.) The API matches that of malloc() and friends. */
> +
> +struct __signal_safe_allocator_header
> +{
> + size_t size;
> + void *start;
> +};
> +
> +void *weak_function
> +__signal_safe_memalign (size_t boundary, size_t size)
> +{
> + struct __signal_safe_allocator_header *header;
> + if (boundary < sizeof (*header))
> + boundary = sizeof (*header);
> +
> + /* Boundary must be a power of two. */
> + if (boundary & (boundary - 1) == 0)
> + return NULL;
> +
> + size_t pg = GLRO (dl_pagesize);
> + size_t padded_size;
> + if (boundary <= pg)
> + {
> + /* We'll get a pointer certainly aligned to boundary, so just
> + add one more boundary-sized chunk to hold the header. */
> + padded_size = roundup (size, boundary) + boundary;
> + }
> + else
> + {
> + /* If we want K pages aligned to a J-page boundary, K+J+1 pages
> + contains at least one such region that isn't directly at the start
> + (so we can place the header.) This is wasteful, but you're the one
> + who wanted 64K-aligned TLS. */
> + padded_size = roundup (size, pg) + boundary + pg;
> + }
> +
> +
> + size_t actual_size = roundup (padded_size, pg);
> + void *actual = mmap (NULL, actual_size, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> + if (actual == MAP_FAILED)
> + return NULL;
> +
> + if (boundary <= pg)
> + {
> + header = actual + boundary - sizeof (*header);
> + }
> + else
> + {
> + intptr_t actual_pg = ((intptr_t) actual) / pg;
> + intptr_t boundary_pg = boundary / pg;
> + intptr_t start_pg = actual_pg + boundary_pg;
> + start_pg -= start_pg % boundary_pg;
> + if (start_pg > (actual_pg + 1))
> + {
> + int ret = munmap (actual, (start_pg - actual_pg - 1) * pg);
> + assert (ret == 0);
> + actual = (void *) ((start_pg - 1) * pg);
> + }
> + char *start = (void *) (start_pg * pg);
> + header = start - sizeof (*header);
> +
> + }
> + header->size = actual_size;
> + header->start = actual;
> + void *ptr = header;
> + ptr += sizeof (*header);
> + if (((intptr_t) ptr) % boundary != 0)
> + _dl_fatal_printf ("__signal_safe_memalign produced incorrect alignment\n");
> + return ptr;
> +}
> +
> +void * weak_function
> +__signal_safe_malloc (size_t size)
> +{
> + return __signal_safe_memalign (1, size);
> +}
> +
> +void weak_function
> +__signal_safe_free (void *ptr)
> +{
> + if (ptr == NULL)
> + return;
> +
> + struct __signal_safe_allocator_header *header = ((char *) ptr) - sizeof (*header);
> + int ret = munmap (header->start, header->size);
> +
> + assert (ret == 0);
> +}
> +
> +void * weak_function
> +__signal_safe_realloc (void *ptr, size_t size)
> +{
> + if (size == 0)
> + {
> + __signal_safe_free (ptr);
> + return NULL;
> + }
> + if (ptr == NULL)
> + return __signal_safe_malloc (size);
> +
> + struct __signal_safe_allocator_header *header = ((char *) ptr) - sizeof (*header);
> + size_t old_size = header->size;
> + if (old_size - sizeof (*header) >= size)
> + return ptr;
> +
> + void *new_ptr = __signal_safe_malloc (size);
> + if (new_ptr == NULL)
> + return NULL;
> +
> + memcpy (new_ptr, ptr, old_size);
> + __signal_safe_free (ptr);
> +
> + return new_ptr;
> +}
> +
> +void * weak_function
> +__signal_safe_calloc (size_t nmemb, size_t size)
> +{
> + void *ptr = __signal_safe_malloc (nmemb * size);
> + if (ptr == NULL)
> + return NULL;
> + return memset (ptr, 0, nmemb * size);
> +}
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 576d9a1..0956b72 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -293,7 +293,7 @@ allocate_dtv (void *result)
> initial set of modules. This should avoid in most cases expansions
> of the dtv. */
> dtv_length = GL(dl_tls_max_dtv_idx) + DTV_SURPLUS;
> - dtv = calloc (dtv_length + 2, sizeof (dtv_t));
> + dtv = __signal_safe_calloc (dtv_length + 2, sizeof (dtv_t));
> if (dtv != NULL)
> {
> /* This is the initial length of the dtv. */
> @@ -479,11 +479,11 @@ _dl_deallocate_tls (void *tcb, bool dealloc_tcb)
> for (size_t cnt = 0; cnt < dtv[-1].counter; ++cnt)
> if (! dtv[1 + cnt].pointer.is_static
> && dtv[1 + cnt].pointer.val != TLS_DTV_UNALLOCATED)
> - free (dtv[1 + cnt].pointer.val);
> + __signal_safe_free (dtv[1 + cnt].pointer.val);
>
> /* The array starts with dtv[-1]. */
> if (dtv != GL(dl_initial_dtv))
> - free (dtv - 1);
> + __signal_safe_free (dtv - 1);
>
> if (dealloc_tcb)
> {
> @@ -525,8 +525,7 @@ static void *
> allocate_and_init (struct link_map *map)
> {
> void *newp;
> -
> - newp = __libc_memalign (map->l_tls_align, map->l_tls_blocksize);
> + newp = __signal_safe_memalign (map->l_tls_align, map->l_tls_blocksize);
> if (newp == NULL)
> oom ();
>
> @@ -596,25 +595,27 @@ _dl_update_slotinfo (unsigned long int req_modid)
> if (gen <= dtv[0].counter)
> continue;
>
> + size_t modid = total + cnt;
> +
> /* If there is no map this means the entry is empty. */
> struct link_map *map = listp->slotinfo[cnt].map;
> if (map == NULL)
> {
> /* If this modid was used at some point the memory
> might still be allocated. */
> - if (! dtv[total + cnt].pointer.is_static
> - && dtv[total + cnt].pointer.val != TLS_DTV_UNALLOCATED)
> + if (dtv[-1].counter >= modid
> + && !dtv[modid].pointer.is_static
> + && dtv[modid].pointer.val != TLS_DTV_UNALLOCATED)
> {
> - free (dtv[total + cnt].pointer.val);
> - dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED;
> + __signal_safe_free (dtv[modid].pointer.val);
> + dtv[modid].pointer.val = TLS_DTV_UNALLOCATED;
> }
>
> continue;
> }
>
> + assert (modid == map->l_tls_modid);
> /* Check whether the current dtv array is large enough. */
> - size_t modid = map->l_tls_modid;
> - assert (total + cnt == modid);
> if (dtv[-1].counter < modid)
> {
> /* Reallocate the dtv. */
> @@ -628,17 +629,17 @@ _dl_update_slotinfo (unsigned long int req_modid)
> {
> /* This is the initial dtv that was allocated
> during rtld startup using the dl-minimal.c
> - malloc instead of the real malloc. We can't
> + malloc instead of the real allocator. We can't
> free it, we have to abandon the old storage. */
>
> - newp = malloc ((2 + newsize) * sizeof (dtv_t));
> + newp = __signal_safe_malloc ((2 + newsize) * sizeof (dtv_t));
> if (newp == NULL)
> oom ();
> memcpy (newp, &dtv[-1], (2 + oldsize) * sizeof (dtv_t));
> }
> else
> {
> - newp = realloc (&dtv[-1],
> + newp = __signal_safe_realloc (&dtv[-1],
> (2 + newsize) * sizeof (dtv_t));
> if (newp == NULL)
> oom ();
> @@ -668,7 +669,7 @@ _dl_update_slotinfo (unsigned long int req_modid)
> deallocate even if it is this dtv entry we are
> supposed to load. The reason is that we call
> memalign and not malloc. */
> - free (dtv[modid].pointer.val);
> + __signal_safe_free (dtv[modid].pointer.val);
>
> /* This module is loaded dynamically- We defer memory
> allocation. */
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 778cf9b..adeece6 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -988,6 +988,12 @@ extern void *_dl_allocate_tls_storage (void)
> extern void *_dl_allocate_tls_init (void *) internal_function;
> rtld_hidden_proto (_dl_allocate_tls_init)
>
> +extern void *__signal_safe_memalign (size_t boundary, size_t size);
> +extern void *__signal_safe_malloc (size_t size);
> +extern void __signal_safe_free (void *ptr);
> +extern void *__signal_safe_realloc (void *ptr, size_t size);
> +extern void *__signal_safe_calloc (size_t nmemb, size_t size);
> +
> /* Deallocate memory allocated with _dl_allocate_tls. */
> extern void _dl_deallocate_tls (void *tcb, bool dealloc_tcb) internal_function;
> rtld_hidden_proto (_dl_deallocate_tls)
> --
> 1.8.5.1