diff --git a/src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java b/src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java index 27759a78845..ab2fa82b10e 100644 --- a/src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java +++ b/src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java @@ -62,6 +62,7 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { * Returns a function descriptor with the given argument layouts appended to the argument layout array * of this function descriptor. * @param addedLayouts the argument layouts to append. + * @throws IllegalArgumentException if one of the layouts in {@code addedLayouts} is a padding layout. * @return the new function descriptor. */ FunctionDescriptor appendArgumentLayouts(MemoryLayout... addedLayouts); @@ -72,6 +73,7 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { * @param index the index at which to insert the arguments * @param addedLayouts the argument layouts to insert at given index. * @return the new function descriptor. + * @throws IllegalArgumentException if one of the layouts in {@code addedLayouts} is a padding layout. * @throws IllegalArgumentException if {@code index < 0 || index > argumentLayouts().size()}. */ FunctionDescriptor insertArgumentLayouts(int index, MemoryLayout... addedLayouts); @@ -79,6 +81,7 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { /** * Returns a function descriptor with the given memory layout as the new return layout. * @param newReturn the new return layout. + * @throws IllegalArgumentException if {@code newReturn} is a padding layout. * @return the new function descriptor. */ FunctionDescriptor changeReturnLayout(MemoryLayout newReturn); @@ -96,10 +99,12 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { * The carrier type of a layout is determined as follows: * * + * @apiNote A function descriptor cannot, by construction, contain any padding layouts. As such, it is not + * necessary to specify how padding layout should be mapped to carrier types. + * * @return the method type consisting of the carrier types of the layouts in this function descriptor * @throws IllegalArgumentException if one or more layouts in the function descriptor can not be mapped to carrier * types (e.g. if they are sequence layouts or padding layouts). @@ -110,6 +115,8 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { * Creates a function descriptor with the given return and argument layouts. * @param resLayout the return layout. * @param argLayouts the argument layouts. + * @throws IllegalArgumentException if {@code resLayout} is a padding layout. + * @throws IllegalArgumentException if one of the layouts in {@code argLayouts} is a padding layout. * @return the new function descriptor. */ static FunctionDescriptor of(MemoryLayout resLayout, MemoryLayout... argLayouts) { @@ -121,6 +128,7 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl { /** * Creates a function descriptor with the given argument layouts and no return layout. * @param argLayouts the argument layouts. + * @throws IllegalArgumentException if one of the layouts in {@code argLayouts} is a padding layout. * @return the new function descriptor. */ static FunctionDescriptor ofVoid(MemoryLayout... argLayouts) { diff --git a/src/java.base/share/classes/jdk/internal/foreign/FunctionDescriptorImpl.java b/src/java.base/share/classes/jdk/internal/foreign/FunctionDescriptorImpl.java index 0723af6ba0d..da1727285bf 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/FunctionDescriptorImpl.java +++ b/src/java.base/share/classes/jdk/internal/foreign/FunctionDescriptorImpl.java @@ -28,6 +28,8 @@ import java.lang.foreign.FunctionDescriptor; import java.lang.foreign.GroupLayout; import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; +import java.lang.foreign.PaddingLayout; +import java.lang.foreign.SequenceLayout; import java.lang.foreign.ValueLayout; import java.lang.invoke.MethodType; import java.util.ArrayList; @@ -47,6 +49,13 @@ public final class FunctionDescriptorImpl implements FunctionDescriptor { private final List argLayouts; private FunctionDescriptorImpl(MemoryLayout resLayout, List argLayouts) { + if (resLayout instanceof PaddingLayout) { + throw new IllegalArgumentException("Unsupported padding layout return in function descriptor: " + resLayout); + } + Optional paddingLayout = argLayouts.stream().filter(l -> l instanceof PaddingLayout).findAny(); + if (paddingLayout.isPresent()) { + throw new IllegalArgumentException("Unsupported padding layout argument in function descriptor: " + paddingLayout.get()); + } this.resLayout = resLayout; this.argLayouts = List.copyOf(argLayouts); } @@ -120,10 +129,11 @@ public final class FunctionDescriptorImpl implements FunctionDescriptor { private static Class carrierTypeFor(MemoryLayout layout) { if (layout instanceof ValueLayout valueLayout) { return valueLayout.carrier(); - } else if (layout instanceof GroupLayout) { + } else if (layout instanceof GroupLayout || layout instanceof SequenceLayout) { return MemorySegment.class; } else { - throw new IllegalArgumentException("Unsupported layout: " + layout); + // Note: we should not worry about padding layouts, as they cannot be present in a function descriptor + throw new AssertionError("Cannot get here"); } } diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java index e16097f8a29..f7a2b9114c1 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java @@ -34,7 +34,6 @@ import jdk.internal.foreign.abi.riscv64.linux.LinuxRISCV64Linker; import jdk.internal.foreign.abi.x64.sysv.SysVx64Linker; import jdk.internal.foreign.abi.x64.windows.Windowsx64Linker; import jdk.internal.foreign.layout.AbstractLayout; -import jdk.internal.foreign.layout.ValueLayouts; import java.lang.foreign.AddressLayout; import java.lang.foreign.GroupLayout; @@ -116,11 +115,20 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch protected abstract ByteOrder linkerByteOrder(); private void checkLayouts(FunctionDescriptor descriptor) { - descriptor.returnLayout().ifPresent(this::checkLayoutsRecursive); - descriptor.argumentLayouts().forEach(this::checkLayoutsRecursive); + descriptor.returnLayout().ifPresent(this::checkLayout); + descriptor.argumentLayouts().forEach(this::checkLayout); } - private void checkLayoutsRecursive(MemoryLayout layout) { + private void checkLayout(MemoryLayout layout) { + // Note: we should not worry about padding layouts, as they cannot be present in a function descriptor + if (layout instanceof SequenceLayout) { + throw new IllegalArgumentException("Unsupported layout: " + layout); + } else { + checkLayoutRecursive(layout); + } + } + + private void checkLayoutRecursive(MemoryLayout layout) { checkHasNaturalAlignment(layout); if (layout instanceof ValueLayout vl) { checkByteOrder(vl); @@ -131,7 +139,7 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch // check element offset before recursing so that an error points at the // outermost layout first checkMemberOffset(sl, member, lastUnpaddedOffset, offset); - checkLayoutsRecursive(member); + checkLayoutRecursive(member); offset += member.bitSize(); if (!(member instanceof PaddingLayout)) { @@ -142,14 +150,14 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch } else if (layout instanceof UnionLayout ul) { long maxUnpaddedLayout = 0; for (MemoryLayout member : ul.memberLayouts()) { - checkLayoutsRecursive(member); + checkLayoutRecursive(member); if (!(member instanceof PaddingLayout)) { maxUnpaddedLayout = Long.max(maxUnpaddedLayout, member.bitSize()); } } checkGroupSize(ul, maxUnpaddedLayout); } else if (layout instanceof SequenceLayout sl) { - checkLayoutsRecursive(sl.elementLayout()); + checkLayoutRecursive(sl.elementLayout()); } } diff --git a/test/jdk/java/foreign/TestFunctionDescriptor.java b/test/jdk/java/foreign/TestFunctionDescriptor.java index bd1c524c23a..8482181230d 100644 --- a/test/jdk/java/foreign/TestFunctionDescriptor.java +++ b/test/jdk/java/foreign/TestFunctionDescriptor.java @@ -113,9 +113,10 @@ public class TestFunctionDescriptor extends NativeTestHelper { public void testCarrierMethodType() { FunctionDescriptor fd = FunctionDescriptor.of(C_INT, C_INT, - MemoryLayout.structLayout(C_INT, C_INT)); + MemoryLayout.structLayout(C_INT, C_INT), + MemoryLayout.sequenceLayout(3, C_INT)); MethodType cmt = fd.toMethodType(); - assertEquals(cmt, MethodType.methodType(int.class, int.class, MemorySegment.class)); + assertEquals(cmt, MethodType.methodType(int.class, int.class, MemorySegment.class, MemorySegment.class)); } @Test(expectedExceptions = IllegalArgumentException.class) @@ -140,4 +141,29 @@ public class TestFunctionDescriptor extends NativeTestHelper { fd.insertArgumentLayouts(2, C_INT); } + @Test(expectedExceptions = IllegalArgumentException.class) + public void testBadPaddingInVoidFunction() { + FunctionDescriptor.ofVoid(MemoryLayout.paddingLayout(8)); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testBadPaddingInNonVoidFunction() { + FunctionDescriptor.of(MemoryLayout.paddingLayout(8)); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testBadPaddingInAppendArgLayouts() { + FunctionDescriptor.ofVoid().appendArgumentLayouts(MemoryLayout.paddingLayout(8)); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testBadPaddingInInsertArgLayouts() { + FunctionDescriptor.ofVoid().insertArgumentLayouts(0, MemoryLayout.paddingLayout(8)); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testBadPaddingInChangeRetLayout() { + FunctionDescriptor.ofVoid().changeReturnLayout(MemoryLayout.paddingLayout(8)); + } + } diff --git a/test/jdk/java/foreign/TestIllegalLink.java b/test/jdk/java/foreign/TestIllegalLink.java index 9bce8d53f2f..f076789b579 100644 --- a/test/jdk/java/foreign/TestIllegalLink.java +++ b/test/jdk/java/foreign/TestIllegalLink.java @@ -109,14 +109,6 @@ public class TestIllegalLink extends NativeTestHelper { @DataProvider public static Object[][] types() { List cases = new ArrayList<>(Arrays.asList(new Object[][]{ - { - FunctionDescriptor.of(MemoryLayout.paddingLayout(64)), - "Unsupported layout: x64" - }, - { - FunctionDescriptor.ofVoid(MemoryLayout.paddingLayout(64)), - "Unsupported layout: x64" - }, { FunctionDescriptor.of(MemoryLayout.sequenceLayout(2, C_INT)), "Unsupported layout: [2:i32]"