8344026: Ubsan: prevent potential integer overflow in c1_LIRGenerator…

…_<arch>.cpp file Reviewed-by: aph, epeter, mdoerr
azvegint · Dec 12, 2024 · 77e4932 · 77e4932
1 parent 3f2556b
commit 77e4932
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 21 deletions.
diff --git a/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp
@@ -277,18 +277,20 @@ 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) {
-
-  if (is_power_of_2(c - 1)) {
-    __ shift_left(left, exact_log2(c - 1), tmp);
+  juint u_value = (juint)c;
+  if (is_power_of_2(u_value - 1)) {
+    __ shift_left(left, exact_log2(u_value - 1), tmp);
     __ add(tmp, left, result);
     return true;
-  } else if (is_power_of_2(c + 1)) {
-    __ shift_left(left, exact_log2(c + 1), tmp);
+  } else if (is_power_of_2(u_value + 1)) {
+    __ shift_left(left, exact_log2(u_value + 1), tmp);
     __ sub(tmp, left, result);
     return true;
-  } else {
-    return false;
+  } else if (c == -1) {
+    __ negate(left, result);
+    return true;
   }
+  return false;
 }
 
 void LIRGenerator::store_stack_parameter (LIR_Opr item, ByteSize offset_from_sp) {

diff --git a/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp b/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
@@ -328,16 +328,20 @@ 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)) {
-    LIR_Address::Scale scale = (LIR_Address::Scale) log2i_exact(c + 1);
+  juint u_value = (juint)c;
+  if (is_power_of_2(u_value + 1)) {
+    LIR_Address::Scale scale = (LIR_Address::Scale) log2i_exact(u_value + 1);
     LIR_Address* addr = new LIR_Address(left, left, scale, 0, T_INT);
     __ sub(LIR_OprFact::address(addr), left, result); // rsb with shifted register
     return true;
-  } else if (is_power_of_2(c - 1)) {
-    LIR_Address::Scale scale = (LIR_Address::Scale) log2i_exact(c - 1);
+  } else if (is_power_of_2(u_value - 1)) {
+    LIR_Address::Scale scale = (LIR_Address::Scale) log2i_exact(u_value - 1);
     LIR_Address* addr = new LIR_Address(left, left, scale, 0, T_INT);
     __ add(left, LIR_OprFact::address(addr), result); // add with shifted register
     return true;
+  } else if (c == -1) {
+    __ negate(left, result);
+    return true;
   }
   return false;
 }

diff --git a/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp b/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp
@@ -296,14 +296,20 @@ 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.
+  juint u_value = (juint)c;
+  if (is_power_of_2(u_value + 1)) {
+    __ shift_left(left, log2i_exact(u_value + 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(u_value - 1)) {
+    __ shift_left(left, log2i_exact(u_value - 1), result);
     __ add(result, left, result);
     return true;
+  } else if (c == -1) {
+    __ negate(left, result);
+    return true;
   }
   return false;
 }

diff --git a/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp b/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp
@@ -227,19 +227,25 @@ 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) {
+  juint u_value = (juint)c;
   if (tmp->is_valid()) {
-    if (is_power_of_2(c + 1)) {
+    if (is_power_of_2(u_value + 1)) {
       __ move(left, tmp);
-      __ shift_left(left, log2i_exact(c + 1), left);
+      __ shift_left(left, log2i_exact(u_value + 1), left);
       __ sub(left, tmp, result);
       return true;
-    } else if (is_power_of_2(c - 1)) {
+    } else if (is_power_of_2(u_value - 1)) {
       __ move(left, tmp);
-      __ shift_left(left, log2i_exact(c - 1), left);
+      __ shift_left(left, log2i_exact(u_value - 1), left);
       __ add(left, tmp, result);
       return true;
     }
   }
+
+  if (c == -1) {
+    __ negate(left, result);
+    return true;
+  }
   return false;
 }
 
@@ -496,8 +502,8 @@ void LIRGenerator::do_ArithmeticOp_Int(ArithmeticOp* x) {
     if (x->op() == Bytecodes::_imul) {
       bool use_tmp = false;
       if (right_arg->is_constant()) {
-        int iconst = right_arg->get_jint_constant();
-        if (is_power_of_2(iconst - 1) || is_power_of_2(iconst + 1)) {
+        juint u_const = (juint)right_arg->get_jint_constant();
+        if (is_power_of_2(u_const - 1) || is_power_of_2(u_const + 1)) {
           use_tmp = true;
         }
       }

diff --git a/test/hotspot/jtreg/compiler/c1/StrengthReduceCheck.java b/test/hotspot/jtreg/compiler/c1/StrengthReduceCheck.java
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2024 IBM Corporation. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @author Amit Kumar
+ * @bug 8344026
+ * @library /test/lib
+ * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -Xcomp -Xbatch -XX:TieredStopAtLevel=1 compiler.c1.StrengthReduceCheck
+ */
+
+package compiler.c1;
+
+import jdk.test.lib.Asserts;
+
+public class StrengthReduceCheck {
+
+    static int test1(int x) {
+        // Multiply by 2 ^ 30 - 1
+        x = x * 1073741823;
+        return x;
+    }
+
+    static int test2(int x) {
+        // Multiply by 2 ^ 30 + 1
+        x = x * 1073741825;
+        return x;
+    }
+
+    static int test3(int x) {
+        // Multiply by INT_MIN
+        x = x * -2147483648;
+        return x;
+    }
+
+    static int test4(int x) {
+        x = x * -1;
+        return x;
+    }
+
+    public static void main(String[] args) {
+        for (int i =0; i < 1000; ++i) {
+            Asserts.assertEQ(test1(26071999), -1099813823);
+            Asserts.assertEQ(test2(26071999), -1047669825);
+            Asserts.assertEQ(test3(26071999), -2147483648);
+            Asserts.assertEQ(test4(26071999), -26071999);
+        }
+    }
+}
+