RFR: 8318127: align_up has potential overflow [v8]

[Kim Barrett]

On Tue, 26 Nov 2024 01:07:48 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/utilities/align.hpp line 75:
>> 
>>> 73: template<typename T, typename A, ENABLE_IF(std::is_integral<T>::value)>
>>> 74: constexpr T align_up(T size, A alignment) {
>>> 75:   T mask = checked_cast<T>(alignment_mask(alignment));
>> 
>> Should we assert that mask >= 0 or trust that the result of alignment_mask() is never negative?
>
> alignment_mask requires the alignment to be a power of 2, so can't return a
> negative value.
> 
> But now I'm wondering why the size and alignment are allowed to be of
> different types in all these functions.  If alignment doesn't fit in T then it
> seems like lots of things can go wrong.  Maybe alignment_mask should take the
> size type as an additional (non-deduced) template parameter and return a value
> of that type after checking it fits?  Or something like that, as that isn't
> quite right for is_aligned.  All uses of alignment_mask do some implicit or
> explicit conversion involving the size type.
> 
> Sounds like there's some followup work to do.  I'll file a JBS issue.

https://bugs.openjdk.org/browse/JDK-8345005
Validate alignment in align_down and related functions

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

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