Message ID | mvmr3q2l1yr.fsf@hawking.suse.de |
---|---|
State | New, archived |
Headers |
Received: (qmail 110747 invoked by alias); 27 May 2015 11:00:39 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 110734 invoked by uid 89); 27 May 2015 11:00:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx2.suse.de From: Andreas Schwab <schwab@suse.de> To: libc-alpha@sourceware.org Subject: [PATCH] Fix alignment of sem_t for AArch64 ILP32 X-Yow: Ha ha Ha ha Ha ha Ha Ha Ha Ha -- When will I EVER stop HAVING FUN?!! Date: Wed, 27 May 2015 13:00:28 +0200 Message-ID: <mvmr3q2l1yr.fsf@hawking.suse.de> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain |
Commit Message
Andreas Schwab
May 27, 2015, 11 a.m. UTC
sem_t must have the same alignment as struct new_sem. Andreas. * sysdeps/aarch64/nptl/bits/semaphore.h (sem_t) [!__LP64__]: Use long long for __align. --- sysdeps/aarch64/nptl/bits/semaphore.h | 4 ++++ 1 file changed, 4 insertions(+)
Comments
On 05/27/2015 07:00 AM, Andreas Schwab wrote: > sem_t must have the same alignment as struct new_sem. > > Andreas. > > * sysdeps/aarch64/nptl/bits/semaphore.h (sem_t) [!__LP64__]: Use > long long for __align. > --- > sysdeps/aarch64/nptl/bits/semaphore.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/sysdeps/aarch64/nptl/bits/semaphore.h b/sysdeps/aarch64/nptl/bits/semaphore.h > index 047dd4e..d9f4ab6 100644 > --- a/sysdeps/aarch64/nptl/bits/semaphore.h > +++ b/sysdeps/aarch64/nptl/bits/semaphore.h > @@ -31,5 +31,9 @@ > typedef union > { > char __size[__SIZEOF_SEM_T]; > +#ifdef __LP64__ > long int __align; > +#else > + long long int __align; > +#endif > } sem_t; Why not unconditionally use "long long" as the type? Or if you really prefer the different types, just use "__INT64_TYPE__ __align" instead? Either way seems like it makes the code a little more readable, though I'd prefer the former (and it is more friendly to non-gcc compilers). More generally, I'm not sure why we don't just use __attribute__((aligned(8))) on a simple struct holding the __size[] array, rather than playing union games. Obviously this still would have to be per-platform due to __HAVE_64B_ATOMICS, but just explicitly specifying the alignment does seem cleaner. I guess it is arguably more friendly to non-gcc compilers using the public headers to do it the way it has been done traditionally. None of these suggestions change C++ type mangling as far as I can see, so are presumably all safe from an ABI perspective.
On Fri, May 29, 2015 at 3:32 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: > On 05/27/2015 07:00 AM, Andreas Schwab wrote: >> >> sem_t must have the same alignment as struct new_sem. >> >> Andreas. >> >> * sysdeps/aarch64/nptl/bits/semaphore.h (sem_t) [!__LP64__]: Use >> long long for __align. >> --- >> sysdeps/aarch64/nptl/bits/semaphore.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/sysdeps/aarch64/nptl/bits/semaphore.h >> b/sysdeps/aarch64/nptl/bits/semaphore.h >> index 047dd4e..d9f4ab6 100644 >> --- a/sysdeps/aarch64/nptl/bits/semaphore.h >> +++ b/sysdeps/aarch64/nptl/bits/semaphore.h >> @@ -31,5 +31,9 @@ >> typedef union >> { >> char __size[__SIZEOF_SEM_T]; >> +#ifdef __LP64__ >> long int __align; >> +#else >> + long long int __align; >> +#endif >> } sem_t; > > > Why not unconditionally use "long long" as the type? Or if you really > prefer > the different types, just use "__INT64_TYPE__ __align" instead? Either way > seems like it makes the code a little more readable, though I'd prefer > the former (and it is more friendly to non-gcc compilers). > > More generally, I'm not sure why we don't just use > __attribute__((aligned(8))) > on a simple struct holding the __size[] array, rather than playing union > games. > Obviously this still would have to be per-platform due to > __HAVE_64B_ATOMICS, > but just explicitly specifying the alignment does seem cleaner. I guess it > is > arguably more friendly to non-gcc compilers using the public headers to > do it the way it has been done traditionally. > > None of these suggestions change C++ type mangling as far as I can see, > so are presumably all safe from an ABI perspective. Or better yet use __kernel_signed_long type. Thanks, Andrew > > -- > Chris Metcalf, EZChip Semiconductor > http://www.ezchip.com >
Andrew Pinski <pinskia@gmail.com> writes:
> Or better yet use __kernel_signed_long type.
This has nothing to do with the kernel. It's a pure user-space
requirement that sem_t must be 64-bit aligned.
Andreas.
diff --git a/sysdeps/aarch64/nptl/bits/semaphore.h b/sysdeps/aarch64/nptl/bits/semaphore.h index 047dd4e..d9f4ab6 100644 --- a/sysdeps/aarch64/nptl/bits/semaphore.h +++ b/sysdeps/aarch64/nptl/bits/semaphore.h @@ -31,5 +31,9 @@ typedef union { char __size[__SIZEOF_SEM_T]; +#ifdef __LP64__ long int __align; +#else + long long int __align; +#endif } sem_t;