On Thu, 18 Jun 2026 12:22:34 GMT, Quan Anh Mai <[email protected]> wrote:

>> The scaled index computation for `elem_size > 8` in 
>> `adjust_for_flat_array()` could overflow if we used a sufficiently large 
>> array. However, we currently cannot create such a huge array because of 
>> running out memory first:
>> 
>> With `factor = 2`, we have `elem_size = 16`. To get the `MulI` to overflow, 
>> we need an array length larger than `max_jint / 2`. Thus, we require a 
>> payload larger than
>> 
>> elem_size * length = 16 * (max_jint / 2) = max_jint * 8
>> 
>> But the max flat array size is restricted by 
>> `FlatArrayKlass::max_elements()` to not exceed `max_jint * 8` bytes. So, we 
>> are always getting an OOM before actually observing an overflow.
>> 
>> Nevertheless, I think we should still add a safety check in 
>> `adjust_for_flat_array()` if this upper limit ever changes in the future. I 
>> suggest to use a `guarantee()` here to also cover product builds. 
>> Alternatively, we could also use an `assert` + compilation bailout. But 
>> given that we cannot currently observe this failure, I suggest to go with a 
>> `guarantee()`.
>> 
>> Since we do not know the exact array length, I propose to calculate the 
>> maximum possible array size and compare it to the maximum scaled index. If 
>> we ever change `FlatArrayKlass::max_elements()` to allow bigger arrays, we 
>> would catch that with the added `guarantee()`.
>> 
>> Thanks,
>> Christian
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Looks good to me.

Thanks @merykitty for your review!

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

PR Comment: https://git.openjdk.org/valhalla/pull/2566#issuecomment-4746413752

Reply via email to