From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13961 invoked by alias); 28 Aug 2013 02:13:45 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 13948 invoked by uid 89); 28 Aug 2013 02:13:44 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Aug 2013 02:13:44 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.9 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7S2DW5f003852 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 27 Aug 2013 22:13:33 -0400 Received: from [10.3.113.100] (ovpn-113-100.phx2.redhat.com [10.3.113.100]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r7S2DVRf031064; Tue, 27 Aug 2013 22:13:31 -0400 Message-ID: <521D5CCA.1070300@redhat.com> Date: Wed, 28 Aug 2013 02:13:00 -0000 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Venkataramanan Kumar CC: libc-ports@sourceware.org, Marcus Shawcroft , Marcus Shawcroft , Patch Tracking Subject: Re: [RFC] [PATCH] [Aarch64] : Stack guard support in glibc References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00052.txt.bz2 On 08/26/2013 12:15 AM, Venkataramanan Kumar wrote: > I also checked the canary value and keeps changing from run to run. Is it faster than accessing a global? > glibc.tls.stack.guard.aarch64.diff Please review: http://sourceware.org/glibc/wiki/Contribution%20checklist Please run the entire testsuite and report if there are no regressions. You need a ChangeLog and you should also write up some text for a NEWS entry that will go out with 2.19 talking about the new feature for ARM. > Index: tls.h > =================================================================== > --- tls.h (revision 23742) > +++ tls.h (working copy) > @@ -68,10 +68,15 @@ > # define TLS_TCB_SIZE sizeof (tcbhead_t) > > /* This is the size we need before TCB. */ > -# define TLS_PRE_TCB_SIZE sizeof (struct pthread) > +# define TLS_PRE_TCB_SIZE \ > + (sizeof (struct pthread) \ > + + (PTHREAD_STRUCT_END_PADDING < 2 * sizeof (uintptr_t) \ > + ? ((2 * sizeof (uintptr_t) + __alignof__ (struct pthread) - 1) \ > + & ~(__alignof__ (struct pthread) - 1)) \ > + : 0)) Please use ALIGN_UP or ALIGN_DOWN from libc-internal.h. I know you're copying this from IA64, but it should still use the newer macros. > /* Alignment requirements for the TCB. */ > -# define TLS_TCB_ALIGN __alignof__ (tcbhead_t) > +# define TLS_TCB_ALIGN __alignof__ (struct pthread) > /* Install the dtv pointer. The pointer passed is to the element with > index -1 which contain the length. */ > @@ -98,12 +103,28 @@ > > /* Return the thread descriptor for the current thread. */ > # define THREAD_SELF \ > - ((struct pthread *)__builtin_thread_pointer () - 1) > + ((struct pthread *)((char *) __builtin_thread_pointer () - TLS_PRE_TCB_SIZE)) > > /* Magic for libthread_db to know how to do THREAD_SELF. */ > # define DB_THREAD_SELF \ > - CONST_THREAD_AREA (64, sizeof (struct pthread)) > + CONST_THREAD_AREA (64, TLS_PRE_TCB_SIZE) Did you check to see if this impacts gdb in any way? e.g. Install new glibc into a system, then build and run the gdb testsuite and see if anything fails? > +/* Set the stack guard field in TCB head. */ > +#define THREAD_SET_STACK_GUARD(value) \ > + (((uintptr_t *) __builtin_thread_pointer ())[-1] = (value)) > +#define THREAD_COPY_STACK_GUARD(descr) \ > + (((uintptr_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))[-1] \ > + = ((uintptr_t *) __builtin_thread_pointer ())[-1]) > + > +/* Set the pointer guard field in TCB head. */ > +#define THREAD_GET_POINTER_GUARD() \ > + (((uintptr_t *) __builtin_thread_pointer ())[-2]) > +#define THREAD_SET_POINTER_GUARD(value) \ > + (((uintptr_t *) __builtin_thread_pointer ())[-2] = (value)) > +#define THREAD_COPY_POINTER_GUARD(descr) \ > + (((uintptr_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))[-2] \ > + = THREAD_GET_POINTER_GUARD ()) > + Please see the Style and Conventions: http://sourceware.org/glibc/wiki/Style_and_Conventions#Nested_C_Preprocessor_Directives > /* Access to data in the thread descriptor is easy. */ > # define THREAD_GETMEM(descr, member) \ > descr->member I've only done a very light review here, please repost v2 and we can do a more thorough review. Cheers, Carlos.