On Thu, 23 Mar 2023 10:31:27 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Just did a test with jshell:
>>
>>
>> jshell> Linker linker = Linker.nativeLinker();
>>
>> jshell> MethodHandle handleValue =
>> linker.downcallHandle(FunctionDescriptor.ofVoid(MemoryLayout.structLayout(ValueLayout.JAVA_INT)));
>> handleValue ==> MethodHandle(MemorySegment,MemorySegment)void
>>
>> jshell> handleValue.invoke(MemorySegment.ofAddress(1), null);
>> | Exception java.lang.NullPointerException
>> | at (#5:1)
>>
>>
>> So, yes, passing `null` for a "by-value" struct issues NPEs. I also get same
>> exception when calling a method handle which passes a struct by reference:
>>
>>
>> jshell> MethodHandle handleRef =
>> linker.downcallHandle(FunctionDescriptor.ofVoid(ValueLayout.ADDRESS));
>> handleRef ==> MethodHandle(MemorySegment,MemorySegment)void
>>
>> jshell> handleRef.invoke(MemorySegment.ofAddress(1), null);
>> | Exception java.lang.NullPointerException
>> | at (#7:1)
>>
>>
>> Given what the impl does, to be fair I don't mind the NPE (it seems a good
>> exception to throw if we're passing a raw `null`). But we should document it
>> at such.
>>
>> It also seems like the first parameter of the downcall has somewhat better
>> checks (but still NPEs):
>>
>>
>> jshell> handleRef.invoke(null, MemorySegment.ofAddress(1));
>> | Exception java.lang.NullPointerException
>> | at Objects.requireNonNull (Objects.java:233)
>> | at SharedUtils.checkSymbol (SharedUtils.java:351)
>> | at (#8:1)
>>
>>
>> And:
>>
>>
>> jshell> handleRef.invoke(MemorySegment.NULL, MemorySegment.ofAddress(1));
>> | Exception java.lang.IllegalArgumentException: Symbol is NULL:
>> MemorySegment{ heapBase: Optional.empty address:0 limit: 0 }
>> | at SharedUtils.checkSymbol (SharedUtils.java:353)
>> | at (#9:1)
>>
>>
>> I think this last case is the _only_ one where IAE is warranted - as the
>> role of the first parameter is special, and we can't tolerate a
>> `MemorySegment::NULL` there. For all other reference parameters, the
>> downcall handle should just check that they are non-null, and throw if so.
>
> The returned method handle will throw an {@link IllegalArgumentException} if
> the {@link MemorySegment}
> representing the target address of the foreign function is the {@link
> MemorySegment#NULL} address.
> The returned method handle will additionally throw {@link
> NullPointerException} if any parameter of type {@link MemorySegment} is
> {@code null}.
We should also NPE when the passed `SegmentAllocator` for struct returns is
`null`, so for the last sentence I think this should say:
The returned method handle will additionally throw {@link NullPointerException}
if any argument passed to it is {@code null}.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1146264320
