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

[Jorn Vernee]

On Tue, 5 Nov 2024 14:24:20 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>>> What happens here if an element is already aligned, and `padding.byteSize() == element.byteAlignment()`? e.g. `MemoryLayout.structLayout(MemoryLayout.paddingLayout(4), ValueLayout.JAVA_INT)`?
>> 
>> This is an error that existed before this PR. On the main branch:
>> 
>> 
>>     @Test
>>     public void alignedByInitialPadding() {
>>         Linker linker = Linker.nativeLinker();
>>         var struct = MemoryLayout.structLayout(
>>                 MemoryLayout.paddingLayout(Integer.BYTES),
>>                 JAVA_INT);
>>         var fd = FunctionDescriptor.of(struct, struct, struct);
>>         linker.downcallHandle(fd);
>>     }
>> 
>> 
>> would produce:
>> 
>> 
>> test TestLinker.alignedByInitialPadding(): failure
>> java.lang.IllegalArgumentException: Member layout 'i4', of '[x4i4]' found at unexpected offset: 4 != 0
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.checkMemberOffset(AbstractLinker.java:235)
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayoutRecursive(AbstractLinker.java:195)
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayout(AbstractLinker.java:176)
>> 	at java.base/java.util.Optional.ifPresent(Optional.java:178)
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayouts(AbstractLinker.java:167)
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle0(AbstractLinker.java:98)
>> 	at java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle(AbstractLinker.java:92)
>> 	at TestLinker.alignedByInitialPadding(TestLinker.java:160)
>> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>
> I've raised a separate issue for this: https://bugs.openjdk.org/browse/JDK-8343620

Looks like you removed the check here: https://github.com/openjdk/jdk/pull/21041/commits/0c134821d6ad7d3c7668d9cc88f7d329b5648827

I think that is good. We already check whether struct fields are aligned when creating a struct layout.

The existing `checkMemberOffset` does the right thing for this case. If tries to manually compute the expected offset of the field (using minimal amount of padding), and then checks if the field actually appears at that offset.

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

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