RFR: 8318127: align_up has potential overflow

Kim Barrett kbarrett at openjdk.org
Sat Sep 21 06:28:37 UTC 2024


On Mon, 16 Sep 2024 08:31:59 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Hi everyone,
>> 
>> The `align_up` function contained code which could potentially overflow and produce an incorrect result. This PR adds an assert to check for such.
>> 
>> Additionally, two test case that previously caused an overflow have been updated to use the highest possible values that do not trigger an overflow.
>
> src/hotspot/share/utilities/align.hpp line 76:
> 
>> 74: constexpr T align_up(T size, A alignment) {
>> 75:   T mask = checked_cast<T>(alignment_mask(alignment));
>> 76:   assert(size <= std::numeric_limits<T>::max() - mask, "overflow");
> 
> I don't really understand this assertion. `align_up((uint32_t)0fffff_ffff, 16) == 0`, because `uint32_t` is an unsigned type:
> 
> _An unsigned integer type has the same width N as the corresponding signed integer type. The range of representable values for the unsigned type is 0 to 2 N − 1 (inclusive); arithmeticfor the unsigned type is performed modulo 2**N_.
> [Note 2 : Unsigned arithmetic does not overflow. Overflow for signed arithmetic yields undefined behavior ]

The JBS issue is using "overflow" in the sense of "high bits of the mathematical result are discarded".
Fixed-width unsigned arithmetic can certainly overflow in that sense.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20808#discussion_r1769487022


More information about the hotspot-dev mailing list