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

Am Tue, Apr 23, 2024 at 05:45:31PM -0700 schrieb Michael Forney:
> 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.
>

I don'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 someone is using something like libucontext,
manipulating ucontexts directly, and allocating them in the static data
section or on the stack. In that case, the alignment is important, or
else the getcontext()/setcontext() call will fail because the vector
register access will fail. A few ABIs have vector registers as preserved
registers (PowerPC with Altivec comes to mind).

Failure would be fine if it had just never worked, but we already have
versions out there that work right now. And updating your libc only to
find new unexplained crashes is not really a good way to spend a work
day.

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

That is also a good point.

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.