[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.

https://github.com/dart-lang/sdk/issues/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>
This commit is contained in:
Aart Bik 2019-09-04 00:52:27 +00:00 committed by commit-bot@chromium.org
parent 54fdd559d8
commit 622747502e
3 changed files with 65 additions and 9 deletions

View file

@ -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 @@ SwitchDispatch:
(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 @@ SwitchDispatch:
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 @@ SwitchDispatch:
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 @@ SwitchDispatch:
(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 @@ SwitchDispatch:
(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()) +

View file

@ -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);
};

View file

@ -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);
}