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