Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Apr 2024 17:45:31 -0700
From: Michael Forney <mforney@...rney.org>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: Alignment attribute in headers

Rich Felker <dalias@...c.org> wrote:
> 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.

I checked the other instances, and I think the ones that Markus
identified (riscv32/riscv64 bits/signal.h) are the only ones that
are missing padding. I can prepare a patch if Markus doesn't plan
to.

> 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.

If there's really no point to the alignment specifiers for mcontext
sub-structures, should they just be removed completely?

If there is a point to the alignment specifiers, I think the structs
should be defined in such a way that their alignment doesn't depend
on the compiler you used.

My worry is that some application might use these structs as fields
inside their own structs, and save data from a signal handler in
them. Without the alignment specifier, they could potentially have
incorrect offsets, breaking ABI compatibility between GNU C and
non-GNU C compilers.

For x32 struct shm_info, the structure is allocated by the application.
Even if the kernel doesn't care when populating it, it seems plausible
to me that it might be part of a larger struct.

> > 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.

Why ignore the C11/C++11 versions? I know it's somewhat annoying
to handle the different cases, but that seems better to me than to
sacrifice correctness on compilers that don't support the attribute.

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.