On Wed, 22 Mar 2023 20:07:01 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Ah. True. This is true about all the MS parameters as well.
>
> Yes - although there is a distinction between passed by value and by ref. I
> wouldn't be surprised if we just threw NPE for by-value.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1145978702
