languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-15 21:50:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

[vm/compiler] Fix bug in Cid range computation

Browse source 
Rationale:
Make sure open range is closed when skipping
special cids. This bug was probably undetected
for a while, since only kByteBufferCid is adjacent
to kNullCid.

https://github.com/flutter/flutter/issues/28260

Change-Id: Ic22c83aeb26c3130132dc901004a0ad818246dbc
Reviewed-on: https://dart-review.googlesource.com/c/93926
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
Aart Bik 2019-02-25 22:31:24 +00:00 committed by commit-bot@chromium.org
parent afb8747fc2
commit fa0ba2220e
7 changed files with 120 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
runtime/vm/compiler/aot/precompiler.cc
View file

@ -126,19 +126,6 @@ static void Jump(const Error& error) {
Thread::Current()->long_jump_base()->Jump(1, error);
}
TypeRangeCache::TypeRangeCache(Precompiler* precompiler,
Thread* thread,
intptr_t num_cids)
: precompiler_(precompiler),
thread_(thread),
lower_limits_(thread->zone()->Alloc<intptr_t>(num_cids)),
upper_limits_(thread->zone()->Alloc<intptr_t>(num_cids)) {
for (intptr_t i = 0; i < num_cids; i++) {
lower_limits_[i] = kNotComputed;
upper_limits_[i] = kNotComputed;
}
}
RawError* Precompiler::CompileAll() {
LongJumpScope jump;
if (setjmp(*jump.Set()) == 0) {

19
runtime/vm/compiler/aot/precompiler.h
View file

@ -29,25 +29,6 @@ class Precompiler;
class FlowGraph;
class PrecompilerEntryPointsPrinter;
class TypeRangeCache : public ValueObject {
public:
TypeRangeCache(Precompiler* precompiler, Thread* thread, intptr_t num_cids);
~TypeRangeCache();
bool InstanceOfHasClassRange(const AbstractType& type,
intptr_t* lower_limit,
intptr_t* upper_limit);
private:
static const intptr_t kNotComputed = -1;
static const intptr_t kNotContiguous = -2;
Precompiler* precompiler_;
Thread* thread_;
intptr_t* lower_limits_;
intptr_t* upper_limits_;
};
class SymbolKeyValueTrait {
public:
// Typedefs needed for the DirectChainedHashMap template.

10
runtime/vm/compiler/backend/flow_graph_compiler.cc
View file

@ -2059,7 +2059,10 @@ bool FlowGraphCompiler::GenerateSubtypeRangeCheck(Register class_id_reg,
Label* is_subtype) {
HierarchyInfo* hi = Thread::Current()->hierarchy_info();
if (hi != NULL) {
const CidRangeVector& ranges = hi->SubtypeRangesForClass(type_class);
const CidRangeVector& ranges =
hi->SubtypeRangesForClass(type_class,
/*include_abstract=*/false,
/*exclude_null=*/false);
if (ranges.length() <= kMaxNumberOfCidRangesToTest) {
GenerateCidRangesCheck(assembler(), class_id_reg, ranges, is_subtype);
return true;
@ -2161,7 +2164,10 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
const bool can_use_simple_cid_range_test =
hi->CanUseSubtypeRangeCheckFor(dst_type);
if (can_use_simple_cid_range_test) {
const CidRangeVector& ranges = hi->SubtypeRangesForClass(type_class);
const CidRangeVector& ranges =
hi->SubtypeRangesForClass(type_class,
/*include_abstract=*/false,
/*exclude_null=*/false);
if (ranges.length() <= kMaxNumberOfCidRangesToTest) {
if (is_non_smi) {
__ LoadClassId(scratch_reg, instance_reg);

92
runtime/vm/compiler/backend/il.cc
View file

@ -109,23 +109,34 @@ class SubclassFinder {
const CidRangeVector& HierarchyInfo::SubtypeRangesForClass(
const Class& klass,
bool include_abstract) {
bool include_abstract,
bool exclude_null) {
ClassTable* table = thread()->isolate()->class_table();
const intptr_t cid_count = table->NumCids();
CidRangeVector** cid_ranges =
include_abstract ? &cid_subtype_ranges_abstract_ : &cid_subtype_ranges_;
if (*cid_ranges == NULL) {
CidRangeVector** cid_ranges = nullptr;
if (include_abstract) {
ASSERT(!exclude_null);
cid_ranges = &cid_subtype_ranges_abstract_nullable_;
} else if (exclude_null) {
ASSERT(!include_abstract);
cid_ranges = &cid_subtype_ranges_nonnullable_;
} else {
ASSERT(!include_abstract);
ASSERT(!exclude_null);
cid_ranges = &cid_subtype_ranges_nullable_;
}
if (*cid_ranges == nullptr) {
*cid_ranges = new CidRangeVector[cid_count];
}
CidRangeVector& ranges = (*cid_ranges)[klass.id()];
if (ranges.length() == 0) {
if (!FLAG_precompiled_mode) {
BuildRangesForJIT(table, &ranges, klass, /*use_subtype_test=*/true,
include_abstract);
BuildRangesForJIT(table, &ranges, klass,
/*use_subtype_test=*/true,
/*include_abstract*/ false);
} else {
BuildRangesFor(table, &ranges, klass, /*use_subtype_test=*/true,
include_abstract);
BuildRangesFor(table, &ranges, klass,
/*use_subtype_test=*/true, include_abstract, exclude_null);
}
}
return ranges;
@ -142,19 +153,28 @@ const CidRangeVector& HierarchyInfo::SubclassRangesForClass(
CidRangeVector& ranges = cid_subclass_ranges_[klass.id()];
if (ranges.length() == 0) {
if (!FLAG_precompiled_mode) {
BuildRangesForJIT(table, &ranges, klass, /*use_subtype_test=*/true);
BuildRangesForJIT(table, &ranges, klass,
/*use_subtype_test=*/true,
/*include_abstract=*/false);
} else {
BuildRangesFor(table, &ranges, klass, /*use_subtype_test=*/false);
BuildRangesFor(table, &ranges, klass,
/*use_subtype_test=*/false,
/*include_abstract=*/false,
/*exclude_null=*/false);
}
}
return ranges;
}
// Build the ranges either for:
// "<obj> as <Type>", or
// "<obj> is <Type>"
void HierarchyInfo::BuildRangesFor(ClassTable* table,
CidRangeVector* ranges,
const Class& klass,
bool use_subtype_test,
bool include_abstract) {
bool include_abstract,
bool exclude_null) {
Zone* zone = thread()->zone();
ClassTable* class_table = thread()->isolate()->class_table();
@ -166,56 +186,66 @@ void HierarchyInfo::BuildRangesFor(ClassTable* table,
AbstractType& super_type = AbstractType::Handle(zone);
const intptr_t cid_count = table->NumCids();
// Iterate over all cids to find the ones to be included in the ranges.
intptr_t start = -1;
intptr_t end = -1;
for (intptr_t cid = kInstanceCid; cid < cid_count; ++cid) {
// Create local zone because deep hierarchies may allocate lots of handles
// within one iteration of this loop.
StackZone stack_zone(thread());
HANDLESCOPE(thread());
// Some cases are "don't care", i.e., they may or may not be included,
// whatever yields the least number of ranges for efficiency.
if (!table->HasValidClassAt(cid)) continue;
if (cid == kTypeArgumentsCid) continue;
if (cid == kVoidCid) continue;
if (cid == kDynamicCid) continue;
if (cid == kNullCid) continue;
cls = table->At(cid);
if (!include_abstract && cls.is_abstract()) continue;
if (cls.is_patch()) continue;
if (cls.IsTopLevel()) continue;
// We are either interested in [CidRange]es of subclasses or subtypes.
bool test_succeded = false;
if (use_subtype_test) {
bool test_succeeded = false;
if (cid == kNullCid) {
test_succeeded = !exclude_null;
} else if (use_subtype_test) {
cls_type = cls.RareType();
test_succeded = cls_type.IsSubtypeOf(dst_type, Heap::kNew);
test_succeeded = cls_type.IsSubtypeOf(dst_type, Heap::kNew);
} else {
while (!cls.IsObjectClass()) {
if (cls.raw() == klass.raw()) {
test_succeded = true;
test_succeeded = true;
break;
}
super_type = cls.super_type();
const intptr_t type_class_id = super_type.type_class_id();
cls = class_table->At(type_class_id);
}
}
if (start == -1 && test_succeded) {
start = cid;
} else if (start != -1 && !test_succeded) {
CidRange range(start, cid - 1);
if (test_succeeded) {
// On success, open a new or continue any open range.
if (start == -1) start = cid;
end = cid;
} else if (start != -1) {
// On failure, close any open range from start to end
// (the latter is the most recent succesful "do-care" cid).
ASSERT(start <= end);
CidRange range(start, end);
ranges->Add(range);
start = -1;
end = -1;
}
}
// Construct last range (either close open one, or add invalid).
if (start != -1) {
CidRange range(start, cid_count - 1);
ASSERT(start <= end);
CidRange range(start, end);
ranges->Add(range);
}
if (start == -1 && ranges->length() == 0) {
} else if (ranges->length() == 0) {
CidRange range;
ASSERT(range.IsIllegalRange());
ranges->Add(range);
@ -228,8 +258,8 @@ void HierarchyInfo::BuildRangesForJIT(ClassTable* table,
bool use_subtype_test,
bool include_abstract) {
if (dst_klass.InVMHeap()) {
BuildRangesFor(table, ranges, dst_klass, use_subtype_test,
include_abstract);
BuildRangesFor(table, ranges, dst_klass, use_subtype_test, include_abstract,
/*exclude_null=*/false);
return;
}
@ -417,10 +447,14 @@ bool HierarchyInfo::CanUseGenericSubtypeRangeCheckFor(
bool HierarchyInfo::InstanceOfHasClassRange(const AbstractType& type,
intptr_t* lower_limit,
intptr_t* upper_limit) {
ASSERT(FLAG_precompiled_mode);
if (CanUseSubtypeRangeCheckFor(type)) {
const Class& type_class =
Class::Handle(thread()->zone(), type.type_class());
const CidRangeVector& ranges = SubtypeRangesForClass(type_class);
const CidRangeVector& ranges =
SubtypeRangesForClass(type_class,
/*include_abstract=*/false,
/*exclude_null=*/true);
if (ranges.length() == 1) {
const CidRange& range = ranges[0];
if (!range.IsIllegalRange()) {

29
runtime/vm/compiler/backend/il.h
View file

@ -190,8 +190,9 @@ class HierarchyInfo : public ThreadStackResource {
public:
explicit HierarchyInfo(Thread* thread)
: ThreadStackResource(thread),
cid_subtype_ranges_(NULL),
cid_subtype_ranges_abstract_(NULL),
cid_subtype_ranges_nullable_(NULL),
cid_subtype_ranges_abstract_nullable_(NULL),
cid_subtype_ranges_nonnullable_(NULL),
cid_subclass_ranges_(NULL) {
thread->set_hierarchy_info(this);
}
@ -199,18 +200,22 @@ class HierarchyInfo : public ThreadStackResource {
~HierarchyInfo() {
thread()->set_hierarchy_info(NULL);
delete[] cid_subtype_ranges_;
cid_subtype_ranges_ = NULL;
delete[] cid_subtype_ranges_nullable_;
cid_subtype_ranges_nullable_ = NULL;
delete[] cid_subtype_ranges_abstract_;
cid_subtype_ranges_abstract_ = NULL;
delete[] cid_subtype_ranges_abstract_nullable_;
cid_subtype_ranges_abstract_nullable_ = NULL;
delete[] cid_subtype_ranges_nonnullable_;
cid_subtype_ranges_nonnullable_ = NULL;
delete[] cid_subclass_ranges_;
cid_subclass_ranges_ = NULL;
}
const CidRangeVector& SubtypeRangesForClass(const Class& klass,
bool include_abstract = false);
bool include_abstract,
bool exclude_null);
const CidRangeVector& SubclassRangesForClass(const Class& klass);
bool InstanceOfHasClassRange(const AbstractType& type,
@ -242,7 +247,8 @@ class HierarchyInfo : public ThreadStackResource {
CidRangeVector* ranges,
const Class& klass,
bool use_subtype_test,
bool include_abstract = false);
bool include_abstract,
bool exclude_null);
// In JIT mode we use hierarchy information stored in the [RawClass]s
// direct_subclasses_/direct_implementors_ arrays.
@ -250,10 +256,11 @@ class HierarchyInfo : public ThreadStackResource {
CidRangeVector* ranges,
const Class& klass,
bool use_subtype_test,
bool include_abstract = false);
bool include_abstract);
CidRangeVector* cid_subtype_ranges_;
CidRangeVector* cid_subtype_ranges_abstract_;
CidRangeVector* cid_subtype_ranges_nullable_;
CidRangeVector* cid_subtype_ranges_abstract_nullable_;
CidRangeVector* cid_subtype_ranges_nonnullable_;
CidRangeVector* cid_subclass_ranges_;
};

9
runtime/vm/type_testing_stubs.cc
View file

@ -246,7 +246,10 @@ void TypeTestingStubGenerator::BuildOptimizedTypeTestStubFastCases(
// Check the cid ranges which are a subtype of [type].
if (hi->CanUseSubtypeRangeCheckFor(type)) {
const CidRangeVector& ranges = hi->SubtypeRangesForClass(type_class);
const CidRangeVector& ranges =
hi->SubtypeRangesForClass(type_class,
/*include_abstract=*/false,
/*exclude_null=*/false);
const Type& int_type = Type::Handle(Type::IntType());
const bool smi_is_ok = int_type.IsSubtypeOf(type, Heap::kNew);
@ -410,7 +413,9 @@ void TypeTestingStubGenerator::BuildOptimizedTypeArgumentValueCheck(
} else {
const Class& type_class = Class::Handle(type_arg.type_class());
const CidRangeVector& ranges =
hi->SubtypeRangesForClass(type_class, /*include_abstract=*/true);
hi->SubtypeRangesForClass(type_class,
/*include_abstract=*/true,
/*exclude_null=*/false);
Label is_subtype;
__ SmiUntag(class_id_reg);

24
tests/language_2/vm/regress_flutter_28260_test.dart Normal file
View file

@ -0,0 +1,24 @@
// 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
// Bug cid ranges (https://github.com/flutter/flutter/issues/28260).
import 'dart:typed_data';
import "package:expect/expect.dart";
foo() {
ByteBuffer a = null;
var dataMap = Map<String, dynamic>();
dataMap['data'] = a;
return (dataMap['data'] is ByteBuffer);
}
void main() {
for (int i = 0; i < 20; i++) {
Expect.equals(false, foo());
}
}