On Sun, 16 Nov 2025 18:09:47 GMT, Harald Eilertsen <[email protected]> wrote:
>> `jdk.internal.foreign.SegmentFactories::allocateNativeInternal` assumes that
>> the underlying implementation of malloc aligns allocations on 16 byte
>> boundaries for 64 bit platforms, and 8 byte boundaries on 32 bit platforms.
>> So for any allocation where the requested alignment is less than or equal to
>> this default alignment it makes no adjustment.
>>
>> However, this assumption does not hold for all allocators. Specifically
>> jemallc, used by libc on FreeBSD will align small allocations on 8 or 4 byte
>> boundaries, respectively. This causes allocateNativeInternal to sometimes
>> return memory that is not properly aligned when the requested alignment is
>> exactly 16 bytes.
>>
>> To make sure we honour the requested alignment when it exaclty matches the
>> quantum as defined by MAX_MALLOC_ALIGN, this patch ensures that we adjust
>> the alignment also in this case.
>>
>> This should make no difference for platforms where malloc allready aligns on
>> the quantum, except for a few unnecessary trivial calculations.
>>
>> This work was sponsored by: The FreeBSD Foundation
>
> Harald Eilertsen has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix calculation of allocationSize when byteAlignment < MAX_MALLOC_ALIGN
>
> This work was sponsored by: The FreeBSD Foundation
src/java.base/share/classes/jdk/internal/foreign/SegmentFactories.java line 207:
> 205: long result;
> 206: if (byteAlignment > MAX_MALLOC_ALIGN || alignedSize %
> byteAlignment != 0) {
> 207: allocationSize = alignedSize + byteAlignment -
> Math.min(MAX_MALLOC_ALIGN, alignedSize);
This doesn't look correct to me. Let's say we want to do a 7 byte allocation
(with `init == false`), aligned to 4. We get `7 % 4 -> 3` so we enter this
branch, and then `allocationSize = 7 + 4 - 7`, and we end up allocating only 4
bytes. The previous computation only works if `MAX_MALLOC_ALIGN` is greater
than `byteAlignment`.
I think it's simpler to just replace `MAX_MALLOC_ALIGN` in this `if` statement
with an 'expected alignment' derived from the size.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28235#discussion_r2534441590
