RFR: 8340205: Native linker allows MemoryLayout consisting of only PaddingLayout [v17]

[Jorn Vernee]

On Wed, 20 Nov 2024 13:46:49 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR prevents sequence layout with padding to be used with the Linker.
>
> Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 36 additional commits since the last revision:
> 
>  - Update after comments
>  - Merge branch 'master' into linker-padding-layout-only
>  - Fix failing test
>  - Merge branch 'master' into linker-padding-layout-only
>  - Update after comments
>  - Merge branch 'master' into linker-padding-layout-only
>  - Simplify exception testing
>  - Merge branch 'master' into linker-padding-layout-only
>  - Remove redundant check
>  - Rephrase doc
>  - ... and 26 more: https://git.openjdk.org/jdk/compare/2f4888ed...28b3ad6d

Could you also add test cases for the other two unions I mentioned?


MemoryLayout.unionLayout(MemoryLayout.paddingLayout(4), ValueLayout.JAVA_INT);
MemoryLayout.unionLayout(MemoryLayout.paddingLayout(5), ValueLayout.JAVA_INT):


Both of those should be rejected.

src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java line 222:

> 220:                 if (!(member instanceof PaddingLayout pl)) {
> 221:                     maxUnpaddedLayout = Long.max(maxUnpaddedLayout, member.byteSize());
> 222:                 } else {

I think we're still missing a check here to see if a union contains at most 1 padding layout?

Something like this:


MemoryLayout.unionLayout(
    MemoryLayout.sequenceLayout(3, ValueLayout.JAVA_INT),
    ValueLayout.JAVA_LONG,
    MemoryLayout.paddingLayout(16),
    MemoryLayout.paddingLayout(16)):


Should be rejected.

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

PR Review: https://git.openjdk.org/jdk/pull/21041#pullrequestreview-2449463637
PR Review Comment: https://git.openjdk.org/jdk/pull/21041#discussion_r1850835087