RFR: 8236634: Memory Access API tests fail on 32-bit

[Maurizio Cimadamore]

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