On Mon, May 3, 2021 at 8:42 PM Eric W. Biederman [off-list ref] wrote:

Marco Elver [off-list ref] writes:

quoted

On Mon, 3 May 2021 at 23:04, Eric W. Biederman [off-list ref] wrote:

quoted

"Eric W. Beiderman" [off-list ref] writes:

quoted

From: "Eric W. Biederman" <redacted>

The si_perf code really wants to add a u64 field.  This change enables
that by reorganizing the definition of siginfo_t, so that a 64bit
field can be added without increasing the alignment of other fields.

If you can, it'd be good to have an explanation for this, because it's
not at all obvious -- some future archeologist will wonder how we ever
came up with this definition of siginfo...

(I see the trick here is that before the union would have changed
alignment, introducing padding after the 3 ints -- but now because the
3 ints are inside the union the union's padding no longer adds padding
for these ints.  Perhaps you can explain it better than I can. Also
see below.)

Yes.  The big idea is adding a 64bit field into the second union
in the _sigfault case will increase the alignment of that second
union to 64bit.

In the 64bit case the alignment is already 64bit so it is not an
issue.

In the 32bit case there are 3 ints followed by a pointer.  When the
64bit member is added the alignment of _segfault becomes 64bit.  That
64bit alignment after 3 ints changes the location of the 32bit pointer.

By moving the 3 preceding ints into _segfault that does not happen.



There remains one very subtle issue that I think isn't a problem
but I would appreciate someone else double checking me.


The old definition of siginfo_t on 32bit almost certainly had 32bit
alignment.  With the addition of a 64bit member siginfo_t gains 64bit
alignment.  This difference only matters if the 64bit field is accessed.
Accessing a 64bit field with 32bit alignment will cause unaligned access
exceptions on some (most?) architectures.

For the 64bit field to be accessed the code needs to be recompiled with
the new headers.  Which implies that when everything is recompiled
siginfo_t will become 64bit aligned.


So the change should be safe unless someone is casting something with
32bit alignment into siginfo_t.

How about if someone has a field of type siginfo_t as an element of a
struct? For example:

struct foo {
  int x;
  siginfo_t y;
};

With this change wouldn't the y field move from offset 4 to offset 8?

Peter