RFR: 8344026: Ubsan: prevent potential integer overflow in c1_LIRGenerator_<arch>.cpp file [v4]
Amit Kumar
amitkumar at openjdk.org
Fri Nov 22 05:15:16 UTC 2024
On Wed, 20 Nov 2024 23:29:25 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:
> I have looked further into the PPC64 code and figured out that `load_nonconstant()` loads all values which are not simm16 into a register: https://github.com/openjdk/jdk/blob/b9bf447209db5d7f6bb16a0310421dbe4170500c/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp#L487C11-L487C29
>
> https://github.com/openjdk/jdk/blob/b9bf447209db5d7f6bb16a0310421dbe4170500c/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp#L59
>
> only accepts simm16 values.
> So, `strength_reduce_multiply` will never get a value which overflows on PPC64. That's the reason why PPC64 is not affected by the UB bug.
> In your comment above, I guess you missed that `is_power_of_2((juint)c + 1)` returns true for c = MAX_INT. So, the optimization can be used for multiplications with MAX_INT if the UB is fixed.
>
> I also think that having a more similar solution for all platforms would be nice. For PPC64 and some other platforms, this may only be a cleanup, not a bug fix.
>
> In addition, the title is no longer up to date. You're changing more than s390 code. If you prefer to fix only UB on the affected platforms, this will also be fine with me.
Thanks @TheRealMDoerr for the suggestions. Yes I missed `INT_MAX+1` case. But my thoughts were that `INT_MAX+1` will go beyond the integer range so need to worry about that.
I have updated title, PR description and did similar changes for aarch, arm also. Please have a look again.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22144#issuecomment-2492889539
More information about the hotspot-compiler-dev
mailing list