Hi Nick,
thanks for the fixes - this generally looks good - the only bit I'm less
sure of is the Unsafe::allocateMemory change. It looks good per se, but
I guess a more targeted fix is also possible (e.g. by checking for size
> Integer.MAX_VALUE in case address size is 4 when we allocate the
native segment).
I will leave the decision on what to do on this specific issue to others
more versed than me in that part of the code.
As for integrating this - what are your plans? JDK 14 or panama repo? I
see you have targeted 14 in the issue, which is fine, but the priority
is P4. Needs to be at least a P3 to be pushed in the RDP1 period (and I
think a case can be made for it being a P3 - since it's desirable for
tests to pass on all platforms). Note that while we have no priority
requirements on test/doc bugs (all can be fixed under RDP1/2), there
requirement kicks in here because of your change to Unsafe (or the
equivalent change to the memory access API implementation).
An alternative would be to fix all the tests and keeping existing
priority (and maybe fix the testTooBigForArrays so that it does nothing
on 32-bits) - and then deciding what to do with Unsafe::allocateMemory
in a separate followup issue.
Maurizio
On 07/01/2020 08:21, Nick Gasson wrote:
Hi,
Please review this set of fixes for the new memory access API on
32-bit Arm/x86.
Bug: https://bugs.openjdk.java.net/browse/JDK-8236634
Webrev: http://cr.openjdk.java.net/~ngasson/8236634/webrev.0/
libNativeAccess.c fails to build with warnings enabled due to casting
pointers to integers of different widths. Added an intermediate cast to
uintptr_t to silence this.
TestArrays::testTooBigForArray calls MemorySegment.allocateNative with
size Integer.MAX_VALUE * 2 (0xFFFF_FFFE). This gets passed to
Unsafe_AllocateMemory0 which aligns the size up to HeapWordSize. But
on a 32-bit system this overflows the value of size_t, resulting in a
call to os::malloc(0). Added an overflow check after the call to
align_up().
MemorySegment::makeNativeSegment assumes that the result of
Unsafe::allocateMemory will always be at least 16 byte aligned. But on a
32-bit system the result can be 8 byte aligned. Made the MAX_ALIGN
constant conditional on the address size.
Several of the tests in TestByteBuffer.java allocate a byte[] array on
the heap and then read or write Java long/double from the array base
address. On a 32-bit system the array data is located immediately after
12 bytes of header so these eight byte accesses are misaligned. I made a
minimal fix here to just skip the long/double cases on 32-bit systems.
Tested jdk_foreign on x86_32, arm32, x86_64, and aarch64.
Thanks,
Nick
