[vm/interpreter] Enforce consistent interpreter/compiler unboxing view

Rationale:
Both the interpreter and compiler had some logic on when fields
could be unboxed. Rather than duplicating this logic, this CL
uses the same methods to get a more consistent view of unboxing.
Note that this add a slight overhead to the interpreter, but
only a slight. We could avoid this by duplicating the logic
to enforce consistency, but this has the danger of getting
similar issues in the future.

#37821

Change-Id: Icf0c1ad518d75c63e9da72e1f75eaf1cf903587f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115301
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>

runtime/vm/interpreter.cc

@@ -13,6 +13,7 @@
 
 #include "vm/compiler/assembler/assembler.h"
 #include "vm/compiler/assembler/disassembler_kbc.h"
+#include "vm/compiler/backend/flow_graph_compiler.h"
 #include "vm/compiler/frontend/bytecode_reader.h"
 #include "vm/compiler/jit/compiler.h"
 #include "vm/cpu.h"
@@ -389,6 +390,9 @@ Interpreter::Interpreter()
     }
   }
 #endif
+  // Make sure interpreter's unboxing view is consistent with compiler.
+  supports_unboxed_doubles_ = FlowGraphCompiler::SupportsUnboxedDoubles();
+  supports_unboxed_simd128_ = FlowGraphCompiler::SupportsUnboxedSimd128();
 }
 
 Interpreter::~Interpreter() {
@@ -2317,7 +2321,7 @@ RawObject* Interpreter::Call(RawFunction* function,
         (field->ptr()->is_nullable_ != kNullCid) &&
         Field::UnboxingCandidateBit::decode(field->ptr()->kind_bits_);
     classid_t guarded_cid = field->ptr()->guarded_cid_;
-    if (unboxing && (guarded_cid == kDoubleCid)) {
+    if (unboxing && (guarded_cid == kDoubleCid) && supports_unboxed_doubles_) {
       double raw_value = Double::RawCast(value)->ptr()->value_;
       ASSERT(*(reinterpret_cast<RawDouble**>(instance->ptr()) +
                offset_in_words) == null_value);  // Initializing store.
@@ -2329,7 +2333,8 @@ RawObject* Interpreter::Call(RawFunction* function,
       instance->StorePointer(
           reinterpret_cast<RawDouble**>(instance->ptr()) + offset_in_words, box,
           thread);
-    } else if (unboxing && (guarded_cid == kFloat32x4Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat32x4Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float32x4::RawCast(value)->ptr()->value_);
       ASSERT(*(reinterpret_cast<RawFloat32x4**>(instance->ptr()) +
@@ -2342,7 +2347,8 @@ RawObject* Interpreter::Call(RawFunction* function,
       instance->StorePointer(
           reinterpret_cast<RawFloat32x4**>(instance->ptr()) + offset_in_words,
           box, thread);
-    } else if (unboxing && (guarded_cid == kFloat64x2Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat64x2Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float64x2::RawCast(value)->ptr()->value_);
       ASSERT(*(reinterpret_cast<RawFloat64x2**>(instance->ptr()) +
@@ -3078,20 +3084,23 @@ RawObject* Interpreter::Call(RawFunction* function,
         (field->ptr()->is_nullable_ != kNullCid) &&
         Field::UnboxingCandidateBit::decode(field->ptr()->kind_bits_);
     classid_t guarded_cid = field->ptr()->guarded_cid_;
-    if (unboxing && (guarded_cid == kDoubleCid)) {
+    if (unboxing && (guarded_cid == kDoubleCid) && supports_unboxed_doubles_) {
+      ASSERT(FlowGraphCompiler::SupportsUnboxedDoubles());
       double raw_value = Double::RawCast(value)->ptr()->value_;
       // AllocateDouble places result at SP[0]
       if (!AllocateDouble(thread, raw_value, pc, FP, SP)) {
         HANDLE_EXCEPTION;
       }
-    } else if (unboxing && (guarded_cid == kFloat32x4Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat32x4Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float32x4::RawCast(value)->ptr()->value_);
       // AllocateFloat32x4 places result at SP[0]
       if (!AllocateFloat32x4(thread, raw_value, pc, FP, SP)) {
         HANDLE_EXCEPTION;
       }
-    } else if (unboxing && (guarded_cid == kFloat64x2Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat64x2Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float64x2::RawCast(value)->ptr()->value_);
       // AllocateFloat64x2 places result at SP[0]
@@ -3193,20 +3202,22 @@ RawObject* Interpreter::Call(RawFunction* function,
         (field->ptr()->is_nullable_ != kNullCid) &&
         Field::UnboxingCandidateBit::decode(field->ptr()->kind_bits_);
     classid_t guarded_cid = field->ptr()->guarded_cid_;
-    if (unboxing && (guarded_cid == kDoubleCid)) {
+    if (unboxing && (guarded_cid == kDoubleCid) && supports_unboxed_doubles_) {
       double raw_value = Double::RawCast(value)->ptr()->value_;
       RawDouble* box =
           *(reinterpret_cast<RawDouble**>(instance->ptr()) + offset_in_words);
       ASSERT(box != null_value);  // Non-initializing store.
       box->ptr()->value_ = raw_value;
-    } else if (unboxing && (guarded_cid == kFloat32x4Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat32x4Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float32x4::RawCast(value)->ptr()->value_);
       RawFloat32x4* box = *(reinterpret_cast<RawFloat32x4**>(instance->ptr()) +
                             offset_in_words);
       ASSERT(box != null_value);  // Non-initializing store.
       raw_value.writeTo(box->ptr()->value_);
-    } else if (unboxing && (guarded_cid == kFloat64x2Cid)) {
+    } else if (unboxing && (guarded_cid == kFloat64x2Cid) &&
+               supports_unboxed_simd128_) {
       simd128_value_t raw_value;
       raw_value.readFrom(Float64x2::RawCast(value)->ptr()->value_);
       RawFloat64x2* box = *(reinterpret_cast<RawFloat64x2**>(instance->ptr()) +

runtime/vm/interpreter.h

@@ -275,7 +275,11 @@ class Interpreter {
   bool is_debugging_;
 #endif  // !PRODUCT
 
+  bool supports_unboxed_doubles_;
+  bool supports_unboxed_simd128_;
+
   friend class InterpreterSetjmpBuffer;
+
   DISALLOW_COPY_AND_ASSIGN(Interpreter);
 };
 

tests/language_2/vm/regression_37821_test.dart

@@ -0,0 +1,41 @@
+// 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
+
+import "package:expect/expect.dart";
+
+import 'dart:typed_data';
+
+// Found by DartFuzzing: inconsistent view of unboxing
+// https://github.com/dart-lang/sdk/issues/37821
+
+double var5 = 0.5521203015696288;
+
+class X0 {
+  double fld0_1 = 0.44794902547497595;
+  void run() {
+    for (int loc1 = 0; loc1 < 11; loc1++) {
+      var5 = fld0_1;
+    }
+    fld0_1 -= 20;
+  }
+}
+
+class X1 extends X0 {
+  void run() {
+    super.run();
+  }
+}
+
+@pragma("vm:never-inline")
+void checkMe(double value) {
+  Expect.approxEquals(0.44794902547497595, value);
+}
+
+main() {
+  Expect.approxEquals(0.5521203015696288, var5);
+  new X1().run();
+  checkMe(var5);
+}