Integrated: 8323688: C2: Fix UB of jlong overflow in PhaseIdealLoop::is_counted_loop()

Christian Hagedorn chagedorn at openjdk.org
Tue Sep 24 06:49:40 UTC 2024


On Tue, 3 Sep 2024 07:35:10 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

This pull request has now been integrated.

Changeset: 1dd60b62
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/1dd60b62e384090b13a08d2afa62e49ef52bc46c
Stats:     21 lines in 1 file changed: 20 ins; 0 del; 1 mod

8323688: C2: Fix UB of jlong overflow in PhaseIdealLoop::is_counted_loop()

Reviewed-by: thartmann, kvn

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

PR: https://git.openjdk.org/jdk/pull/20828


More information about the hotspot-compiler-dev mailing list