[turboshaft] Prevent signed division overflow in loop unrolling

DadaIsCrazy · V8 LUCI CQ · commit 940de8213dad · 2024-08-05T09:52:09.000Z
Signed division can overflow in a single case: min_int/-1. This is undefined behavior, and could lead in particular to floating point exceptions. Bug: chromium:42202729 Change-Id: Ieb702a24d204bc81fc912f1085beb28d22e7b1b5 Fixed: chromium:356183775 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5756762 Auto-Submit: Darius Mercadier <dmercadier@chromium.org> Reviewed-by: Matthias Liedtke <mliedtke@chromium.org> Commit-Queue: Darius Mercadier <dmercadier@chromium.org> Cr-Commit-Position: refs/heads/main@{#95470}
diff --git a/src/compiler/turboshaft/loop-unrolling-reducer.cc b/src/compiler/turboshaft/loop-unrolling-reducer.cc
@@ -362,10 +362,20 @@ bool SubWillOverflow(Int lhs, Int rhs) {
   }
 }
 
+template <class Int>
+bool DivWillOverflow(Int dividend, Int divisor) {
+  if constexpr (std::is_unsigned_v<Int>) {
+    return false;
+  } else {
+    return dividend == std::numeric_limits<Int>::min() && divisor == -1;
+  }
+}
+
 }  // namespace
 
-// Returns true if the loop `for (i = init, i cmp_op max; i = i binop_cst
-// binop_op)` has fewer than `max_iter_` iterations.
+// Returns true if the loop
+// `for (i = init, i cmp_op max; i = i binop_op binop_cst)` has fewer than
+// `max_iter_` iterations.
 template <class Int>
 IterationCount StaticCanonicalForLoopMatcher::CountIterationsImpl(
     Int init, Int max, CmpOp cmp_op, Int binop_cst, BinOp binop_op,
@@ -419,6 +429,7 @@ IterationCount StaticCanonicalForLoopMatcher::CountIterationsImpl(
         // eventually stop.
         return {};
       }
+      DCHECK(!DivWillOverflow(max - init, binop_cst));
       Int quotient = (max - init) / binop_cst;
       DCHECK_GE(quotient, 0);
       return IterationCount::Approx(quotient);
@@ -434,6 +445,7 @@ IterationCount StaticCanonicalForLoopMatcher::CountIterationsImpl(
         // eventually stop.
         return {};
       }
+      if (DivWillOverflow(max - init, binop_cst)) return {};
       Int quotient = (max - init) / binop_cst;
       DCHECK_GE(quotient, 0);
       return IterationCount::Approx(quotient);
@@ -469,8 +481,9 @@ IterationCount StaticCanonicalForLoopMatcher::CountIterationsImpl(
   return {};
 }
 
-// Returns true if the loop `for (i = init, i cmp_op max; i = i binop_cst
-// binop_op)` has fewer than `max_iter_` iterations.
+// Returns true if the loop
+// `for (i = initial_input, i cmp_op cmp_cst; i = i binop_op binop_cst)` has
+// fewer than `max_iter_` iterations.
 IterationCount StaticCanonicalForLoopMatcher::CountIterations(
     uint64_t cmp_cst, CmpOp cmp_op, uint64_t initial_input, uint64_t binop_cst,
     BinOp binop_op, WordRepresentation binop_rep, bool loop_if_cond_is) const {
diff --git a/test/mjsunit/compiler/regress-356183775.js b/test/mjsunit/compiler/regress-356183775.js
@@ -0,0 +1,11 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+for (let i = 0; i < 50; i++) {
+  for (let j = 2; -2147483647 < j; j--) {
+    if (j == -30000) {
+      break;
+    }
+  }
+}
