RFR: 8323688: C2: Fix UB of jlong overflow in PhaseIdealLoop::is_counted_loop() [v2]

Vladimir Kozlov kvn at openjdk.org
Tue Sep 3 16:02:19 UTC 2024


On Tue, 3 Sep 2024 09:30:32 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> The computation of `final_correction` in `PhaseIdealLoop::is_counted_loop()` could overflow which is UB:
>> 
>> https://github.com/openjdk/jdk/blob/dc4fd896289db1d2f6f7bbf5795fec533448a48c/src/hotspot/share/opto/loopnode.cpp#L1958-L1967
>> 
>> `canonicalized_correction` equals `max_int - 1` if stride is `max_int`. `limit_correction` is at most `max_int - 1` in that case. Adding both together will overflow. I don't think that any compiler would wrongly optimize this and we have not observed any issues with that. But we should still fix this UB.
>> 
>> The fix I propose is to simply bail out with very large positive and negative strides such that we avoid an over- or underflow with the existing logic (see added comments for how the upper bound for the stride is determined). These large strides should be very uncommon in practice and even if we encounter these, the loop would only run for a few iterations. So, a bailout seems fine. This bailout has the additional benefit that we avoid other possibly unknown issues or issues in the future with counted loops having large edge-case strides like `min_int`.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/hotspot/share/opto/loopnode.cpp
>   
>   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

Good.

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

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20828#pullrequestreview-2277872619


More information about the hotspot-compiler-dev mailing list