Merge pull request #17414 from ChengJin01/ffi_remove_null_seg_check_f…

…rom_ptr_v0.39.0

[Jtreg/FFI/0.39] Remove the null segment check for pointer

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

@@ -62,6 +62,9 @@
 import java.lang.foreign.SegmentScope;
 /*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 import java.lang.foreign.ValueLayout;
+/*[IF JAVA_SPEC_VERSION >= 20]*/
+import static java.lang.foreign.ValueLayout.*;
+/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 import java.lang.foreign.VaList;
 /*[IF JAVA_SPEC_VERSION >= 20]*/
 import jdk.internal.foreign.Utils;
@@ -103,13 +106,6 @@ public class InternalDowncallHandler {
 	/*[IF JAVA_SPEC_VERSION == 16]*/
 	private Addressable functionAddr;
 	/*[ENDIF] JAVA_SPEC_VERSION == 16 */
-	/*[IF JAVA_SPEC_VERSION >= 18]*/
-	/*[IF JAVA_SPEC_VERSION <= 19]*/
-	private static final Class<?> addrClass = Addressable.class;
-	/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
-	/*[ELSE] JAVA_SPEC_VERSION >= 18 */
-	private static final Class<?> addrClass = MemoryAddress.class;
-	/*[ENDIF] JAVA_SPEC_VERSION >= 18 */
 	private long cifNativeThunkAddr;
 	private long argTypesAddr;
 	private MemoryLayout[] argLayoutArray;
@@ -145,9 +141,11 @@ public class InternalDowncallHandler {
 	private static final MethodHandle intToLongArgFilter;
 	private static final MethodHandle floatToLongArgFilter;
 	private static final MethodHandle doubleToLongArgFilter;
-	/*[IF JAVA_SPEC_VERSION <= 19]*/
+	/*[IF JAVA_SPEC_VERSION >= 20]*/
+	private MethodHandle memSegmtOfPtrToLongArgFilter;
+	/*[ELSE] JAVA_SPEC_VERSION >= 20 */
 	private MethodHandle memAddrToLongArgFilter;
-	/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
+	/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 	private MethodHandle memSegmtToLongArgFilter;
 
 	/* Return value filters that convert the Long object to the primitive types/MemoryAddress/MemorySegment. */
@@ -292,13 +290,20 @@ private void validateMemScope(ResourceScope memScope) throws IllegalStateExcepti
 	}
 	/*[ENDIF] JAVA_SPEC_VERSION >= 17 */
 
-	/*[IF JAVA_SPEC_VERSION <= 19]*/
+	/*[IF JAVA_SPEC_VERSION >= 20]*/
+	/* Intended for memSegmtOfPtrToLongArgFilter that converts the memory segment
+	 * of the passed-in pointer argument to long.
+	 */
+	private final long memSegmtOfPtrToLongArg(MemorySegment argValue) throws IllegalStateException {
+		addMemArgScope(argValue.scope());
+		return UpcallMHMetaData.getNativeArgRetSegmentOfPtr(argValue);
+	}
+	/*[ELSEIF JAVA_SPEC_VERSION >= 18] */
 	/* Intended for memAddrToLongArgFilter that converts the memory address to long.
 	 * Note: the passed-in argument can be an instance of MemoryAddress, MemorySegment
 	 * or VaList which extends Addressable in OpenJDK since Java 18 featured with
 	 * JEP419 (Second Incubator).
 	 */
-	/*[IF JAVA_SPEC_VERSION >= 18]*/
 	private final long memAddrToLongArg(Addressable argValue) throws IllegalStateException {
 		/* Only check MemorySegment and VaList given MemoryAddress.scope() doesn't exist in JDK17. */
 		if (argValue instanceof MemorySegment value) {
@@ -319,19 +324,33 @@ else if (argValue instanceof NativeSymbol value) {
 			addMemArgScope(value.scope());
 		}
 		/*[ENDIF] JAVA_SPEC_VERSION == 18 */
-		return argValue.address().toRawLongValue();
+
+		/* Instead of assigning nativeAddr with argValue.address().toRawLongValue() by fefault,
+		 * (which triggers the exception in the case of the on-heap segment in JDK19), the
+		 * argument is validated at first in the case of MemorySegment/MemoryAddress.
+		 */
+		long nativeAddr = 0;
+		if (argValue instanceof MemorySegment value) {
+			nativeAddr = UpcallMHMetaData.getNativeArgRetSegmentOfPtr(value);
+		} else if (argValue instanceof MemoryAddress value) {
+			nativeAddr = UpcallMHMetaData.getNativeArgRetAddrOfPtr(value);
+		} else {
+			nativeAddr = argValue.address().toRawLongValue();
+		}
+		return nativeAddr;
 	}
 	/*[ELSEIF JAVA_SPEC_VERSION == 17]*/
+	/* Intended for memAddrToLongArgFilter that converts the memory address to long. */
 	private final long memAddrToLongArg(MemoryAddress argValue) throws IllegalStateException {
 		addMemArgScope(argValue.scope());
-		return argValue.address().toRawLongValue();
+		return UpcallMHMetaData.getNativeArgRetAddrOfPtr(argValue);
 	}
 	/*[ELSE] JAVA_SPEC_VERSION == 17 */
+	/* Intended for memAddrToLongArgFilter that converts the memory address to long. */
 	private static final long memAddrToLongArg(MemoryAddress argValue) {
 		return argValue.address().toRawLongValue();
 	}
-	/*[ENDIF] JAVA_SPEC_VERSION >= 18 */
-	/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
+	/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 
 	/* Intended for memSegmtToLongArgFilter that converts the memory segment to long. */
 	private final long memSegmtToLongArg(MemorySegment argValue) throws IllegalStateException {
@@ -340,12 +359,7 @@ private final long memSegmtToLongArg(MemorySegment argValue) throws IllegalState
 		/*[ELSE] JAVA_SPEC_VERSION == 19 */
 		addMemArgScope(argValue.scope());
 		/*[ENDIF] JAVA_SPEC_VERSION == 19 */
-
-		/*[IF JAVA_SPEC_VERSION >= 20]*/
-		return argValue.address();
-		/*[ELSE] JAVA_SPEC_VERSION >= 20 */
-		return argValue.address().toRawLongValue();
-		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+		return UpcallMHMetaData.getNativeArgRetSegment(argValue);
 	}
 
 	/* Intended for longObjToVoidRetFilter that converts the Long object to void. */
@@ -500,9 +514,13 @@ public InternalDowncallHandler(Addressable downcallAddr, MethodType functionMeth
 		try {
 			/*[IF JAVA_SPEC_VERSION >= 20]*/
 			longObjToMemSegmtRetFilter = lookup.bind(this, "longObjToMemSegmtRet", methodType(MemorySegment.class, Object.class)); //$NON-NLS-1$
-			/*[ELSE] JAVA_SPEC_VERSION >= 20 */
-			memAddrToLongArgFilter = lookup.bind(this, "memAddrToLongArg", methodType(long.class, addrClass)); //$NON-NLS-1$
+			memSegmtOfPtrToLongArgFilter = lookup.bind(this, "memSegmtOfPtrToLongArg", methodType(long.class, MemorySegment.class)); //$NON-NLS-1$
+			/*[ELSEIF JAVA_SPEC_VERSION >= 18]*/
+			memAddrToLongArgFilter = lookup.bind(this, "memAddrToLongArg", methodType(long.class, Addressable.class)); //$NON-NLS-1$
+			/*[ELSE] JAVA_SPEC_VERSION >= 18 */
+			memAddrToLongArgFilter = lookup.bind(this, "memAddrToLongArg", methodType(long.class, MemoryAddress.class)); //$NON-NLS-1$
 			/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+
 			memSegmtToLongArgFilter = lookup.bind(this, "memSegmtToLongArg", methodType(long.class, MemorySegment.class)); //$NON-NLS-1$
 		} catch (ReflectiveOperationException e) {
 			throw new InternalError(e);
@@ -635,7 +653,7 @@ private MethodHandle permuteMH(MethodHandle targetHandle, MethodType nativeMetho
 		/* Convert the argument values to long via filterArguments() prior to the native call. */
 		MethodHandle[] argFilters = new MethodHandle[nativeArgCount];
 		for (int argIndex = 0; argIndex < nativeArgCount; argIndex++) {
-			argFilters[argIndex] = getArgumentFilter(argTypeClasses[argIndex]);
+			argFilters[argIndex] = getArgumentFilter(argTypeClasses[argIndex], argLayoutArray[argIndex]);
 		}
 		resultHandle = filterArguments(resultHandle, argPosition, argFilters);
 
@@ -655,7 +673,7 @@ private MethodHandle permuteMH(MethodHandle targetHandle, MethodType nativeMetho
 	}
 
 	/* Obtain the filter that converts the passed-in argument to long against its type. */
-	private MethodHandle getArgumentFilter(Class<?> argTypeClass) {
+	private MethodHandle getArgumentFilter(Class<?> argTypeClass, MemoryLayout argLayout) {
 		/* Set the filter to null in the case of long by default as there is no conversion for long. */
 		MethodHandle filterMH = null;
 
@@ -684,7 +702,15 @@ private MethodHandle getArgumentFilter(Class<?> argTypeClass) {
 		} else
 		/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
 		if (argTypeClass == MemorySegment.class) {
-			filterMH = memSegmtToLongArgFilter;
+			/*[IF JAVA_SPEC_VERSION >= 20]*/
+			/* The address layout for pointer might come with different representations of ADDRESS. */
+			if (argLayout instanceof OfAddress) {
+				filterMH = memSegmtOfPtrToLongArgFilter;
+			} else
+			/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+			{
+				filterMH = memSegmtToLongArgFilter;
+			}
 		}
 
 		return filterMH;

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

@@ -27,11 +27,18 @@
 
 /*[IF JAVA_SPEC_VERSION >= 20]*/
 import java.lang.foreign.Arena;
+import java.lang.foreign.MemorySegment;
 import java.lang.foreign.SegmentScope;
 import jdk.internal.foreign.MemorySessionImpl;
 /*[ELSEIF JAVA_SPEC_VERSION == 19]*/
+import java.lang.foreign.Addressable;
+import java.lang.foreign.MemoryAddress;
+import java.lang.foreign.MemorySegment;
 import java.lang.foreign.MemorySession;
 /*[ELSE] JAVA_SPEC_VERSION == 19 */
+import jdk.incubator.foreign.Addressable;
+import jdk.incubator.foreign.MemoryAddress;
+import jdk.incubator.foreign.MemorySegment;
 import jdk.incubator.foreign.ResourceScope;
 /*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 
@@ -54,7 +61,7 @@ final class UpcallMHMetaData {
 	 * by MethodHandleResolver.upcallLinkCallerMethod().
 	 */
 	private Object[] invokeCache;
-	/* The argument array stores the memory specific argument(struct/pointer) object
+	/* The argument array stores the memory specific argument (struct/pointer) object
 	 * being allocated in native for upcall to stop GC from updating the previously
 	 * allocated argument reference when allocating the next argument.
 	 */
@@ -68,14 +75,14 @@ final class UpcallMHMetaData {
 	private ResourceScope scope;
 	/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 
-	private static synchronized native void resolveUpcallDataFields();
+	private static synchronized native void resolveUpcallDataInfo();
 
 	static {
-		/* Resolve the fields (offset in the JCL constant pool of VM) specific to the metadata plus the fields
-		 * of MemoryAddressImpl and NativeMemorySegmentImpl given the generated macros from vmconstantpool.xml
-		 * depend on their offsets to access the corresponding fields in the process of the upcall.
+		/* Resolve the methods/fields (offset in the JCL constant pool of VM) related to the metadata
+		 * given the generated macros from vmconstantpool.xml depend on their offsets to access the
+		 * corresponding methods/fields in the process of the upcall.
 		 */
-		resolveUpcallDataFields();
+		resolveUpcallDataInfo();
 	}
 
 	/*[IF JAVA_SPEC_VERSION >= 20]*/
@@ -101,4 +108,82 @@ final class UpcallMHMetaData {
 		this.scope = ((scope != null) && (scope.ownerThread() != null)) ? scope : ResourceScope.newSharedScope();
 		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 	}
+
+	/* Determine whether the memory segment of the passed-in/returned pointer is allocated
+	 * in the native memory or not and return its native address if valid.
+	 *
+	 * Note:
+	 * The method is shared in java (downcall) and in native (upcall) via the calling-in from the dispatcher.
+	 */
+	static long getNativeArgRetSegmentOfPtr(MemorySegment argRetSegmentOfPtr) {
+		if (!argRetSegmentOfPtr.isNative()) {
+			throw new IllegalArgumentException("Heap segment not allowed: " + argRetSegmentOfPtr);
+		}
+
+		/*[IF JAVA_SPEC_VERSION >= 20]*/
+		return argRetSegmentOfPtr.address();
+		/*[ELSE] JAVA_SPEC_VERSION >= 20 */
+		return argRetSegmentOfPtr.address().toRawLongValue();
+		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+	}
+
+	/*[IF JAVA_SPEC_VERSION <= 19]*/
+	/* Determine whether the memory address of the passed-in/returned pointer is allocated
+	 * in the native memory or not and return its native address if valid.
+	 *
+	 * Note:
+	 * The method is shared in java (downcall) and in native (upcall) via the calling-in from the dispatcher.
+	 */
+	static long getNativeArgRetAddrOfPtr(MemoryAddress argRetAddrOfPtr) {
+		/*[IF JAVA_SPEC_VERSION > 17]*/
+		/* Validate the native address as MemoryAddress.isNative() is removed in JDK18/19. */
+		if (argRetAddrOfPtr.toRawLongValue() == 0)
+		/*[ELSE] JAVA_SPEC_VERSION > 17 */
+		if (!argRetAddrOfPtr.isNative())
+		/*[ENDIF] JAVA_SPEC_VERSION > 17 */
+		{
+			throw new IllegalArgumentException("A heap address is not allowed: " + argRetAddrOfPtr);
+		}
+
+		return argRetAddrOfPtr.toRawLongValue();
+	}
+	/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
+
+	/* Determine whether the passed-in/returned segment is allocated in the native memory or not
+	 * and return its native address if valid; otherwise, return the address of an newly allocated
+	 * native segment with all values copied from the heap segment.
+	 *
+	 * Note:
+	 * The method is shared in java (downcall) and in native (upcall) via the calling-in from the dispatcher.
+	 */
+	static long getNativeArgRetSegment(MemorySegment argRetSegment) {
+		/*[IF JAVA_SPEC_VERSION >= 20]*/
+		/* MemorySegment.NULL is introduced since JDK20+. */
+		if (argRetSegment == MemorySegment.NULL) {
+			throw new NullPointerException("A NULL memory segment is not allowed for struct.");
+		}
+		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+		MemorySegment nativeSegment = argRetSegment;
+
+		/* Copy all values in the heap segment to a newly allocated native segment
+		 * given a heap segment with a zero address can't be accessed in native.
+		 */
+		if (!argRetSegment.isNative()) {
+			/*[IF JAVA_SPEC_VERSION >= 20]*/
+			SegmentScope scope = SegmentScope.global();
+			/*[ELSEIF JAVA_SPEC_VERSION == 19]*/
+			MemorySession scope = MemorySession.global();
+			/*[ELSE] JAVA_SPEC_VERSION == 19 */
+			ResourceScope scope = ResourceScope.globalScope();
+			/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+			nativeSegment = MemorySegment.allocateNative(argRetSegment.byteSize(), scope);
+			nativeSegment.copyFrom(argRetSegment);
+		}
+
+		/*[IF JAVA_SPEC_VERSION >= 20]*/
+		return nativeSegment.address();
+		/*[ELSE] JAVA_SPEC_VERSION >= 20 */
+		return nativeSegment.address().toRawLongValue();
+		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
+	}
 }

runtime/oti/vmconstantpool.xml

@@ -481,6 +481,11 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-excepti
 
 	<!-- Static method references needed to support Foreign Linker API. -->
 	<staticmethodref class="java/lang/invoke/MethodHandleResolver" name="upcallLinkCallerMethod" signature="(Ljava/lang/Class;Ljava/lang/invoke/MethodType;)Ljava/lang/Object;" versions="16-"/>
+	<staticmethodref class="openj9/internal/foreign/abi/UpcallMHMetaData" name="getNativeArgRetAddrOfPtr" signature="(Ljdk/incubator/foreign/MemoryAddress;)J" flags="opt_openjdkFfi" versions="16-18"/>
+	<staticmethodref class="openj9/internal/foreign/abi/UpcallMHMetaData" name="getNativeArgRetAddrOfPtr" signature="(Ljava/lang/foreign/MemoryAddress;)J" flags="opt_openjdkFfi" versions="19"/>
+	<staticmethodref class="openj9/internal/foreign/abi/UpcallMHMetaData" name="getNativeArgRetSegmentOfPtr" signature="(Ljava/lang/foreign/MemorySegment;)J" flags="opt_openjdkFfi" versions="20-"/>
+	<staticmethodref class="openj9/internal/foreign/abi/UpcallMHMetaData" name="getNativeArgRetSegment" signature="(Ljdk/incubator/foreign/MemorySegment;)J" flags="opt_openjdkFfi" versions="16-18"/>
+	<staticmethodref class="openj9/internal/foreign/abi/UpcallMHMetaData" name="getNativeArgRetSegment" signature="(Ljava/lang/foreign/MemorySegment;)J" flags="opt_openjdkFfi" versions="19-"/>
 
 	<staticmethodref class="jdk/internal/loader/NativeLibraries" name="load" signature="(Ljdk/internal/loader/NativeLibraries$NativeLibraryImpl;Ljava/lang/String;ZZZ)Z" versions="15-18"/>
 	<staticmethodref class="jdk/internal/loader/NativeLibraries" name="load" signature="(Ljdk/internal/loader/NativeLibraries$NativeLibraryImpl;Ljava/lang/String;ZZ)Z" versions="19-"/>

runtime/tests/clinkerffi/CMakeLists.txt

@@ -164,6 +164,7 @@ omr_add_exports(clinkerffitests
 	add2DoubleStructs_returnStruct
 	add2DoubleStructs_returnStructPointer
 	add3DoubleStructs_returnStruct
+	validateNullAddrArgument
 	add2BoolsWithOrByUpcallMH
 	addBoolAndBoolFromPointerWithOrByUpcallMH
 	addBoolAndBoolFromNativePtrWithOrByUpcallMH
@@ -341,6 +342,7 @@ omr_add_exports(clinkerffitests
 	addDoubleAndIntDoubleLongFromStructByUpcallMH
 	return254BytesFromStructByUpcallMH
 	return4KBytesFromStructByUpcallMH
+	validateReturnNullAddrByUpcallMH
 	addIntsFromVaList
 	addLongsFromVaList
 	addDoublesFromVaList

runtime/tests/clinkerffi/downcall.c

@@ -1990,3 +1990,20 @@ add3DoubleStructs_returnStruct(stru_Double_Double_Double arg1, stru_Double_Doubl
 	doubleStruct.elem3 = arg1.elem3 + arg2.elem3;
 	return doubleStruct;
 }
+
+/**
+ * Validate that a null pointer of struct is successfully passed
+ * from java to native in downcall.
+ *
+ * @param arg1 an integer
+ * @param arg2 a pointer to struct with two integers
+ * @return the value of arg1
+ *
+ * Note:
+ * arg2 is a null pointer passed from java to native.
+ */
+int
+validateNullAddrArgument(int arg1, stru_Int_Int *arg2)
+{
+	return arg1;
+}

runtime/tests/clinkerffi/module.xml

@@ -155,6 +155,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-excepti
 		<export name="add2DoubleStructs_returnStruct"/>
 		<export name="add2DoubleStructs_returnStructPointer"/>
 		<export name="add3DoubleStructs_returnStruct"/>
+		<export name="validateNullAddrArgument"/>
 		<export name="add2BoolsWithOrByUpcallMH"/>
 		<export name="addBoolAndBoolFromPointerWithOrByUpcallMH"/>
 		<export name="addBoolAndBoolFromNativePtrWithOrByUpcallMH"/>
@@ -332,6 +333,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-excepti
 		<export name="addDoubleAndIntDoubleLongFromStructByUpcallMH"/>
 		<export name="return254BytesFromStructByUpcallMH"/>
 		<export name="return4KBytesFromStructByUpcallMH"/>
+		<export name="validateReturnNullAddrByUpcallMH"/>
 		<export name="addIntsFromVaList"/>
 		<export name="addLongsFromVaList"/>
 		<export name="addDoublesFromVaList"/>

runtime/tests/clinkerffi/upcall.c

@@ -2847,3 +2847,22 @@ return4KBytesFromStructByUpcallMH(stru_4K_Bytes (*upcallMH)())
 {
 	return (*upcallMH)();
 }
+
+/**
+ * Validate that a null pointer is successfully returned
+ * from the upcall method to native.
+ *
+ * @param arg1 a pointer to the 1st struct with two ints
+ * @param arg2 the 2nd struct with two ints
+ * @param upcallMH the function pointer to the upcall method
+ * @return a pointer to struct with two ints (arg1)
+ *
+ * Note:
+ * A null pointer is returned from upcallMH().
+ */
+stru_Int_Int *
+validateReturnNullAddrByUpcallMH(stru_Int_Int *arg1, stru_Int_Int arg2, stru_Int_Int * (*upcallMH)(stru_Int_Int *, stru_Int_Int))
+{
+	(*upcallMH)(arg1, arg2);
+	return arg1;
+}

runtime/vm/OutOfLineINL.hpp

@@ -47,7 +47,7 @@ J9OutOfLineINLMethod OutOfLineINL_java_lang_invoke_NativeMethodHandle_freeJ9Nati
 J9OutOfLineINLMethod OutOfLineINL_openj9_internal_foreign_abi_InternalDowncallHandler_resolveRequiredFields;
 J9OutOfLineINLMethod OutOfLineINL_openj9_internal_foreign_abi_InternalDowncallHandler_initCifNativeThunkData;
 J9OutOfLineINLMethod OutOfLineINL_openj9_internal_foreign_abi_InternalUpcallHandler_allocateUpcallStub;
-J9OutOfLineINLMethod OutOfLineINL_openj9_internal_foreign_abi_UpcallMHMetaData_resolveUpcallDataFields;
+J9OutOfLineINLMethod OutOfLineINL_openj9_internal_foreign_abi_UpcallMHMetaData_resolveUpcallDataInfo;
 #endif /* JAVA_SPEC_VERSION >= 16 */
 }