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

[Martin Doerr]

On Fri, 15 Nov 2024 10:04:51 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.

I'm thinking about improving it to optimize more cases:

diff --git a/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp b/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp
index 7973e9d0545..aa948facba1 100644
--- a/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp
+++ b/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp
@@ -296,14 +296,19 @@ void LIRGenerator::cmp_reg_mem(LIR_Condition condition, LIR_Opr reg, LIR_Opr bas
 
 bool LIRGenerator::strength_reduce_multiply(LIR_Opr left, jint c, LIR_Opr result, LIR_Opr tmp) {
   assert(left != result, "should be different registers");
-  if (is_power_of_2(c + 1)) {
-    __ shift_left(left, log2i_exact(c + 1), result);
+  // Using unsigned arithmetics to avoid undefined behavior due to integer overflow.
+  // The involved operations are not sensitive to signedness.
+  if (is_power_of_2((juint)c + 1)) {
+    __ shift_left(left, log2i_exact((juint)c + 1), result);
     __ sub(result, left, result);
     return true;
-  } else if (is_power_of_2(c - 1)) {
-    __ shift_left(left, log2i_exact(c - 1), result);
+  } else if (is_power_of_2((juint)c - 1)) {
+    __ shift_left(left, log2i_exact((juint)c - 1), result);
     __ add(result, left, result);
     return true;
+  } else if (c == -1) {
+    __ negate(left, result);
+    return true;
   }
   return false;
 }

The same should work for s390, too. Opinions?

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

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