This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #11787] Fix stack guard size accounting
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Szabolcs Nagy <szabolcs dot nagy at arm dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Cc: nd at arm dot com
- Date: Wed, 20 Dec 2017 09:46:02 -0800
- Subject: Re: [PATCH][BZ #11787] Fix stack guard size accounting
- Authentication-results: sourceware.org; auth=none
- References: <5A2FE5ED.1080300@arm.com>
On 12/12/2017 06:21 AM, Szabolcs Nagy wrote:
> Previously if user requested S stack and G guard when creating a
> thread, the total mapping was S and the actual available stack was
> S - G - static_tls, which is not what the user requested.
>
> This patch fixes the guard size accounting by pretending the user
> requested S + G stack. This way all later logic works out except
> when reporting the user requested stack size (pthread_getattr_np)
> or when computing the minimal stack size (__pthread_get_minstack).
>
> Normally this will increase thread stack allocations by one page.
> TLS accounting is not affected, that will require a separate fix.
>
> 2017-12-12 Szabolcs Nagy <szabolcs.nagy@arm.com>
>
> [BZ #11787]
> * nptl/allocatestack.c (allocate_stack): Add guardsize to stacksize.
> * nptl/nptl-init.c (__pthread_get_minstack): Remove guardsize from
> stacksize.
> * nptl/pthread_getattr_np.c (pthread_getattr_np): Likewise.
LGTM with the refactoring fixed.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
>
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 1cc789319564b468cf07bdb1304b27dc5a91e86f..9525322b1f92bb34aa21dcab28566aecd7434e90 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -532,6 +532,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
> /* Make sure the size of the stack is enough for the guard and
> eventually the thread descriptor. */
> guardsize = (attr->guardsize + pagesize_m1) & ~pagesize_m1;
> + size += guardsize;
OK.
> if (__builtin_expect (size < ((guardsize + __static_tls_size
> + MINIMAL_REST_STACK + pagesize_m1)
> & ~pagesize_m1),
> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index 869e926f17aa218ab0b122eeda935069ef419ae5..e5c0bdfbebbcc501e4135280f09d55d021943363 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -473,8 +473,5 @@ strong_alias (__pthread_initialize_minimal_internal,
> size_t
> __pthread_get_minstack (const pthread_attr_t *attr)
> {
> - struct pthread_attr *iattr = (struct pthread_attr *) attr;
> -
> - return (GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN
> - + iattr->guardsize);
> + return GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN;
> }
We no longer need pthread_attr_t. Please refactor.
> diff --git a/nptl/pthread_getattr_np.c b/nptl/pthread_getattr_np.c
> index 06093b3d9270c2fcaa5af191852c9eca25dd506f..c5f82f8d22c74fd9e2433dff0a0d98d78e272484 100644
> --- a/nptl/pthread_getattr_np.c
> +++ b/nptl/pthread_getattr_np.c
> @@ -57,9 +57,10 @@ pthread_getattr_np (pthread_t thread_id, pthread_attr_t *attr)
> /* The sizes are subject to alignment. */
> if (__glibc_likely (thread->stackblock != NULL))
> {
> - iattr->stacksize = thread->stackblock_size;
> + iattr->stacksize = thread->stackblock_size - thread->guardsize;
OK.
> #if _STACK_GROWS_DOWN
> - iattr->stackaddr = (char *) thread->stackblock + iattr->stacksize;
> + iattr->stackaddr = (char *) thread->stackblock
> + + thread->stackblock_size;
OK.
> #else
> iattr->stackaddr = (char *) thread->stackblock;
> #endif
--
Cheers,
Carlos.