From 68ba7ee5c8f152a268b1e95d52417783346d12b7 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Mon, 25 Nov 2024 07:42:57 +0000 Subject: [PATCH] 8340205: Native linker allows MemoryLayout consisting of only PaddingLayout Reviewed-by: jvernee, mcimadamore --- .../classes/java/lang/foreign/Linker.java | 56 ++++----- .../internal/foreign/abi/AbstractLinker.java | 71 +++++++++-- test/jdk/java/foreign/TestLinker.java | 115 +++++++++++++++++- 3 files changed, 191 insertions(+), 51 deletions(-) diff --git a/src/java.base/share/classes/java/lang/foreign/Linker.java b/src/java.base/share/classes/java/lang/foreign/Linker.java index 5474fef66da..bfa205e2fad 100644 --- a/src/java.base/share/classes/java/lang/foreign/Linker.java +++ b/src/java.base/share/classes/java/lang/foreign/Linker.java @@ -241,50 +241,40 @@ import java.util.stream.Stream; * * *

- * All native linker implementations support a well-defined subset of layouts. More formally, - * a layout {@code L} is supported by a native linker {@code NL} if: + * A native linker only supports function descriptors whose argument/return layouts are + * well-formed layouts. More formally, a layout `L` is well-formed if: *

- * - * Linker implementations may optionally support additional layouts, such as - * packed struct layouts. A packed struct is a struct in which there is - * at least one member layout {@code L} that has an alignment constraint less strict - * than its natural alignment. This allows to avoid padding between member layouts, - * as well as avoiding padding at the end of the struct layout. For example: - - * {@snippet lang = java: - * // No padding between the 2 element layouts: - * MemoryLayout noFieldPadding = MemoryLayout.structLayout( - * ValueLayout.JAVA_INT, - * ValueLayout.JAVA_DOUBLE.withByteAlignment(4)); - * - * // No padding at the end of the struct: - * MemoryLayout noTrailingPadding = MemoryLayout.structLayout( - * ValueLayout.JAVA_DOUBLE.withByteAlignment(4), - * ValueLayout.JAVA_INT); - * } *

- * A native linker only supports function descriptors whose argument/return layouts are - * layouts supported by that linker and are not sequence layouts. + * A function descriptor is well-formed if its argument and return layouts are + * well-formed and are not sequence layouts. A native linker is guaranteed to reject + * function descriptors that are not well-formed. However, a native linker can still + * reject well-formed function descriptors, according to platform-specific rules. + * For example, some native linkers may reject packed struct layouts -- struct + * layouts whose member layouts feature relaxed alignment constraints, to avoid + * the insertion of additional padding. * *

Function pointers

* 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 28391df7a0b..5b40ac17750 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 @@ -57,7 +57,6 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodType; import java.util.HashSet; import java.util.List; -import java.nio.ByteOrder; import java.util.Objects; import java.util.Set; @@ -189,6 +188,7 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch checkHasNaturalAlignment(layout); long offset = 0; long lastUnpaddedOffset = 0; + PaddingLayout preceedingPadding = null; for (MemoryLayout member : sl.memberLayouts()) { // check element offset before recursing so that an error points at the // outermost layout first @@ -196,29 +196,65 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch checkStructMember(member, offset); offset += member.byteSize(); - if (!(member instanceof PaddingLayout)) { + if (!(member instanceof PaddingLayout pl)) { lastUnpaddedOffset = offset; + if (preceedingPadding != null) { + preceedingPadding = null; + } + } else { + if (preceedingPadding != null) { + throw new IllegalArgumentException("The padding layout " + pl + + " was preceded by another padding layout " + preceedingPadding + + inMessage(sl)); + } + preceedingPadding = pl; } } - checkGroupSize(sl, lastUnpaddedOffset); + checkNotAllPadding(sl); + checkGroup(sl, lastUnpaddedOffset); } else if (layout instanceof UnionLayout ul) { checkHasNaturalAlignment(layout); - long maxUnpaddedLayout = 0; + // We need to know this up front + long maxUnpaddedLayout = ul.memberLayouts().stream() + .filter(l -> !(l instanceof PaddingLayout)) + .mapToLong(MemoryLayout::byteSize) + .max() + .orElse(0); + + boolean hasPadding = false; + for (MemoryLayout member : ul.memberLayouts()) { checkLayoutRecursive(member); - if (!(member instanceof PaddingLayout)) { - maxUnpaddedLayout = Long.max(maxUnpaddedLayout, member.byteSize()); + if (member instanceof PaddingLayout pl) { + if (hasPadding) { + throw new IllegalArgumentException("More than one padding" + inMessage(ul)); + } + hasPadding = true; + if (pl.byteSize() <= maxUnpaddedLayout) { + throw new IllegalArgumentException("Superfluous padding " + pl + inMessage(ul)); + } } } - checkGroupSize(ul, maxUnpaddedLayout); + checkGroup(ul, maxUnpaddedLayout); } else if (layout instanceof SequenceLayout sl) { checkHasNaturalAlignment(layout); + if (sl.elementLayout() instanceof PaddingLayout pl) { + throw memberException(sl, pl, + "not supported because a sequence of a padding layout is not allowed"); + } checkLayoutRecursive(sl.elementLayout()); } } - // check for trailing padding - private void checkGroupSize(GroupLayout gl, long maxUnpaddedOffset) { + // check elements are not all padding layouts + private static void checkNotAllPadding(StructLayout sl) { + if (!sl.memberLayouts().isEmpty() && sl.memberLayouts().stream().allMatch(e -> e instanceof PaddingLayout)) { + throw new IllegalArgumentException("Layout '" + sl + "' is non-empty and only has padding layouts"); + } + } + + // check trailing padding + private static void checkGroup(GroupLayout gl, long maxUnpaddedOffset) { long expectedSize = Utils.alignUp(maxUnpaddedOffset, gl.byteAlignment()); if (gl.byteSize() != expectedSize) { throw new IllegalArgumentException("Layout '" + gl + "' has unexpected size: " @@ -226,17 +262,28 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch } } + private static String inMessage(GroupLayout gl) { + return " in " + gl; + } + // checks both that there is no excess padding between 'memberLayout' and // the previous layout - private void checkMemberOffset(StructLayout parent, MemoryLayout memberLayout, + private static void checkMemberOffset(StructLayout parent, MemoryLayout memberLayout, long lastUnpaddedOffset, long offset) { long expectedOffset = Utils.alignUp(lastUnpaddedOffset, memberLayout.byteAlignment()); if (expectedOffset != offset) { - throw new IllegalArgumentException("Member layout '" + memberLayout + "', of '" + parent + "'" + - " found at unexpected offset: " + offset + " != " + expectedOffset); + throw memberException(parent, memberLayout, + "found at unexpected offset: " + offset + " != " + expectedOffset); } } + private static IllegalArgumentException memberException(MemoryLayout parent, + MemoryLayout member, + String info) { + return new IllegalArgumentException( + "Member layout '" + member + "', of '" + parent + "' " + info); + } + private void checkSupported(ValueLayout valueLayout) { valueLayout = valueLayout.withoutName(); if (valueLayout instanceof AddressLayout addressLayout) { diff --git a/test/jdk/java/foreign/TestLinker.java b/test/jdk/java/foreign/TestLinker.java index 82d58300088..4bc19965f19 100644 --- a/test/jdk/java/foreign/TestLinker.java +++ b/test/jdk/java/foreign/TestLinker.java @@ -35,6 +35,9 @@ import org.testng.annotations.Test; import java.lang.foreign.FunctionDescriptor; import java.lang.foreign.Linker; import java.lang.foreign.MemoryLayout; +import java.lang.foreign.PaddingLayout; +import java.lang.foreign.SequenceLayout; +import java.lang.foreign.StructLayout; import java.lang.foreign.ValueLayout; import java.lang.invoke.MethodHandle; import java.util.ArrayList; @@ -42,12 +45,8 @@ import java.util.Arrays; import java.util.List; import static java.lang.foreign.MemoryLayout.*; -import static java.lang.foreign.ValueLayout.JAVA_CHAR; -import static java.lang.foreign.ValueLayout.JAVA_SHORT; -import static org.testng.Assert.assertNotNull; -import static org.testng.Assert.assertSame; -import static org.testng.Assert.assertNotSame; -import static org.testng.Assert.assertTrue; +import static java.lang.foreign.ValueLayout.*; +import static org.testng.Assert.*; public class TestLinker extends NativeTestHelper { @@ -150,6 +149,110 @@ public class TestLinker extends NativeTestHelper { assertTrue(layout instanceof ValueLayout); } + @Test + public void embeddedPaddingLayout() { + PaddingLayout padding = MemoryLayout.paddingLayout(64).withByteAlignment(64); + SequenceLayout sequence = MemoryLayout.sequenceLayout(2, padding); + StructLayout struct = MemoryLayout.structLayout(sequence); + FunctionDescriptor fd = FunctionDescriptor.of(struct, struct); + Linker linker = Linker.nativeLinker(); + var x = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertTrue(x.getMessage().contains("not supported because a sequence of a padding layout is not allowed")); + } + + @Test + public void groupLayoutWithOnlyPadding() { + PaddingLayout padding = MemoryLayout.paddingLayout(1); + StructLayout struct = MemoryLayout.structLayout(padding); + FunctionDescriptor fd = FunctionDescriptor.of(struct, struct); + Linker linker = Linker.nativeLinker(); + var x = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertTrue(x.getMessage().contains("is non-empty and only has padding layouts")); + } + + @Test + public void interwovenPadding() { + Linker linker = Linker.nativeLinker(); + var padding1 = MemoryLayout.paddingLayout(1); + var padding2 = MemoryLayout.paddingLayout(2).withByteAlignment(2); + + var struct = MemoryLayout.structLayout(JAVA_BYTE, padding1, padding2, JAVA_INT); + + var fd = FunctionDescriptor.of(struct, struct, struct); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), + "The padding layout x2 was preceded by another padding layout x1 in [b1x1x2i4]"); + } + + @Test + public void stackedPadding() { + Linker linker = Linker.nativeLinker(); + var struct32 = MemoryLayout.structLayout(MemoryLayout.sequenceLayout(4, JAVA_LONG)); + var padding1 = MemoryLayout.paddingLayout(1); + var padding2 = MemoryLayout.paddingLayout(2).withByteAlignment(2); + var padding4 = MemoryLayout.paddingLayout(4).withByteAlignment(4); + var padding8 = MemoryLayout.paddingLayout(8).withByteAlignment(8); + var padding16 = MemoryLayout.paddingLayout(16).withByteAlignment(16); + var padding32 = MemoryLayout.paddingLayout(32).withByteAlignment(32); + var union = MemoryLayout.unionLayout(struct32, padding32); + var struct = MemoryLayout.structLayout(JAVA_BYTE, padding1, padding2, padding4, padding8, padding16, union); + var fd = FunctionDescriptor.of(struct, struct, struct); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), + "The padding layout x2 was preceded by another padding layout x1 in [b1x1x2x4x8x16[[[4:j8]]|x32]]"); + } + + @Test + public void paddingUnionByteSize3() { + Linker linker = Linker.nativeLinker(); + var union = MemoryLayout.unionLayout(MemoryLayout.paddingLayout(3), ValueLayout.JAVA_INT); + var fd = FunctionDescriptor.of(union, union, union); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), "Superfluous padding x3 in [x3|i4]"); + } + + @Test + public void paddingUnionByteSize4() { + Linker linker = Linker.nativeLinker(); + var union = MemoryLayout.unionLayout(MemoryLayout.paddingLayout(4), ValueLayout.JAVA_INT); + var fd = FunctionDescriptor.of(union, union, union); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), "Superfluous padding x4 in [x4|i4]"); + } + + @Test + public void paddingUnionByteSize5() { + Linker linker = Linker.nativeLinker(); + var union = MemoryLayout.unionLayout(MemoryLayout.paddingLayout(5), ValueLayout.JAVA_INT); + var fd = FunctionDescriptor.of(union, union, union); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), "Layout '[x5|i4]' has unexpected size: 5 != 4"); + } + + @Test + public void paddingUnionSeveral() { + Linker linker = Linker.nativeLinker(); + var union = MemoryLayout.unionLayout( + MemoryLayout.sequenceLayout(3, ValueLayout.JAVA_INT), + ValueLayout.JAVA_LONG, + MemoryLayout.paddingLayout(16), + MemoryLayout.paddingLayout(16)); + var fd = FunctionDescriptor.of(union, union, union); + var e = expectThrows(IllegalArgumentException.class, () -> linker.downcallHandle(fd)); + assertEquals(e.getMessage(), "More than one padding in [[3:i4]|j8|x16|x16]"); + } + + @Test + public void sequenceOfZeroElements() { + Linker linker = Linker.nativeLinker(); + var sequence0a8 = MemoryLayout.sequenceLayout(0, JAVA_LONG); + var sequence3a1 = MemoryLayout.sequenceLayout(3, JAVA_BYTE); + var padding5a1 = MemoryLayout.paddingLayout(5); + var struct8a8 = MemoryLayout.structLayout(sequence0a8, sequence3a1, padding5a1); + var fd = FunctionDescriptor.of(struct8a8, struct8a8, struct8a8); + linker.downcallHandle(fd); + } + @DataProvider public static Object[][] canonicalTypeNames() { return new Object[][]{