RFR: 8344026: [s390x] ubsan failure: signed integer overflow in c1_LIRGenerator_s390.cpp [v4]

[Martin Doerr]

On Tue, 19 Nov 2024 15:48:49 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> This PR adds `c > 0 && c < max_jint` check in c1_LIRGenerator_s390.cpp. Please look JBS for more info.
>
> Amit Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove dummy code

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.

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

PR Comment: https://git.openjdk.org/jdk/pull/22144#issuecomment-2489747680