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

[Jorn Vernee]

On Mon, 4 Nov 2024 12:13:26 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> 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 17 additional commits since the last revision:
>> 
>>  - Rephrase liker arg requirements
>>  - Merge branch 'master' into linker-padding-layout-only
>>  - Add rule checkings in AbstractLinker and add tests
>>  - Merge branch 'master' into linker-padding-layout-only
>>  - Update Linker docs
>>  - Merge branch 'master' into linker-padding-layout-only
>>  - Reword doce
>>  - Add to specification and refine detection of PL GLs
>>  - Add specific message for group layouts with only padding layouts
>>  - Merge branch 'master' into linker-padding-layout-only
>>  - ... and 7 more: https://git.openjdk.org/jdk/compare/7d8aec13...0f4e93ff
>
> src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java line 270:
> 
>> 268:                     " does not align the element " + element +
>> 269:                     " (at byte offset " + pos + ")" + inMessage(gl));
>> 270:         }
> 
> What happens here if an element is already aligned, and `padding.byteSize() == element.byteAlignment()`? e.g. `MemoryLayout.structLayout(MemoryLayout.paddingLayout(4), ValueLayout.JAVA_INT)`?

Also, don't we already check the alignment before when calling `checkMemberOffset`? Why is it checked again here?

> test/jdk/java/foreign/TestLinker.java line 183:
> 
>> 181: 
>> 182:         var fd = FunctionDescriptor.of(struct, struct, struct);
>> 183:         assertThrows(IllegalArgumentException.class, () ->linker.downcallHandle(fd));
> 
> Suggestion:
> 
>         assertThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd));

Could you please check the exception message as well? And in `stackedPadding`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21041#discussion_r1827639666
PR Review Comment: https://git.openjdk.org/jdk/pull/21041#discussion_r1827644509