[vm/compiler] Fix truncating logic

aartbik · commit-bot@chromium.org · commit d9a9346c3fe8 · 2019-09-05T21:13:43.000Z
Rationale: The representation mask should not be used to do the truncation, which is more subtle around the sign bit. With regression test. #38147 Change-Id: I9745f6d73f4761eb6210e0b60b2b0504994f5cbf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115607 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Aart Bik <ajcbik@google.com>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
@@ -1982,6 +1982,26 @@ static int64_t RepresentationMask(Representation r) {
                               (64 - RepresentationBits(r)));
 }
 
+static int64_t TruncateTo(int64_t v, Representation r) {
+  switch (r) {
+    case kTagged: {
+      // Smi occupies word minus kSmiTagShift bits.
+      const intptr_t kTruncateBits =
+          (kBitsPerInt64 - kBitsPerWord) + kSmiTagShift;
+      return Utils::ShiftLeftWithTruncation(v, kTruncateBits) >> kTruncateBits;
+    }
+    case kUnboxedInt32:
+      return Utils::ShiftLeftWithTruncation(v, kBitsPerInt32) >> kBitsPerInt32;
+    case kUnboxedUint32:
+      return v & kMaxUint32;
+    case kUnboxedInt64:
+      return v;
+    default:
+      UNREACHABLE();
+      return 0;
+  }
+}
+
 static bool ToIntegerConstant(Value* value, int64_t* result) {
   if (!value->BindsToConstant()) {
     UnboxInstr* unbox = value->definition()->AsUnbox();
@@ -1993,7 +2013,7 @@ static bool ToIntegerConstant(Value* value, int64_t* result) {
 
         case kUnboxedUint32:
           if (ToIntegerConstant(unbox->value(), result)) {
-            *result &= RepresentationMask(kUnboxedUint32);
+            *result = TruncateTo(*result, kUnboxedUint32);
             return true;
           }
           break;
@@ -2341,8 +2361,8 @@ RawInteger* BinaryIntegerOpInstr::Evaluate(const Integer& left,
 
   if (!result.IsNull()) {
     if (is_truncating()) {
-      int64_t truncated = result.AsTruncatedInt64Value();
-      truncated &= RepresentationMask(representation());
+      const int64_t truncated =
+          TruncateTo(result.AsTruncatedInt64Value(), representation());
       result = Integer::New(truncated, Heap::kOld);
       ASSERT(IsRepresentable(result, representation()));
     } else if (!IsRepresentable(result, representation())) {
@@ -2475,7 +2495,6 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
     return this;
   }
 
-  const int64_t range_mask = RepresentationMask(representation());
   if (is_truncating()) {
     switch (op_kind()) {
       case Token::kMUL:
@@ -2484,7 +2503,7 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
       case Token::kBIT_AND:
       case Token::kBIT_OR:
       case Token::kBIT_XOR:
-        rhs = (rhs & range_mask);
+        rhs = TruncateTo(rhs, representation());
         break;
       default:
         break;
@@ -2527,21 +2546,21 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
     case Token::kBIT_AND:
       if (rhs == 0) {
         return right()->definition();
-      } else if (rhs == range_mask) {
+      } else if (rhs == RepresentationMask(representation())) {
         return left()->definition();
       }
       break;
     case Token::kBIT_OR:
       if (rhs == 0) {
         return left()->definition();
-      } else if (rhs == range_mask) {
+      } else if (rhs == RepresentationMask(representation())) {
         return right()->definition();
       }
       break;
     case Token::kBIT_XOR:
       if (rhs == 0) {
         return left()->definition();
-      } else if (rhs == range_mask) {
+      } else if (rhs == RepresentationMask(representation())) {
         UnaryIntegerOpInstr* bit_not = UnaryIntegerOpInstr::Make(
             representation(), Token::kBIT_NOT, left()->CopyWithType(),
             GetDeoptId(), range());
diff --git a/tests/language_2/vm/regression_38147_test.dart b/tests/language_2/vm/regression_38147_test.dart
@@ -0,0 +1,27 @@
+// 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 --optimization_counter_threshold=10
+
+import "package:expect/expect.dart";
+
+import 'dart:typed_data';
+
+// Found by DartFuzzing: truncating shift
+// https://github.com/dart-lang/sdk/issues/38147
+
+int bar(int x, int y) {
+  return (x << y) & 0xffff;
+}
+
+@pragma("vm:never-inline")
+int foo() {
+  return bar(-61, 12);
+}
+
+main() {
+  for (int i = 0; i < 20; i++) {
+    Expect.equals(12288, foo());
+  }
+}
