Skip to content

Commit

Permalink
Merge pull request #17408 from ChengJin01/ffi_remove_null_seg_check_f…
Browse files Browse the repository at this point in the history 
…rom_ptr

[Jtreg/FFI] Remove the null segment check for pointer
  • Loading branch information
@tajila
tajila committed May 16, 2023
2 parents 826d49a + 21851c1 commit 22b17b8
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 31 deletions.
3 changes: 2 additions & 1 deletion jcl/src/java.base/share/classes/openj9/internal/foreign/abi/InternalDowncallHandler.java
View file
Expand Up @@ -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 */
Expand Down
13 changes: 1 addition & 12 deletions jcl/src/java.base/share/classes/openj9/internal/foreign/abi/UpcallMHMetaData.java
View file
Expand Up @@ -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);
}
Expand All @@ -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)
Expand All @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions runtime/tests/clinkerffi/CMakeLists.txt
View file
Expand Up @@ -164,6 +164,7 @@ omr_add_exports(clinkerffitests
add2DoubleStructs_returnStruct
add2DoubleStructs_returnStructPointer
add3DoubleStructs_returnStruct
validateNullAddrArgument
add2BoolsWithOrByUpcallMH
addBoolAndBoolFromPointerWithOrByUpcallMH
addBoolAndBoolFromNativePtrWithOrByUpcallMH
Expand Down Expand Up @@ -341,6 +342,7 @@ omr_add_exports(clinkerffitests
addDoubleAndIntDoubleLongFromStructByUpcallMH
return254BytesFromStructByUpcallMH
return4KBytesFromStructByUpcallMH
validateReturnNullAddrByUpcallMH
addIntsFromVaList
addLongsFromVaList
addDoublesFromVaList
Expand Down
17 changes: 17 additions & 0 deletions runtime/tests/clinkerffi/downcall.c
View file
Expand Up @@ -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;
}
2 changes: 2 additions & 0 deletions runtime/tests/clinkerffi/module.xml
View file
Expand Up @@ -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"/>
Expand Down Expand Up @@ -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"/>
Expand Down
19 changes: 19 additions & 0 deletions runtime/tests/clinkerffi/upcall.c
View file
Expand Up @@ -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;
}
5 changes: 2 additions & 3 deletions .../functional/Java17andUp/src_170/org/openj9/test/jep389/downcall/InvalidDownCallTests.java
View file
Expand Up @@ -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.*")
Expand Down
7 changes: 4 additions & 3 deletions test/functional/Java17andUp/src_170/org/openj9/test/jep389/upcall/InvalidUpCallTests.java
View file
Expand Up @@ -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()) {
Expand All @@ -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);
}
}

Expand Down
5 changes: 2 additions & 3 deletions test/functional/Java20andUp/src/org/openj9/test/jep434/downcall/InvalidDownCallTests.java
View file
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions test/functional/Java20andUp/src/org/openj9/test/jep434/upcall/InvalidUpCallTests.java
View file
Expand Up @@ -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()) {
Expand All @@ -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);
}
}

Expand Down
5 changes: 2 additions & 3 deletions test/functional/Java21andUp/src/org/openj9/test/jep442/downcall/InvalidDownCallTests.java
View file
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions test/functional/Java21andUp/src/org/openj9/test/jep442/upcall/InvalidUpCallTests.java
View file
Expand Up @@ -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()) {
Expand All @@ -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);
}
}

Expand Down

0 comments on commit 22b17b8

Please sign in to comment.