[VM/compiler] Fix range analysis for 64-bit fixed-size integers

This CL corrects handling of overflows in range analysis to
account for wrap-around (which happens with new integer semantics).

* If there is an overflow while doing computations in range
  analysis, the resulting range is approximated as full int64 range.

* For symbolic range boundaries 'symbol + offset', offset is checked
  to stay within [kMinInt64 - kSmiMin, kMaxInt64 - kSmiMax] in order
  to guarantee that overflow doesn't occur.

Issue: https://github.com/dart-lang/sdk/issues/31920
Change-Id: I2c16adbe3597e9b718ed2f6ce7210426fcc9e6a6
Reviewed-on: https://dart-review.googlesource.com/39423
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>

runtime/platform/utils.h

@@ -31,7 +31,7 @@ class Utils {
   static inline T Abs(T x) {
     // Note: as a general rule, it is not OK to use STL in Dart VM.
     // However, std::numeric_limits<T>::min() and max() are harmless
-    // and worthwile exception from this rule.
+    // and worthwhile exception from this rule.
     ASSERT(x != std::numeric_limits<T>::min());
     if (x < 0) return -x;
     return x;
@@ -45,7 +45,7 @@ class Utils {
     if (x < 0) {
       // Note: as a general rule, it is not OK to use STL in Dart VM.
       // However, std::numeric_limits<T>::min() and max() are harmless
-      // and worthwile exception from this rule.
+      // and worthwhile exception from this rule.
       if (x == std::numeric_limits<T>::min()) {
         return std::numeric_limits<T>::max();
       }

runtime/tests/vm/dart/wrap_around_in_range_analysis_test.dart

@@ -0,0 +1,28 @@
+// Copyright (c) 2018, 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=--no-background-compilation --enable-inlining-annotations
+
+// Test for overflow (wrap-around) during computations in range analysis.
+
+import "package:expect/expect.dart";
+
+const NeverInline = 'NeverInline';
+
+@NeverInline
+int foofoo(int b) {
+  for (int i = 0x7ffffffffffffffc; i <= b; i += 2) {
+    if (i < 0) {
+      return i - 0x4000000000000000;
+    }
+  }
+  return 0;
+}
+
+main() {
+  for (var i = 0; i < 10000; i++) {
+    foofoo(0x7fffffffffffffff);
+  }
+  Expect.equals(foofoo(0x7fffffffffffffff), 4611686018427387904);
+}

runtime/tests/vm/vm.status

@@ -62,6 +62,7 @@ dart/redirection_type_shuffling_test: Skip # Depends on lazy enforcement of type
 dart/simd128float32_array_test: Skip # compilers not aware of Simd128
 dart/simd128float32_test: Skip # compilers not aware of Simd128
 dart/truncating_ints_test: Skip # dart2js doesn't know about --limit-ints-to-64-bits
+dart/wrap_around_in_range_analysis_test: SkipByDesign # The test requires int64.
 
 [ $compiler == dartk ]
 cc/DartAPI_New: Crash

runtime/vm/compiler/backend/range_analysis.cc

@@ -1785,6 +1785,7 @@ RangeBoundary RangeBoundary::FromDefinition(Definition* defn, int64_t offs) {
   if (defn->IsConstant() && defn->AsConstant()->value().IsSmi()) {
     return FromConstant(Smi::Cast(defn->AsConstant()->value()).Value() + offs);
   }
+  ASSERT(IsValidOffsetForSymbolicRangeBoundary(offs));
   return RangeBoundary(kSymbol, reinterpret_cast<intptr_t>(defn), offs);
 }
 
@@ -1846,6 +1847,10 @@ bool RangeBoundary::SymbolicAdd(const RangeBoundary& a,
 
     const int64_t offset = a.offset() + b.ConstantValue();
 
+    if (!IsValidOffsetForSymbolicRangeBoundary(offset)) {
+      return false;
+    }
+
     *result = RangeBoundary::FromDefinition(a.symbol(), offset);
     return true;
   } else if (b.IsSymbol() && a.IsConstant()) {
@@ -1864,6 +1869,10 @@ bool RangeBoundary::SymbolicSub(const RangeBoundary& a,
 
     const int64_t offset = a.offset() - b.ConstantValue();
 
+    if (!IsValidOffsetForSymbolicRangeBoundary(offset)) {
+      return false;
+    }
+
     *result = RangeBoundary::FromDefinition(a.symbol(), offset);
     return true;
   }
@@ -1959,6 +1968,10 @@ static RangeBoundary CanonicalizeBoundary(const RangeBoundary& a,
     }
   } while (changed);
 
+  if (!RangeBoundary::IsValidOffsetForSymbolicRangeBoundary(offset)) {
+    return overflow;
+  }
+
   return RangeBoundary::FromDefinition(symbol, offset);
 }
 
@@ -1975,6 +1988,11 @@ static bool CanonicalizeMaxBoundary(RangeBoundary* a) {
 
   const int64_t offset = range->max().offset() + a->offset();
 
+  if (!RangeBoundary::IsValidOffsetForSymbolicRangeBoundary(offset)) {
+    *a = RangeBoundary::PositiveInfinity();
+    return true;
+  }
+
   *a = CanonicalizeBoundary(
       RangeBoundary::FromDefinition(range->max().symbol(), offset),
       RangeBoundary::PositiveInfinity());
@@ -1995,6 +2013,11 @@ static bool CanonicalizeMinBoundary(RangeBoundary* a) {
 
   const int64_t offset = range->min().offset() + a->offset();
 
+  if (!RangeBoundary::IsValidOffsetForSymbolicRangeBoundary(offset)) {
+    *a = RangeBoundary::NegativeInfinity();
+    return true;
+  }
+
   *a = CanonicalizeBoundary(
       RangeBoundary::FromDefinition(range->min().symbol(), offset),
       RangeBoundary::NegativeInfinity());

runtime/vm/compiler/backend/range_analysis.h

@@ -63,18 +63,23 @@ class RangeBoundary : public ValueObject {
   // Construct a RangeBoundary from a definition and offset.
   static RangeBoundary FromDefinition(Definition* defn, int64_t offs = 0);
 
+  static bool IsValidOffsetForSymbolicRangeBoundary(int64_t offset) {
+    if (FLAG_limit_ints_to_64_bits) {
+      if ((offset > (kMaxInt64 - kSmiMax)) ||
+          (offset < (kMinInt64 - kSmiMin))) {
+        // Avoid creating symbolic range boundaries which can wrap around.
+        return false;
+      }
+    }
+    return true;
+  }
+
   // Construct a RangeBoundary for the constant MinSmi value.
   static RangeBoundary MinSmi() { return FromConstant(Smi::kMinValue); }
 
   // Construct a RangeBoundary for the constant MaxSmi value.
   static RangeBoundary MaxSmi() { return FromConstant(Smi::kMaxValue); }
 
-  // Construct a RangeBoundary for the constant kMin value.
-  static RangeBoundary MinConstant() { return FromConstant(kMin); }
-
-  // Construct a RangeBoundary for the constant kMax value.
-  static RangeBoundary MaxConstant() { return FromConstant(kMax); }
-
   // Construct a RangeBoundary for the constant kMin value.
   static RangeBoundary MinConstant(RangeSize size) {
     switch (size) {
@@ -288,8 +293,15 @@ class RangeBoundary : public ValueObject {
 class Range : public ZoneAllocated {
  public:
   Range() : min_(), max_() {}
+
   Range(RangeBoundary min, RangeBoundary max) : min_(min), max_(max) {
     ASSERT(min_.IsUnknown() == max_.IsUnknown());
+
+    if (FLAG_limit_ints_to_64_bits &&
+        (min_.IsInfinity() || max_.IsInfinity())) {
+      // Value can wrap around, so fall back to the full 64-bit range.
+      SetInt64Range();
+    }
   }
 
   Range(const Range& other)
@@ -326,8 +338,24 @@ class Range : public ZoneAllocated {
 
   const RangeBoundary& min() const { return min_; }
   const RangeBoundary& max() const { return max_; }
-  void set_min(const RangeBoundary& value) { min_ = value; }
-  void set_max(const RangeBoundary& value) { max_ = value; }
+
+  void set_min(const RangeBoundary& value) {
+    min_ = value;
+
+    if (FLAG_limit_ints_to_64_bits && min_.IsInfinity()) {
+      // Value can wrap around, so fall back to the full 64-bit range.
+      SetInt64Range();
+    }
+  }
+
+  void set_max(const RangeBoundary& value) {
+    max_ = value;
+
+    if (FLAG_limit_ints_to_64_bits && max_.IsInfinity()) {
+      // Value can wrap around, so fall back to the full 64-bit range.
+      SetInt64Range();
+    }
+  }
 
   static RangeBoundary ConstantMinSmi(const Range* range) {
     return ConstantMin(range, RangeBoundary::kRangeBoundarySmi);
@@ -453,6 +481,11 @@ class Range : public ZoneAllocated {
  private:
   RangeBoundary min_;
   RangeBoundary max_;
+
+  void SetInt64Range() {
+    min_ = RangeBoundary::MinConstant(RangeBoundary::kRangeBoundaryInt64);
+    max_ = RangeBoundary::MaxConstant(RangeBoundary::kRangeBoundaryInt64);
+  }
 };
 
 class RangeUtils : public AllStatic {
@@ -537,12 +570,6 @@ class RangeAnalysis : public ValueObject {
   bool ConstrainValueAfterBranch(Value* use, Definition* defn);
   void ConstrainValueAfterCheckArrayBound(Value* use, Definition* defn);
 
-  // Replace uses of the definition def that are dominated by instruction dom
-  // with uses of other definition.
-  void RenameDominatedUses(Definition* def,
-                           Instruction* dom,
-                           Definition* other);
-
   // Infer ranges for integer (smi or mint) definitions.
   void InferRanges();
 

runtime/vm/compiler/backend/range_analysis_test.cc

@@ -102,27 +102,46 @@ TEST_CASE(RangeTestsInfinity) {
   EXPECT(RangeBoundary::NegativeInfinity().OverflowedMint());
   EXPECT(RangeBoundary::PositiveInfinity().OverflowedMint());
 
+  const Range fullInt64Range = Range::Full(RangeBoundary::kRangeBoundaryInt64);
+
   Range* all = new Range(RangeBoundary::NegativeInfinity(),
                          RangeBoundary::PositiveInfinity());
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(all->Equals(&fullInt64Range));
+  }
   EXPECT(all->Overlaps(0, 0));
   EXPECT(all->Overlaps(-1, 1));
   EXPECT(!all->IsWithin(0, 100));
+
   Range* positive = new Range(RangeBoundary::FromConstant(0),
                               RangeBoundary::PositiveInfinity());
-  EXPECT(positive->IsPositive());
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(positive->Equals(&fullInt64Range));
+  } else {
+    EXPECT(positive->IsPositive());
+    EXPECT(!positive->Overlaps(-2, -1));
+  }
   EXPECT(positive->Overlaps(0, 1));
   EXPECT(positive->Overlaps(1, 100));
   EXPECT(positive->Overlaps(-1, 0));
-  EXPECT(!positive->Overlaps(-2, -1));
+
   Range* negative = new Range(RangeBoundary::NegativeInfinity(),
                               RangeBoundary::FromConstant(-1));
-  EXPECT(!negative->IsPositive());
-  EXPECT(!negative->Overlaps(0, 1));
-  EXPECT(!negative->Overlaps(1, 100));
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(positive->Equals(&fullInt64Range));
+  } else {
+    EXPECT(!negative->IsPositive());
+    EXPECT(!negative->Overlaps(0, 1));
+    EXPECT(!negative->Overlaps(1, 100));
+  }
   EXPECT(negative->Overlaps(-1, 0));
   EXPECT(negative->Overlaps(-2, -1));
+
   Range* negpos = new Range(RangeBoundary::NegativeInfinity(),
                             RangeBoundary::FromConstant(0));
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(negpos->Equals(&fullInt64Range));
+  }
   EXPECT(!negpos->IsPositive());
 
   Range* a = new Range(RangeBoundary::NegativeInfinity(),
@@ -134,45 +153,74 @@ TEST_CASE(RangeTestsInfinity) {
   Range* c = new Range(RangeBoundary::NegativeInfinity(),
                        RangeBoundary::FromConstant(32));
 
-  EXPECT(a->OnlyLessThanOrEqualTo(31));
-  EXPECT(b->OnlyLessThanOrEqualTo(31));
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(a->Equals(&fullInt64Range));
+    EXPECT(b->Equals(&fullInt64Range));
+    EXPECT(c->Equals(&fullInt64Range));
+  } else {
+    EXPECT(a->OnlyLessThanOrEqualTo(31));
+    EXPECT(b->OnlyLessThanOrEqualTo(31));
+  }
   EXPECT(!c->OnlyLessThanOrEqualTo(31));
 
   Range* unsatisfiable = new Range(RangeBoundary::PositiveInfinity(),
                                    RangeBoundary::NegativeInfinity());
-  EXPECT(unsatisfiable->IsUnsatisfiable());
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(unsatisfiable->Equals(&fullInt64Range));
+  } else {
+    EXPECT(unsatisfiable->IsUnsatisfiable());
+  }
 
   Range* unsatisfiable_right = new Range(RangeBoundary::PositiveInfinity(),
                                          RangeBoundary::FromConstant(0));
-  EXPECT(unsatisfiable_right->IsUnsatisfiable());
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(unsatisfiable_right->Equals(&fullInt64Range));
+  } else {
+    EXPECT(unsatisfiable_right->IsUnsatisfiable());
+  }
 
   Range* unsatisfiable_left = new Range(RangeBoundary::FromConstant(0),
                                         RangeBoundary::NegativeInfinity());
-  EXPECT(unsatisfiable_left->IsUnsatisfiable());
+  if (FLAG_limit_ints_to_64_bits) {
+    EXPECT(unsatisfiable_left->Equals(&fullInt64Range));
+  } else {
+    EXPECT(unsatisfiable_left->IsUnsatisfiable());
+  }
 }
 
 TEST_CASE(RangeUtils) {
+  // Use kMin/kMax instead of +/-inf with limited 64-bit integers, as
+  // any range with a +/-inf bound is converted to the full int64
+  // range due to wrap-around.
+  const RangeBoundary negativeInfinity =
+      (FLAG_limit_ints_to_64_bits
+           ? RangeBoundary::FromConstant(RangeBoundary::kMin)
+           : RangeBoundary::NegativeInfinity());
+  const RangeBoundary positiveInfinity =
+      (FLAG_limit_ints_to_64_bits
+           ? RangeBoundary::FromConstant(RangeBoundary::kMax)
+           : RangeBoundary::PositiveInfinity());
+
   // [-inf, +inf].
-  const Range& range_0 = *(new Range(RangeBoundary::NegativeInfinity(),
-                                     RangeBoundary::PositiveInfinity()));
+  const Range& range_0 = *(new Range(negativeInfinity, positiveInfinity));
   // [-inf, -1].
-  const Range& range_a = *(new Range(RangeBoundary::NegativeInfinity(),
-                                     RangeBoundary::FromConstant(-1)));
+  const Range& range_a =
+      *(new Range(negativeInfinity, RangeBoundary::FromConstant(-1)));
   // [-inf, 0].
-  const Range& range_b = *(new Range(RangeBoundary::NegativeInfinity(),
-                                     RangeBoundary::FromConstant(0)));
+  const Range& range_b =
+      *(new Range(negativeInfinity, RangeBoundary::FromConstant(0)));
   // [-inf, 1].
-  const Range& range_c = *(new Range(RangeBoundary::NegativeInfinity(),
-                                     RangeBoundary::FromConstant(1)));
+  const Range& range_c =
+      *(new Range(negativeInfinity, RangeBoundary::FromConstant(1)));
   // [-1, +inf]
-  const Range& range_d = *(new Range(RangeBoundary::FromConstant(-1),
-                                     RangeBoundary::PositiveInfinity()));
+  const Range& range_d =
+      *(new Range(RangeBoundary::FromConstant(-1), positiveInfinity));
   // [0, +inf]
-  const Range& range_e = *(new Range(RangeBoundary::FromConstant(0),
-                                     RangeBoundary::PositiveInfinity()));
+  const Range& range_e =
+      *(new Range(RangeBoundary::FromConstant(0), positiveInfinity));
   // [1, +inf].
-  const Range& range_f = *(new Range(RangeBoundary::FromConstant(1),
-                                     RangeBoundary::PositiveInfinity()));
+  const Range& range_f =
+      *(new Range(RangeBoundary::FromConstant(1), positiveInfinity));
   // [1, 2].
   const Range& range_g = *(new Range(RangeBoundary::FromConstant(1),
                                      RangeBoundary::FromConstant(2)));
@@ -237,23 +285,53 @@ TEST_CASE(RangeUtils) {
 }
 
 TEST_CASE(RangeBinaryOp) {
-  Range* range_a = new Range(RangeBoundary::FromConstant(-1),
-                             RangeBoundary::PositiveInfinity());
-  range_a->Clamp(RangeBoundary::kRangeBoundaryInt64);
-  EXPECT(range_a->min().ConstantValue() == -1);
-  EXPECT(range_a->max().ConstantValue() == RangeBoundary::kMax);
-  Range* range_b = new Range(RangeBoundary::NegativeInfinity(),
-                             RangeBoundary::FromConstant(1));
-  range_b->Clamp(RangeBoundary::kRangeBoundaryInt64);
-  EXPECT(range_b->min().ConstantValue() == RangeBoundary::kMin);
-  EXPECT(range_b->max().ConstantValue() == 1);
+  if (FLAG_limit_ints_to_64_bits) {
+    Range* range_a =
+        new Range(RangeBoundary::FromConstant(-1),
+                  RangeBoundary::FromConstant(RangeBoundary::kMax));
+    range_a->Clamp(RangeBoundary::kRangeBoundaryInt32);
+    EXPECT(range_a->min().ConstantValue() == -1);
+    EXPECT(range_a->max().ConstantValue() == kMaxInt32);
+    range_a->set_max(RangeBoundary::FromConstant(RangeBoundary::kMax));
 
-  {
-    Range result;
-    Range::BinaryOp(Token::kADD, range_a, range_b, NULL, &result);
-    ASSERT(!Range::IsUnknown(&result));
-    EXPECT(result.min().IsNegativeInfinity());
-    EXPECT(result.max().IsPositiveInfinity());
+    Range* range_b = new Range(RangeBoundary::FromConstant(RangeBoundary::kMin),
+                               RangeBoundary::FromConstant(1));
+    range_b->Clamp(RangeBoundary::kRangeBoundaryInt32);
+    EXPECT(range_b->min().ConstantValue() == kMinInt32);
+    EXPECT(range_b->max().ConstantValue() == 1);
+    range_b->set_min(RangeBoundary::FromConstant(RangeBoundary::kMin));
+
+    {
+      Range result;
+      Range::BinaryOp(Token::kADD, range_a, range_b, NULL, &result);
+      ASSERT(!Range::IsUnknown(&result));
+      EXPECT(!result.min().IsNegativeInfinity());
+      EXPECT(!result.max().IsPositiveInfinity());
+      EXPECT(result.min().Equals(
+          RangeBoundary::MinConstant(RangeBoundary::kRangeBoundaryInt64)));
+      EXPECT(result.max().Equals(
+          RangeBoundary::MaxConstant(RangeBoundary::kRangeBoundaryInt64)));
+    }
+  } else {
+    Range* range_a = new Range(RangeBoundary::FromConstant(-1),
+                               RangeBoundary::PositiveInfinity());
+    range_a->Clamp(RangeBoundary::kRangeBoundaryInt64);
+    EXPECT(range_a->min().ConstantValue() == -1);
+    EXPECT(range_a->max().ConstantValue() == RangeBoundary::kMax);
+
+    Range* range_b = new Range(RangeBoundary::NegativeInfinity(),
+                               RangeBoundary::FromConstant(1));
+    range_b->Clamp(RangeBoundary::kRangeBoundaryInt64);
+    EXPECT(range_b->min().ConstantValue() == RangeBoundary::kMin);
+    EXPECT(range_b->max().ConstantValue() == 1);
+
+    {
+      Range result;
+      Range::BinaryOp(Token::kADD, range_a, range_b, NULL, &result);
+      ASSERT(!Range::IsUnknown(&result));
+      EXPECT(result.min().IsNegativeInfinity());
+      EXPECT(result.max().IsPositiveInfinity());
+    }
   }
 
   // Test that [5, 10] + [0, 5] = [5, 15].