jdk24 java/foreign/largestub/TestLargeStub IllegalArgumentException array length is not legal for long[] or double[]

[pshipton]

https://openj9-jenkins.osuosl.org/job/Test_openjdknext_j9_sanity.openjdk_aarch64_mac_Personal_testList_1/6
jdk_foreign_1
java/foreign/largestub/TestLargeStub.java

14:45:40  STARTED    TestLargeStub::testUpcall '[1] i4, 1'
14:45:40  java.lang.IllegalArgumentException: array length is not legal for long[] or double[]: 250
14:45:40  	at java.base/java.lang.invoke.MethodHandleStatics.newIllegalArgumentException(MethodHandleStatics.java:182)
14:45:40  	at java.base/java.lang.invoke.MethodHandle.spreadArrayChecks(MethodHandle.java:1175)
14:45:40  	at java.base/java.lang.invoke.MethodHandle.asCollectorChecks(MethodHandle.java:1357)
14:45:40  	at java.base/java.lang.invoke.MethodHandle.asCollector(MethodHandle.java:1343)
14:45:40  	at java.base/openj9.internal.foreign.abi.InternalDowncallHandler.permuteMH(InternalDowncallHandler.java:502)
14:45:40  	at java.base/openj9.internal.foreign.abi.InternalDowncallHandler.getBoundMethodHandle(InternalDowncallHandler.java:476)
14:45:40  	at java.base/jdk.internal.foreign.abi.DowncallLinker.getBoundMethodHandle(DowncallLinker.java:47)
14:45:40  	at java.base/jdk.internal.foreign.abi.aarch64.CallArranger.arrangeDowncall(CallArranger.java:192)
14:45:40  	at java.base/jdk.internal.foreign.abi.aarch64.macos.MacOsAArch64Linker.arrangeDowncall(MacOsAArch64Linker.java:64)
14:45:40  	at java.base/jdk.internal.foreign.abi.AbstractLinker.lambda$downcallHandle0$0(AbstractLinker.java:115)
14:45:40  	at java.base/jdk.internal.foreign.abi.SoftReferenceCache$Node.get(SoftReferenceCache.java:49)
14:45:40  	at java.base/jdk.internal.foreign.abi.SoftReferenceCache.get(SoftReferenceCache.java:38)
14:45:40  	at java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle0(AbstractLinker.java:112)
14:45:40  	at java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle(AbstractLinker.java:101)
14:45:40  	at TestLargeStub.testUpcall(TestLargeStub.java:79)
14:45:40  	at java.base/java.lang.reflect.Method.invoke(Method.java:579)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:807)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:294)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:583)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:573)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:270)
14:45:40  	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:636)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:294)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
14:45:40  	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:583)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:573)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:270)
14:45:40  	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:636)
14:45:40  	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:294)
14:45:40  	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:583)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:573)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
14:45:40  	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
14:45:40  	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:270)
14:45:40  	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:636)
14:45:40  	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
14:45:40  	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
14:45:40  FAILED     TestLargeStub::testUpcall '[1] i4, 1'

[github-actions]

Issue Number: 20355
Status: Open
Recommended Components: comp:vm, comp:test, comp:jclextensions
Recommended Assignees: chengjin01, pshipton, babsingh

[tajila]

@babsingh Please take a look

[babsingh]

The test was modified. It added checks for C_INT and C_FLOAT. It expects 248 slots for these layouts.

    private static Stream<Arguments> layouts() {
        return Stream.of(
            arguments(C_INT, 1),
            arguments(C_LONG_LONG, 2),
            arguments(C_FLOAT, 1),
            arguments(C_DOUBLE, 2)
        );
    }

    public void testDowncall(ValueLayout layout, int numSlots) {
        // Link a handle with a large number of arguments, to try and overflow the code buffer
        Linker.nativeLinker().downcallHandle(
                FunctionDescriptor.of(STRUCT_LL,
                        Stream.generate(() -> layout).limit(DOWNCALL_AVAILABLE_SLOTS / numSlots).toArray(MemoryLayout[]::new)),
                Linker.Option.captureCallState("errno"));
    }

While creating the MH for the Downcall, the arguments are transformed to long.class (i.e., converted to an Object array), which limits us to 127 double slots. This results in the failure for the C_INT and C_FLOAT scenarios, producing the reported exception. @tajila, what was the reasoning behind taking the below approach? In practice, it is highly unlikely to encounter a native function that requires more than 127 arguments. How would you prefer to proceed in addressing this issue?

openj9/jcl/src/java.base/share/classes/openj9/internal/foreign/abi/InternalDowncallHandler.java

Lines 630 to 643 in d471e57

	/* Collect and convert the passed-in arguments to an Object array for the underlying native call. */
	private MethodHandle permuteMH(MethodHandle targetHandle, MethodType nativeMethodType) throws NullPointerException, WrongMethodTypeException {
	Class<?>[] argTypeClasses = nativeMethodType.parameterArray();
	int nativeArgCount = argTypeClasses.length;
	/*[IF JAVA_SPEC_VERSION >= 21]*/
	/* Skip the native function address, the segment allocator and the segment
	* for the execution state to the native function's arguments.
	*/
	int argPosition = 3;
	/*[ELSE] JAVA_SPEC_VERSION >= 21 */
	/* Skip the native function address and the segment allocator to the native function's arguments. */
	int argPosition = 2;
	/*[ENDIF] JAVA_SPEC_VERSION >= 21 */
	MethodHandle resultHandle = targetHandle.asCollector(argPosition, long[].class, nativeArgCount);

[tajila]

The current downcall mechanism is quite general as that was the simplest way to implement it for all possible types. We pass the args via JNI so we need to use an array to capture the args or else we need infinite natives for the infinite combinations of params. We had to convert the params to long because its the largest common type for all primitives.

[tajila]

We could modify

    private static final int DOWNCALL_AVAILABLE_SLOTS = 248;
    private static final int UPCALL_AVAILABLE_SLOTS = 250;

To something our JVM can handle

[babsingh]

Sounds good, I will modify the test.

[babsingh]

Opened the below PRs to address the reported issue:

• JDK24: Reduce available slots to match the upper limit supported by OpenJ9 ibmruntimes/openj9-openjdk-jdk24#6
• JDK-next: Reduce available slots to match the upper limit supported by OpenJ9 ibmruntimes/openj9-openjdk-jdk#912

[keithc-ca]

The current downcall mechanism is quite general as that was the simplest way to implement it for all possible types. We pass the args via JNI so we need to use an array to capture the args or else we need infinite natives for the infinite combinations of params. We had to convert the params to long because its the largest common type for all primitives.

Our mapping from multiple parameters to a single (or a small number of) arrays should allow for more parameters, not less. What am I missing?

[babsingh]

What am I missing?

MethodHandle.asCollector enforces the following rule from JVM Spec 4.3.3 - Method Descriptors: The number of parameter slots required by the method's descriptor must be less than 256.

Since we are converting the arguments to an array of longs using MethodHandle.asCollector, the maximum number of elements that can be collected is 128, because each long occupies 2 slots, and the total number of slots must be less than 256.

[babsingh]

Closing since the PRs in #20355 (comment) have been merged.