From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70763 invoked by alias); 20 Dec 2017 17:46:11 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 70746 invoked by uid 89); 20 Dec 2017 17:46:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f193.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=o83u2EzlfK3J+1nuZMI/C8/yETfA7vwz+V0im/pgnX8=; b=cUa836CnPwZJonoJfoMa3+7BzWm5wZ2ERmBr7wi1ifrvZ6qAeRqIbKc+Z/vsI9kwv1 oFqneYI51cZCXY39CvikQm7pMCzKAjRhrW2HDO6xsRRkjJTAmBKUzM+uTfFIA/LGm0op QVMaA834HGOBWF5JHASCKYapuq63rpyNavLopQRkkBSudEz9Q87W3Sf25Ra+IE4+UEI0 newOeXyEl1oGcA8M3IEBvo+lCkSIODNSE1TGQwJmmmzZDov8IB0SGa91BrJyU0qIMW86 sBm0iQ28/ObdjVtx1dyhsk1SvJWJx1t33ypLkIpT/SVgc1bOi5uvivWft925x7vv0TYc oS9w== X-Gm-Message-State: AKGB3mJxcNSGMW8V5Dn9D+AEzqqKcQKg9/qWjZapTOA1WM+pirxIQHyG YYB20hwnh0qVqcZgJdygC/wKxYuAJSw= X-Google-Smtp-Source: ACJfBottF3ySftPTmSKE7bSQPgW05wAy+tdii9c4SWebjrkDBkQhny32/oE1C7IUwj1GX+f+/z/mKQ== X-Received: by 10.200.50.39 with SMTP id x36mr12104788qta.255.1513791965569; Wed, 20 Dec 2017 09:46:05 -0800 (PST) Subject: Re: [PATCH][BZ #11787] Fix stack guard size accounting To: Szabolcs Nagy , GNU C Library Cc: nd@arm.com References: <5A2FE5ED.1080300@arm.com> From: Carlos O'Donell Message-ID: <4d336192-b7b2-256a-d67b-e66897161e34@redhat.com> Date: Wed, 20 Dec 2017 17:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <5A2FE5ED.1080300@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-12/txt/msg00762.txt.bz2 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 > > [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 > > > 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.