Integrated: 7903670: jextract generates wrong packed struct layout

[Maurizio Cimadamore]

On Mon, 19 Feb 2024 11:05:02 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> The change to refer to other struct layouts by name resulted in an interesting issue with the `pragma pack` support. When jextract simply re-inlined all nested struct layouts into the enclosing struct, it was simply necessary to add as many calls to `withByteAlignment` as needed, to force struct alignment to adhere to the pragma directive.
> 
> But now that nested layouts just refer to a layout declared somewhere else, we can no longer add `withByteAlignment` to the fields of the _nested_ struct. So we run into issues if:
> 
> * the enclosing struct is packed, and
> * the nested struct is declared as non-packed
> 
> Because, in this case, the layout declaration of the nested struct just contains natural alignment constraints. As such, when this layout is referred to by the enclosing struct layout (e.g. in a field), packed alignment constraints are violated. Note also that simply calling `withByteAlignment` on the nested struct is not enough, as the struct alignment can be stricter than the alignment of any of the fields.
> 
> In other words, we'd need to duplicate the nested layout into the packed enclosing struct, and decorate all its fields with the necessary `withByteAlignment` calls. I found this solution not satisfying, as now we'd have _two_ (or more!) different declaration for the layout of a struct: one corresponding to the place where it was declared; another for the place where it was _used_ with stricter alignment constraints. This is bad, because if users tweak the layout of the nested struct in any way (e.g. using big endian order for some of its fields), such changes will _not_ be reflected by the packed layout view.
> 
> For this reason, I've opted for a solution where packed views are generated _dynamically_, with the help of a compact function in the main header class. This allows jextract to pack layouts (of any kind) at will, according to the constraints of the struct that is currently emitting.
> 
> In other words, given this:
> 
> 
> typedef union epoll_data
> {
>   void *ptr;
>   int fd;
>   int u32;
>   long u64;
> } epoll_data_t;
> 
> #pragma pack(1)
> struct epoll_event
> {
>   int events;
>   epoll_data_t data;
> } __EPOLL_PACKED; 
> 
> 
> We now generate:
> 
> 
> private static final GroupLayout $LAYOUT = MemoryLayout.unionLayout(
>         foo_h.C_POINTER.withName("ptr"),
>         foo_h.C_INT.withName("fd"),
>         foo_h.C_INT.withName("u32"),
>         foo_h.C_LONG.withName("u64")
>     ).withName("epoll_data");
> 
> private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
>         foo_h.align(...

This pull request has now been integrated.

Changeset: 3de62f26
Author:    Maurizio Cimadamore <mcimadamore at openjdk.org>
URL:       https://git.openjdk.org/jextract/commit/3de62f262c0ca89c37cf5be852e0c304f16843bd
Stats:     51 lines in 4 files changed: 47 ins; 0 del; 4 mod

7903670: jextract generates wrong packed struct layout

Reviewed-by: jvernee

-------------

PR: https://git.openjdk.org/jextract/pull/218