[foreign-preview] RFR: 8282069: Set correct alignment constraints on ValueLayout constants

[Jorn Vernee]

On Fri, 18 Feb 2022 18:14:43 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> Now that we have a good understanding of how to enforce alignment checks for both off-heap _and_ heap segments, we are in a position to set the correct alignment constraints on the various `ValueLayout.JAVA_XYZ` constants.
> 
> The source changes are quite trivial; fixing the tests less so. The main issue is that some of the test are quite liberal in moving things in and out of `byte[]`. E.g. they create a `byte[]` and want to store a `JAVA_LONG` in it. If `JAVA_LONG` has the correct alignment (8 bytes) there's no way we can guarantee correctness and the code now fails (correctly).
> 
> Also, we now proprerly check alignment on bulk copy operations too - so bulk copy tests, which were ok before, now fail when correct alignment constraints are applied.
> 
> I've fixed some of the tests in simple ways - e.g. by using a `long[]` instead of a `byte[]`. But in some of the most complex tests I had to define unaligned layouts, as some of the tests are peeking and poking at memory in ways that is not compatible with aligned access (the byte buffer test is a good one, as byte buffer access is not aligned).
> 
> Also, I had to tweak some code in the `BindingSpecializer` class - some of the bindings can sometimes read multiple fields in a single shot - so e.g. if you have a struct like `[f32 f32]` a single `long` read is performed to read both fields. This means that we do a `JAVA_LONG` read, but with an underlying memory alignment of 4, which is not going to work in some cases (especially in upcalls, where we use an arena allocator to allocate temp buffers; the arena allocator can "pack" structs more than just using `malloc`). For this reason I had to change that code to use unaligned layouts as well.

Marked as reviewed by jvernee (Committer).

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 547:

> 545:     /**
> 546:      * A value layout constant whose size is the same as that of a machine address ({@code size_t}),
> 547:      * bit alignment set to {@code size_t * 8}, and byte order set to {@link ByteOrder#nativeOrder()}.

Suggestion:

     * bit alignment set to {@code sizeof(size_t) * 8}, and byte order set to {@link ByteOrder#nativeOrder()}.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/647