[dart/vm] Fix division-by-zero bug in range analysis

aartbik · commit-bot@chromium.org · commit 1ac0a36fcbe2 · 2019-07-25T00:00:39.000Z
Rationale: The attached test, a simplified version from a case found by fuzzing, yielded the range [4,4] for the left hand side of the division but [25,0] (sic!) for the right hand side. This reverse range caused the logic in the TRUNC evaluation to pass the >= 1 test but introduce the division by zero in the compiler code. The true culprit, however, was the reason for [25,0] which was due to a difference in C++ and Dart shift right semantics. This CL fixes the bug and adds some sanity assert to find more of such cases. #37622 Change-Id: I60e07428435d99589e1177c113ef5e24e1611d0e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110420 Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Aart Bik <ajcbik@google.com>
diff --git a/runtime/vm/compiler/backend/range_analysis.cc b/runtime/vm/compiler/backend/range_analysis.cc
@@ -2442,6 +2442,10 @@ void Range::BinaryOp(const Token::Kind op,
 
   ASSERT(!min.IsUnknown() && !max.IsUnknown());
 
+  // Sanity: avoid [l, u] with constants l > u.
+  ASSERT(!min.IsConstant() || !max.IsConstant() ||
+         min.ConstantValue() <= max.ConstantValue());
+
   *result = Range(min, max);
 }
 
diff --git a/runtime/vm/compiler/backend/range_analysis.h b/runtime/vm/compiler/backend/range_analysis.h
@@ -250,8 +250,10 @@ class RangeBoundary : public ValueObject {
                            int64_t shift_count) {
     ASSERT(value_boundary.IsConstant());
     ASSERT(shift_count >= 0);
-    int64_t value = static_cast<int64_t>(value_boundary.ConstantValue());
-    int64_t result = value >> shift_count;
+    const int64_t value = static_cast<int64_t>(value_boundary.ConstantValue());
+    const int64_t result = (shift_count <= 63)
+                               ? (value >> shift_count)
+                               : (value >= 0 ? 0 : -1);  // Dart semantics
     return RangeBoundary(result);
   }
 
diff --git a/tests/language_2/vm/regression_37622_test.dart b/tests/language_2/vm/regression_37622_test.dart
@@ -0,0 +1,39 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// VMOptions=--deterministic --enable-inlining-annotations
+
+// Issue #37622 found with fuzzing: internal compiler crash (division-by-zero).
+
+import 'dart:typed_data';
+
+import "package:expect/expect.dart";
+
+const String NeverInline = 'NeverInline';
+
+@NeverInline
+int foo() {
+  int x = 0;
+  {
+    int loc0 = 67;
+    while (--loc0 > 0) {
+      --loc0;
+      x += (4 ~/ (Int32x4.wxwx >> loc0));
+      --loc0;
+      --loc0;
+    }
+  }
+}
+
+main() {
+  int x = 0;
+  bool d = false;
+  try {
+    x = foo();
+  } on IntegerDivisionByZeroException catch (e) {
+    d = true;
+  }
+  Expect.equals(x, 0);
+  Expect.isTrue(d);
+}

Original file line number	Diff line number	Diff line change
`@@ -2442,6 +2442,10 @@ void Range::BinaryOp(const Token::Kind op,`
`2442`	`2442`
`2443`	`2443`	`ASSERT(!min.IsUnknown() && !max.IsUnknown());`
`2444`	`2444`
	`2445`	`+ // Sanity: avoid [l, u] with constants l > u.`
	`2446`	`+ ASSERT(!min.IsConstant() \|\| !max.IsConstant() \|\|`
	`2447`	`+ min.ConstantValue() <= max.ConstantValue());`
	`2448`	`+`
`2445`	`2449`	`*result = Range(min, max);`
`2446`	`2450`	`}`
`2447`	`2451`