On Wed, 19 Nov 2025 14:30:17 GMT, Maurizio Cimadamore <[email protected]> wrote:
>> Harald Eilertsen has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Replace conditional with Math.max intrinsic >> >> Co-authored-by: ExE Boss <[email protected]> > > I think the proposed simpler patch makes sense. Basically, it artificially > expands the allocation size to match the alignment, which is probably not > going to cause any side-effect as that is likely what was happening anyway... > @mcimadamore: > > Question regarding `MAX_MALLOC_ALIGN = Unsafe.ADDRESS_SIZE == 4 ? 8 : 16;` > > Is 16 here just a heuristic based on known malloc implementations or is it > backed by something? I can't find anything regarding that in the libc > standard. > > Not related to this particular PR: should `MAX_MALLOC_ALIGN` be > `MIN_MALLOC_ALIGN` instead? Larger allocations might be page size aligned. I think this is a common assumption on malloc/Linux: https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html > The address of a block returned by malloc or realloc in GNU systems is always > a multiple of eight (or sixteen on 64-bit systems) I believe the underlying reason has to do with the system ABI -- e.g. what malloc returns has to provide at least enough alignment as required for the fundamental types supported by the ABI. The main question raised in this PR is whether malloc is technically also allowed to return things that are less aligned than the size of the max supported data type in the ABI. Here is where the C standard changed a bit -- as in C23 is now a bit clearer that, when allocating, say, 4 bytes, malloc is free to return a memory region that is aligned to 4 bytes, not 8 or 16. This is because if you take the `void*` returned by `malloc` you might cast it to `int*` or `char*` and expect it to work fine. If you try and cast it to `long*`, while the type system lets you do that, you will end up with undefined behavior anyway (as the allocated region is not big enough to read/write longs)... which is why (I think) the clarification was added. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28235#issuecomment-3557507640
