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

The changes remove the check on MemoryAddress.NULL(JDK17) MemorySegment.NULL(JDK20) for the pointer argument/return value as this type of segment/address is acceptable in the native function, depending upon how the segment/address is used. Fixes: eclipse-openj9#17400 Signed-off-by: ChengJin01 <[email protected]>
midronij · Jun 1, 2023 · 0b04292 · 0b04292
1 parent 342bc45
commit 0b04292
Show file tree

Hide file tree

Showing 12 changed files with 61 additions and 31 deletions.
diff --git a/jcl/src/java.base/share/classes/openj9/internal/foreign/abi/InternalDowncallHandler.java b/jcl/src/java.base/share/classes/openj9/internal/foreign/abi/InternalDowncallHandler.java
@@ -703,7 +703,8 @@ private MethodHandle getArgumentFilter(Class<?> argTypeClass, MemoryLayout argLa
 		/*[ENDIF] JAVA_SPEC_VERSION <= 19 */
 		if (argTypeClass == MemorySegment.class) {
 			/*[IF JAVA_SPEC_VERSION >= 20]*/
-			if (argLayout == ADDRESS) {
+			/* The address layout for pointer might come with different representations of ADDRESS. */
+			if (argLayout instanceof OfAddress) {
 				filterMH = memSegmtOfPtrToLongArgFilter;
 			} else
 			/*[ENDIF] JAVA_SPEC_VERSION >= 20 */

diff --git a/jcl/src/java.base/share/classes/openj9/internal/foreign/abi/UpcallMHMetaData.java b/jcl/src/java.base/share/classes/openj9/internal/foreign/abi/UpcallMHMetaData.java
@@ -116,12 +116,6 @@ final class UpcallMHMetaData {
 	 * The method is shared in java (downcall) and in native (upcall) via the calling-in from the dispatcher.
 	 */
 	static long getNativeArgRetSegmentOfPtr(MemorySegment argRetSegmentOfPtr) {
-		/*[IF JAVA_SPEC_VERSION >= 20]*/
-		/* Verify MemorySegment.NULL as it is introduced since JDK20. */
-		if (argRetSegmentOfPtr == MemorySegment.NULL) {
-			throw new NullPointerException("A NULL memory segment is not allowed for pointer.");
-		}
-		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */
 		if (!argRetSegmentOfPtr.isNative()) {
 			throw new IllegalArgumentException("Heap segment not allowed: " + argRetSegmentOfPtr);
 		}
@@ -141,10 +135,6 @@ static long getNativeArgRetSegmentOfPtr(MemorySegment argRetSegmentOfPtr) {
 	 * The method is shared in java (downcall) and in native (upcall) via the calling-in from the dispatcher.
 	 */
 	static long getNativeArgRetAddrOfPtr(MemoryAddress argRetAddrOfPtr) {
-		if (argRetAddrOfPtr == MemoryAddress.NULL) {
-			throw new NullPointerException("A NULL memory address is not allowed for pointer.");
-		}
-
 		/*[IF JAVA_SPEC_VERSION > 17]*/
 		/* Validate the native address as MemoryAddress.isNative() is removed in JDK18/19. */
 		if (argRetAddrOfPtr.toRawLongValue() == 0)
@@ -169,8 +159,7 @@ static long getNativeArgRetAddrOfPtr(MemoryAddress argRetAddrOfPtr) {
 	static long getNativeArgRetSegment(MemorySegment argRetSegment) {
 		/*[IF JAVA_SPEC_VERSION >= 20]*/
 		/* MemorySegment.NULL is introduced since JDK20+. */
-		if (argRetSegment == MemorySegment.NULL)
-		{
+		if (argRetSegment == MemorySegment.NULL) {
 			throw new NullPointerException("A NULL memory segment is not allowed for struct.");
 		}
 		/*[ENDIF] JAVA_SPEC_VERSION >= 20 */

diff --git a/runtime/tests/clinkerffi/CMakeLists.txt b/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

diff --git a/runtime/tests/clinkerffi/downcall.c b/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;
+}
diff --git a/runtime/tests/clinkerffi/module.xml b/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"/>

diff --git a/runtime/tests/clinkerffi/upcall.c b/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;
+}
diff --git a/.../functional/Java17andUp/src_170/org/openj9/test/jep389/downcall/InvalidDownCallTests.java b/.../functional/Java17andUp/src_170/org/openj9/test/jep389/downcall/InvalidDownCallTests.java
@@ -436,19 +436,18 @@ public void test_nullValueForStructArgument() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class)
 	public void test_nullSegmentForPtrArgument() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(C_INT.withName("elem1"), C_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(int.class, PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(int.class, PathElement.groupElement("elem2"));
 
 		MethodType mt = MethodType.methodType(int.class, int.class, MemoryAddress.class);
 		FunctionDescriptor fd = FunctionDescriptor.of(C_INT, C_INT, C_POINTER);
-		Addressable functionSymbol = nativeLibLookup.lookup("addIntAndIntsFromStructPointer").get();
+		Addressable functionSymbol = nativeLibLookup.lookup("validateNullAddrArgument").get();
 		MethodHandle mh = clinker.downcallHandle(functionSymbol, mt, fd);
 
 		int result = (int)mh.invoke(19202122, MemoryAddress.NULL);
-		fail("Failed to throw out NullPointerException in the case of the null memory address");
+		Assert.assertEquals(result, 19202122);
 	}
 
 	@Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "A heap address is not allowed.*")

diff --git a/test/functional/Java17andUp/src_170/org/openj9/test/jep389/upcall/InvalidUpCallTests.java b/test/functional/Java17andUp/src_170/org/openj9/test/jep389/upcall/InvalidUpCallTests.java
@@ -170,15 +170,14 @@ public void test_nullValueForReturnStruct() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class, expectedExceptionsMessageRegExp = "A NULL memory address is not allowed for pointer.*")
 	public void test_nullAddrForReturnPtr() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(C_INT.withName("elem1"), C_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(int.class, PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(int.class, PathElement.groupElement("elem2"));
 
 		MethodType mt = MethodType.methodType(MemoryAddress.class, MemoryAddress.class, MemorySegment.class, MemoryAddress.class);
 		FunctionDescriptor fd = FunctionDescriptor.of(C_POINTER, C_POINTER, structLayout, C_POINTER);
-		Addressable functionSymbol = nativeLibLookup.lookup("add2IntStructs_returnStructPointerByUpcallMH").get();
+		Addressable functionSymbol = nativeLibLookup.lookup("validateReturnNullAddrByUpcallMH").get();
 		MethodHandle mh = clinker.downcallHandle(functionSymbol, mt, fd);
 
 		try (ResourceScope scope = ResourceScope.newConfinedScope()) {
@@ -193,7 +192,9 @@ public void test_nullAddrForReturnPtr() throws Throwable {
 			intHandle2.set(structSegmt2, 33445566);
 
 			MemoryAddress resultAddr = (MemoryAddress)mh.invokeExact(structSegmt1.address(), structSegmt2, upcallFuncAddr);
-			fail("Failed to throw out NullPointerException in the case of the null pointer upon return");
+			MemorySegment resultSegmt = resultAddr.asSegment(structLayout.byteSize(), scope);
+			Assert.assertEquals(intHandle1.get(resultSegmt), 11223344);
+			Assert.assertEquals(intHandle2.get(resultSegmt), 55667788);
 		}
 	}
 

diff --git a/test/functional/Java20andUp/src/org/openj9/test/jep434/downcall/InvalidDownCallTests.java b/test/functional/Java20andUp/src/org/openj9/test/jep434/downcall/InvalidDownCallTests.java
@@ -119,18 +119,17 @@ public void test_nullValueForStructArgument() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class)
 	public void test_nullSegmentForPtrArgument() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(JAVA_INT.withName("elem1"), JAVA_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(PathElement.groupElement("elem2"));
 
 		FunctionDescriptor fd = FunctionDescriptor.of(JAVA_INT, JAVA_INT, ADDRESS);
-		MemorySegment functionSymbol = nativeLibLookup.find("addIntAndIntsFromStructPointer").get();
+		MemorySegment functionSymbol = nativeLibLookup.find("validateNullAddrArgument").get();
 		MethodHandle mh = linker.downcallHandle(functionSymbol, fd);
 
 		int result = (int)mh.invoke(19202122, MemorySegment.NULL);
-		fail("Failed to throw out NullPointerException in the case of the null segment");
+		Assert.assertEquals(result, 19202122);
 	}
 
 	@Test(expectedExceptions = NullPointerException.class)

diff --git a/test/functional/Java20andUp/src/org/openj9/test/jep434/upcall/InvalidUpCallTests.java b/test/functional/Java20andUp/src/org/openj9/test/jep434/upcall/InvalidUpCallTests.java
@@ -159,14 +159,13 @@ public void test_nullValueForReturnStruct() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class)
 	public void test_nullSegmentForReturnPtr() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(JAVA_INT.withName("elem1"), JAVA_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(PathElement.groupElement("elem2"));
 
 		FunctionDescriptor fd = FunctionDescriptor.of(ADDRESS, ADDRESS, structLayout, ADDRESS);
-		MemorySegment functionSymbol = nativeLibLookup.find("add2IntStructs_returnStructPointerByUpcallMH").get();
+		MemorySegment functionSymbol = nativeLibLookup.find("validateReturnNullAddrByUpcallMH").get();
 		MethodHandle mh = linker.downcallHandle(functionSymbol, fd);
 
 		try (Arena arena = Arena.openConfined()) {
@@ -181,7 +180,9 @@ public void test_nullSegmentForReturnPtr() throws Throwable {
 			intHandle2.set(structSegmt2, 33445566);
 
 			MemorySegment resultAddr = (MemorySegment)mh.invoke(structSegmt1, structSegmt2, upcallFuncAddr);
-			fail("Failed to throw out NullPointerException in the case of the null segment upon return");
+			MemorySegment resultSegmt = MemorySegment.ofAddress(resultAddr.address(), structLayout.byteSize(), arena.scope());
+			Assert.assertEquals(resultSegmt.get(JAVA_INT, 0), 11223344);
+			Assert.assertEquals(resultSegmt.get(JAVA_INT, 4), 55667788);
 		}
 	}
 

diff --git a/test/functional/Java21andUp/src/org/openj9/test/jep442/downcall/InvalidDownCallTests.java b/test/functional/Java21andUp/src/org/openj9/test/jep442/downcall/InvalidDownCallTests.java
@@ -119,18 +119,17 @@ public void test_nullValueForStructArgument() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class)
 	public void test_nullSegmentForPtrArgument() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(JAVA_INT.withName("elem1"), JAVA_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(PathElement.groupElement("elem2"));
 
 		FunctionDescriptor fd = FunctionDescriptor.of(JAVA_INT, JAVA_INT, ADDRESS);
-		MemorySegment functionSymbol = nativeLibLookup.find("addIntAndIntsFromStructPointer").get();
+		MemorySegment functionSymbol = nativeLibLookup.find("validateNullAddrArgument").get();
 		MethodHandle mh = linker.downcallHandle(functionSymbol, fd);
 
 		int result = (int)mh.invoke(19202122, MemorySegment.NULL);
-		fail("Failed to throw out NullPointerException in the case of the null segment");
+		Assert.assertEquals(result, 19202122);
 	}
 
 	@Test(expectedExceptions = NullPointerException.class)

diff --git a/test/functional/Java21andUp/src/org/openj9/test/jep442/upcall/InvalidUpCallTests.java b/test/functional/Java21andUp/src/org/openj9/test/jep442/upcall/InvalidUpCallTests.java
@@ -159,14 +159,13 @@ public void test_nullValueForReturnStruct() throws Throwable {
 		}
 	}
 
-	@Test(expectedExceptions = NullPointerException.class)
 	public void test_nullSegmentForReturnPtr() throws Throwable {
 		GroupLayout structLayout = MemoryLayout.structLayout(JAVA_INT.withName("elem1"), JAVA_INT.withName("elem2"));
 		VarHandle intHandle1 = structLayout.varHandle(PathElement.groupElement("elem1"));
 		VarHandle intHandle2 = structLayout.varHandle(PathElement.groupElement("elem2"));
 
 		FunctionDescriptor fd = FunctionDescriptor.of(ADDRESS, ADDRESS, structLayout, ADDRESS);
-		MemorySegment functionSymbol = nativeLibLookup.find("add2IntStructs_returnStructPointerByUpcallMH").get();
+		MemorySegment functionSymbol = nativeLibLookup.find("validateReturnNullAddrByUpcallMH").get();
 		MethodHandle mh = linker.downcallHandle(functionSymbol, fd);
 
 		try (Arena arena = Arena.openConfined()) {
@@ -181,7 +180,9 @@ public void test_nullSegmentForReturnPtr() throws Throwable {
 			intHandle2.set(structSegmt2, 33445566);
 
 			MemorySegment resultAddr = (MemorySegment)mh.invoke(structSegmt1, structSegmt2, upcallFuncAddr);
-			fail("Failed to throw out NullPointerException in the case of the null segment upon return");
+			MemorySegment resultSegmt = MemorySegment.ofAddress(resultAddr.address(), structLayout.byteSize(), arena.scope());
+			Assert.assertEquals(resultSegmt.get(JAVA_INT, 0), 11223344);
+			Assert.assertEquals(resultSegmt.get(JAVA_INT, 4), 55667788);
 		}
 	}