RFR: 7903481: Jextract doesn't enforce group layout alignment correctly in some cases

[Maurizio Cimadamore]

On Tue, 30 May 2023 11:49:05 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch overhauls the treatment of pragma packs directives.
> 
> The current logic tries to detect fields occurring at misaligned offsets, and relaxes alignment constraints for these fields.
> However, in cases like this:
> 
> 
> #pragma pack(push, 1)
> struct A {
>    long long a;
>    int b;
> }
> 
> 
> Each field is correctly aligned. But the struct size (12) is not a multiple of its natural alignment (8). As a result, we run into issues when building a sequence layout out of this struct, because of the eager checks added to the layout API.
> 
> This patch fixes support for packed structs "the right way", that is, by asking clang the struct/union alignment, and then making sure that any field is aligned accordingly before the group layout is created.

Note, even with this fix, windows.h does not extract correctly with the latest `panama` branch. This seems to be caused by a bug in the logic with which we visit record fields - either because of a bug in jextract, or because of an issue in libclang itself. Essentially, it is possible for jextract to see struct fields in the "wrong order", which then completely messes up our offset computation logic. More investigation is required to fix that case (esp. to understand if that's a latent issue in jextract code). This issue typically manifests with an exception when a struct layout is created (as the created struct layout has a size that does not conform to the size reported by libclang).

src/main/java/org/openjdk/jextract/impl/TypeImpl.java line 376:

> 374:         try {
> 375:             return Optional.of(getLayoutInternal(t));
> 376:         } catch (UnsupportedOperationException ex) {

These changes are not strictly necessary, but I don't like that we swallow important exceptions and return empty optionals instead.

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

PR Comment: https://git.openjdk.org/jextract/pull/121#issuecomment-1568300381
PR Review Comment: https://git.openjdk.org/jextract/pull/121#discussion_r1210157420