RFR: 8346916: [REDO] align_up has potential overflow

Dean Long dlong at openjdk.org
Fri Feb 21 00:17:52 UTC 2025


On Thu, 20 Feb 2025 10:48:26 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.
> 
> Additionally, I've created `align_up_or_min`, which behaves similarly to the original align_up but handles overflows predictably across all integer types. This new function is used in the locations where overflow checks already exist, providing a safer alternative.

Can you explain what was wrong with the original fix?  The BACKOUT only mentions that tests failed, but doesn't say why.
Also, I fail to see why align_up_or_min is an improvement.  It seems to silently mask errors, and the callers are not checking the result.  Having a size the overflows size_t seems like an error to me.

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

PR Comment: https://git.openjdk.org/jdk/pull/23711#issuecomment-2673005108


More information about the hotspot-dev mailing list