RFR: 8346916: [REDO] align_up has potential overflow [v8]

[Kim Barrett]

On Wed, 5 Mar 2025 14:13:36 GMT, Casper Norrbin <cnorrbin at openjdk.org> wrote:

>> Hi everyone,
>> 
>> The `align_up` function can potentially overflow, resulting in undefined behavior. Most use cases rely on the assumption that aligned_result >= original. To address this, I've added an assertion to verify this condition.
>> 
>> The original PR (#20808) missed cases where overflow checks already existed, so I've now went through usages of `align_up` and found the places with explicit checks. Most notably, #23168 added `align_up_or_null` to metaspace, but this function is also useful elsewhere. Given this, I relocated it to `align.hpp`, alongside the rest of the alignment functions.
>
> Casper Norrbin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   changed psoldgen check back to earlier version

Changes requested by kbarrett (Reviewer).

src/hotspot/share/runtime/globals.hpp line 1423:

> 1421:   product(size_t, MinHeapDeltaBytes, ScaleForWordSize(128*K),               \
> 1422:           "The minimum change in heap space due to GC (in bytes)")          \
> 1423:           range(0, max_uintx / 2 + 1)                                       \

Since the option type is `size_t` the range should have s/max_uintx/SIZE_MAX/.
Though there are several other similar mismatches nearby.  Perhaps leave tidying this
up to a followup change?

src/hotspot/share/utilities/align.hpp line 80:

> 78: 
> 79: template <typename T, typename A>
> 80: inline bool can_align_up(T* ptr, A alignment) {

Maybe this should be later in the file, near the other pointer variants?
Also, there's no need to parameterize the pointer type - just void* suffices.

-------------

PR Review: https://git.openjdk.org/jdk/pull/23711#pullrequestreview-2662324670
PR Review Comment: https://git.openjdk.org/jdk/pull/23711#discussion_r1982089784
PR Review Comment: https://git.openjdk.org/jdk/pull/23711#discussion_r1982096331