Skip to content

Commit 7a3bea1

Browse files
mcimadamoremcimadamore
committed
8307629: FunctionDescriptor::toMethodType should allow sequence layouts (mainline)
Reviewed-by: jvernee
1 parent d2b3eef commit 7a3bea1

File tree

5 files changed

+65
-21
lines changed

5 files changed

+65
-21
lines changed

‎src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl {
6262
* Returns a function descriptor with the given argument layouts appended to the argument layout array
6363
* of this function descriptor.
6464
* @param addedLayouts the argument layouts to append.
65+
* @throws IllegalArgumentException if one of the layouts in {@code addedLayouts} is a padding layout.
6566
* @return the new function descriptor.
6667
*/
6768
FunctionDescriptor appendArgumentLayouts(MemoryLayout... addedLayouts);
@@ -72,13 +73,15 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl {
7273
* @param index the index at which to insert the arguments
7374
* @param addedLayouts the argument layouts to insert at given index.
7475
* @return the new function descriptor.
76+
* @throws IllegalArgumentException if one of the layouts in {@code addedLayouts} is a padding layout.
7577
* @throws IllegalArgumentException if {@code index < 0 || index > argumentLayouts().size()}.
7678
*/
7779
FunctionDescriptor insertArgumentLayouts(int index, MemoryLayout... addedLayouts);
7880

7981
/**
8082
* Returns a function descriptor with the given memory layout as the new return layout.
8183
* @param newReturn the new return layout.
84+
* @throws IllegalArgumentException if {@code newReturn} is a padding layout.
8285
* @return the new function descriptor.
8386
*/
8487
FunctionDescriptor changeReturnLayout(MemoryLayout newReturn);
@@ -96,10 +99,12 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl {
9699
* The carrier type of a layout is determined as follows:
97100
* <ul>
98101
* <li>If the layout is a {@link ValueLayout} the carrier type is determined through {@link ValueLayout#carrier()}.</li>
99-
* <li>If the layout is a {@link GroupLayout} the carrier type is {@link MemorySegment}.</li>
100-
* <li>If the layout is a {@link PaddingLayout}, or {@link SequenceLayout} an {@link IllegalArgumentException} is thrown.</li>
102+
* <li>If the layout is a {@link GroupLayout} or a {@link SequenceLayout}, the carrier type is {@link MemorySegment}.</li>
101103
* </ul>
102104
*
105+
* @apiNote A function descriptor cannot, by construction, contain any padding layouts. As such, it is not
106+
* necessary to specify how padding layout should be mapped to carrier types.
107+
*
103108
* @return the method type consisting of the carrier types of the layouts in this function descriptor
104109
* @throws IllegalArgumentException if one or more layouts in the function descriptor can not be mapped to carrier
105110
* types (e.g. if they are sequence layouts or padding layouts).
@@ -110,6 +115,8 @@ public sealed interface FunctionDescriptor permits FunctionDescriptorImpl {
110115
* Creates a function descriptor with the given return and argument layouts.
111116
* @param resLayout the return layout.
112117
* @param argLayouts the argument layouts.
118+
* @throws IllegalArgumentException if {@code resLayout} is a padding layout.
119+
* @throws IllegalArgumentException if one of the layouts in {@code argLayouts} is a padding layout.
113120
* @return the new function descriptor.
114121
*/
115122
static FunctionDescriptor of(MemoryLayout resLayout, MemoryLayout... argLayouts) {
@@ -121,6 +128,7 @@ static FunctionDescriptor of(MemoryLayout resLayout, MemoryLayout... argLayouts)
121128
/**
122129
* Creates a function descriptor with the given argument layouts and no return layout.
123130
* @param argLayouts the argument layouts.
131+
* @throws IllegalArgumentException if one of the layouts in {@code argLayouts} is a padding layout.
124132
* @return the new function descriptor.
125133
*/
126134
static FunctionDescriptor ofVoid(MemoryLayout... argLayouts) {

‎src/java.base/share/classes/jdk/internal/foreign/FunctionDescriptorImpl.java

+12-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import java.lang.foreign.GroupLayout;
2929
import java.lang.foreign.MemoryLayout;
3030
import java.lang.foreign.MemorySegment;
31+
import java.lang.foreign.PaddingLayout;
32+
import java.lang.foreign.SequenceLayout;
3133
import java.lang.foreign.ValueLayout;
3234
import java.lang.invoke.MethodType;
3335
import java.util.ArrayList;
@@ -47,6 +49,13 @@ public final class FunctionDescriptorImpl implements FunctionDescriptor {
4749
private final List<MemoryLayout> argLayouts;
4850

4951
private FunctionDescriptorImpl(MemoryLayout resLayout, List<MemoryLayout> argLayouts) {
52+
if (resLayout instanceof PaddingLayout) {
53+
throw new IllegalArgumentException("Unsupported padding layout return in function descriptor: " + resLayout);
54+
}
55+
Optional<MemoryLayout> paddingLayout = argLayouts.stream().filter(l -> l instanceof PaddingLayout).findAny();
56+
if (paddingLayout.isPresent()) {
57+
throw new IllegalArgumentException("Unsupported padding layout argument in function descriptor: " + paddingLayout.get());
58+
}
5059
this.resLayout = resLayout;
5160
this.argLayouts = List.copyOf(argLayouts);
5261
}
@@ -120,10 +129,11 @@ public FunctionDescriptorImpl dropReturnLayout() {
120129
private static Class<?> carrierTypeFor(MemoryLayout layout) {
121130
if (layout instanceof ValueLayout valueLayout) {
122131
return valueLayout.carrier();
123-
} else if (layout instanceof GroupLayout) {
132+
} else if (layout instanceof GroupLayout || layout instanceof SequenceLayout) {
124133
return MemorySegment.class;
125134
} else {
126-
throw new IllegalArgumentException("Unsupported layout: " + layout);
135+
// Note: we should not worry about padding layouts, as they cannot be present in a function descriptor
136+
throw new AssertionError("Cannot get here");
127137
}
128138
}
129139

‎src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java

+15-7
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import jdk.internal.foreign.abi.x64.sysv.SysVx64Linker;
3535
import jdk.internal.foreign.abi.x64.windows.Windowsx64Linker;
3636
import jdk.internal.foreign.layout.AbstractLayout;
37-
import jdk.internal.foreign.layout.ValueLayouts;
3837

3938
import java.lang.foreign.AddressLayout;
4039
import java.lang.foreign.GroupLayout;
@@ -116,11 +115,20 @@ public SystemLookup defaultLookup() {
116115
protected abstract ByteOrder linkerByteOrder();
117116

118117
private void checkLayouts(FunctionDescriptor descriptor) {
119-
descriptor.returnLayout().ifPresent(this::checkLayoutsRecursive);
120-
descriptor.argumentLayouts().forEach(this::checkLayoutsRecursive);
118+
descriptor.returnLayout().ifPresent(this::checkLayout);
119+
descriptor.argumentLayouts().forEach(this::checkLayout);
121120
}
122121

123-
private void checkLayoutsRecursive(MemoryLayout layout) {
122+
private void checkLayout(MemoryLayout layout) {
123+
// Note: we should not worry about padding layouts, as they cannot be present in a function descriptor
124+
if (layout instanceof SequenceLayout) {
125+
throw new IllegalArgumentException("Unsupported layout: " + layout);
126+
} else {
127+
checkLayoutRecursive(layout);
128+
}
129+
}
130+
131+
private void checkLayoutRecursive(MemoryLayout layout) {
124132
checkHasNaturalAlignment(layout);
125133
if (layout instanceof ValueLayout vl) {
126134
checkByteOrder(vl);
@@ -131,7 +139,7 @@ private void checkLayoutsRecursive(MemoryLayout layout) {
131139
// check element offset before recursing so that an error points at the
132140
// outermost layout first
133141
checkMemberOffset(sl, member, lastUnpaddedOffset, offset);
134-
checkLayoutsRecursive(member);
142+
checkLayoutRecursive(member);
135143

136144
offset += member.bitSize();
137145
if (!(member instanceof PaddingLayout)) {
@@ -142,14 +150,14 @@ private void checkLayoutsRecursive(MemoryLayout layout) {
142150
} else if (layout instanceof UnionLayout ul) {
143151
long maxUnpaddedLayout = 0;
144152
for (MemoryLayout member : ul.memberLayouts()) {
145-
checkLayoutsRecursive(member);
153+
checkLayoutRecursive(member);
146154
if (!(member instanceof PaddingLayout)) {
147155
maxUnpaddedLayout = Long.max(maxUnpaddedLayout, member.bitSize());
148156
}
149157
}
150158
checkGroupSize(ul, maxUnpaddedLayout);
151159
} else if (layout instanceof SequenceLayout sl) {
152-
checkLayoutsRecursive(sl.elementLayout());
160+
checkLayoutRecursive(sl.elementLayout());
153161
}
154162
}
155163

‎test/jdk/java/foreign/TestFunctionDescriptor.java

+28-2
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,10 @@ public void testEquals() {
113113
public void testCarrierMethodType() {
114114
FunctionDescriptor fd = FunctionDescriptor.of(C_INT,
115115
C_INT,
116-
MemoryLayout.structLayout(C_INT, C_INT));
116+
MemoryLayout.structLayout(C_INT, C_INT),
117+
MemoryLayout.sequenceLayout(3, C_INT));
117118
MethodType cmt = fd.toMethodType();
118-
assertEquals(cmt, MethodType.methodType(int.class, int.class, MemorySegment.class));
119+
assertEquals(cmt, MethodType.methodType(int.class, int.class, MemorySegment.class, MemorySegment.class));
119120
}
120121

121122
@Test(expectedExceptions = IllegalArgumentException.class)
@@ -140,4 +141,29 @@ public void testIllegalInsertArgOutOfBounds() {
140141
fd.insertArgumentLayouts(2, C_INT);
141142
}
142143

144+
@Test(expectedExceptions = IllegalArgumentException.class)
145+
public void testBadPaddingInVoidFunction() {
146+
FunctionDescriptor.ofVoid(MemoryLayout.paddingLayout(8));
147+
}
148+
149+
@Test(expectedExceptions = IllegalArgumentException.class)
150+
public void testBadPaddingInNonVoidFunction() {
151+
FunctionDescriptor.of(MemoryLayout.paddingLayout(8));
152+
}
153+
154+
@Test(expectedExceptions = IllegalArgumentException.class)
155+
public void testBadPaddingInAppendArgLayouts() {
156+
FunctionDescriptor.ofVoid().appendArgumentLayouts(MemoryLayout.paddingLayout(8));
157+
}
158+
159+
@Test(expectedExceptions = IllegalArgumentException.class)
160+
public void testBadPaddingInInsertArgLayouts() {
161+
FunctionDescriptor.ofVoid().insertArgumentLayouts(0, MemoryLayout.paddingLayout(8));
162+
}
163+
164+
@Test(expectedExceptions = IllegalArgumentException.class)
165+
public void testBadPaddingInChangeRetLayout() {
166+
FunctionDescriptor.ofVoid().changeReturnLayout(MemoryLayout.paddingLayout(8));
167+
}
168+
143169
}

‎test/jdk/java/foreign/TestIllegalLink.java

-8
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,6 @@ public static Object[][] downcallOnlyOptions() {
109109
@DataProvider
110110
public static Object[][] types() {
111111
List<Object[]> cases = new ArrayList<>(Arrays.asList(new Object[][]{
112-
{
113-
FunctionDescriptor.of(MemoryLayout.paddingLayout(64)),
114-
"Unsupported layout: x64"
115-
},
116-
{
117-
FunctionDescriptor.ofVoid(MemoryLayout.paddingLayout(64)),
118-
"Unsupported layout: x64"
119-
},
120112
{
121113
FunctionDescriptor.of(MemoryLayout.sequenceLayout(2, C_INT)),
122114
"Unsupported layout: [2:i32]"

0 commit comments

Comments
 (0)