From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 11061248A8 for ; Mon, 22 Apr 2024 01:48:03 +0200 (CEST) Received: (qmail 1692 invoked by uid 550); 21 Apr 2024 23:47:57 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 1657 invoked from network); 21 Apr 2024 23:47:57 -0000 Date: Sun, 21 Apr 2024 19:48:10 -0400 From: Rich Felker To: Michael Forney Cc: musl@lists.openwall.com Message-ID: <20240421234809.GK4163@brightrain.aerifal.cx> References: <3KDMGHI91MHTL.24XCHF6E4X1XG@mforney.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3KDMGHI91MHTL.24XCHF6E4X1XG@mforney.org> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] Alignment attribute in headers On Sat, Apr 20, 2024 at 08:51:35PM -0700, Michael Forney wrote: > I'm looking at changing headers to use C11 alignment specifiers > when available instead of GNU attributes. > > These are used in the following headers: > > arch/loongarch64/bits/signal.h > arch/powerpc/bits/signal.h > arch/powerpc/bits/user.h > arch/powerpc64/bits/signal.h > arch/powerpc64/bits/user.h > arch/riscv32/bits/signal.h > arch/riscv64/bits/signal.h > arch/x32/bits/shm.h > > In some of these cases (powerpc, powerpc64, x32), the attribute is > conditional on __GNUC__, which I think may result in improperly > aligned structs on compilers that don't define this. > > For powerpc/powerpc64 user.h, the attribute is applied to a typedef > of a struct, but alignas can't be used with typedef. I think this > could be fixed by moving the attribute/alignment specifier to the > first (and only) struct member instead. > > Similarly, x32 shm.h uses an attribute after a struct specifier, > which could be made compatible with an alignment specifier in the > same way. Most of these I didn't remember, and were likely added begrudgingly. There's really no point in having alignment specifiers on the sigcontext/mcontext stuff, as it's kernel-allocated. If there are places where explicit padding to match the layout is missing, these are bugs and should be corrected. I tried to get explicit padding put everywhere, asking folks doing new ports to add it (or refrain from deleting it), but it's possible some places were missed. Let's fix them. For all of this, I think it's perfectly acceptable to just use the GNU C attribute and condition it on __GNUC__. Once any missing padding is fixed, the alignment attribute doesn't really matter. > I also noticed that arch/i386/bits/alltypes.h.in uses _Alignas(8) > for C, __attribute__((__aligned__(8))) for GNU C++, and alignas(8) > for non-GNU C++. Looking through commit history, this seems to be > a work around for a gcc 4.7 bug which claims C++11 support but > doesn't offer alignas. Note that max_align_t is only defined for C11 or C++11 and up, so all of the branches of this conditional are assuming one of those conditions is already true. > Do we need to use this same approach for each of the instances above > to handle the three cases (C, GNU C++, non-GNU C++)? > > I see that stdalign.h uses _Alignas conditional on C11 support, but > i386 alltypes.h uses it unconditionally on C. Should i386 alltypes.h > use __attribute__ when __STDC_VERSION__ < 201112L? This is sketchy; the header really should not be used at all with pre-C11. I don't recall why we added it. Maybe to encourage use of the stdalign.h and alignas over the GNU attribute if autoconf detects it's available? I wouldn't be strongly opposed to removing the macro definition for pre-C11. > Something like > > #if __STDC_VERSION__ >= 201112L > /* use _Alignas */ > #elif defined(__cplusplus) && !defined(__GNUC__) > /* use alignas */ > #else > /* use __attribute__((__aligned__(N))) */ > #end For arch-specific bits where the GNUC attribute can be considered part of the psABI requirements, while I don't like this, my leaning is that it's fine to use it directly and ignore the C11 and C++11 versions. I think public interfaces that are not arch-specific should avoid depending on it, but the only one of these is sys/fanotify.h, which is very much a Linuxism feature and it doesn't seem that bad for it to fail on weird compilers. So I'm not sure any of this is much more than a bikeshed topic... Rich