Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 21 Apr 2024 06:54:28 +0200
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Re: Alignment attribute in headers

Am Sat, Apr 20, 2024 at 08:51:35PM -0700 schrieb Michael Forney:
> 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.
>

At least in the case of powerpc, the alignment directives are no-ops
regarding the struct layout. The fields in question are already
correctly aligned in all structures that contain them, all the way up to
ucontext_t. And the only objects of these types that matter usually are
allocated by the kernel on the signal stack, and so the matter is taken
care of there.

There is libucontext, though. It manipulates user-allocated ucontext_ts,
and is therefore dependent on the correct alignment being declared.
Though I don't know if they depend on libc's definitions.

Therefore, and because alignment directives are not portable to all
compilers we target, we should probably insert explicit padding where
necessary, so at least the structure layout is not affected by the
absence of these directives. Because of your prior mail, I can already
report that we need at least
- 12 bytes between uc_sigmask and uc_mcontext in riscv32's ucontext_t
- 8 bytes between uc_sigmask and uc_mcontext in riscv64's ucontext_t

unless I've miscounted.

> 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++)?
>

Normally, public header files need to be compatible with all C and C++
compilers (C89 and C++98), so the cases are
- non-GNU pre-C11 (no support at all)
- GNU-C (__attribute__(__aligned__))
- C11 (_Alignas)
- non-GNU pre-C++11 (no support at all)
- GNU-C++ (__attribute__(__aligned__))
- C++11 (alignas)

We see that non-GNU pre-C11 and non-GNU pre-C++11 fall together, as do
the GNU-C and GNU-C++ cases. And because of the aforementioned bug, we
should probably prefer the GNU-C extension where available, so the logic
comes out to

#ifdef __GNUC__
/* use attribute */
#elif __STDC_VERSION >= 201100L /* I can never remember the month */
/* use _Alignas */
#elif __cplusplus >= 201100L
/* use alignas */
#endif

That should do it.

I think the i386 header file is just wrong.

Ciao,
Markus

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.