Skip to content

Commit

Permalink
[VM runtime] Fix type canonicalization (fixes #32425).
Browse files Browse the repository at this point in the history
A reused type argument vector that is longer than necessary needs to be
shortened to the correct length upon type canonicalization.
The runtime call comparing two instance runtime types also needs to consider
reused vectors.
Add regression test.

Change-Id: Ib3b9620409b9cff313f270c4f3fb7051fecbb604
Reviewed-on: https://dart-review.googlesource.com/45340
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
  • Loading branch information
crelier authored and commit-bot@chromium.org committed Mar 8, 2018
1 parent 0fdbc33 commit 383517d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
10 changes: 9 additions & 1 deletion runtime/lib/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,19 @@ DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 2) {
return Bool::True().raw();
}

if (left.GetTypeArguments() == right.GetTypeArguments()) {
return Bool::True().raw();
}
const TypeArguments& left_type_arguments =
TypeArguments::Handle(left.GetTypeArguments());
const TypeArguments& right_type_arguments =
TypeArguments::Handle(right.GetTypeArguments());
return Bool::Get(left_type_arguments.Equals(right_type_arguments)).raw();
const intptr_t num_type_args = cls.NumTypeArguments();
const intptr_t num_type_params = cls.NumTypeParameters();
return Bool::Get(left_type_arguments.IsSubvectorEquivalent(
right_type_arguments, num_type_args - num_type_params,
num_type_params))
.raw();
}

DEFINE_NATIVE_ENTRY(Object_instanceOf, 4) {
Expand Down
21 changes: 18 additions & 3 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17613,9 +17613,24 @@ RawAbstractType* Type::Canonicalize(TrailPtr trail) const {
// Canonicalize the type arguments.
TypeArguments& type_args = TypeArguments::Handle(zone, arguments());
// In case the type is first canonicalized at runtime, its type argument
// vector may be longer than necessary. This is not an issue.
ASSERT(type_args.IsNull() ||
(type_args.Length() >= cls.NumTypeArguments()));
// vector may be longer than necessary. If so, reallocate a vector of the
// exact size to prevent multiple "canonical" types.
if (!type_args.IsNull()) {
const intptr_t num_type_args = cls.NumTypeArguments();
ASSERT(type_args.Length() >= num_type_args);
if (type_args.Length() > num_type_args) {
TypeArguments& new_type_args =
TypeArguments::Handle(zone, TypeArguments::New(num_type_args));
AbstractType& type_arg = AbstractType::Handle(zone);
for (intptr_t i = 0; i < num_type_args; i++) {
type_arg = type_args.TypeAt(i);
new_type_args.SetTypeAt(i, type_arg);
}
type_args = new_type_args.raw();
set_arguments(type_args);
SetHash(0); // Flush cached hash value.
}
}
type_args = type_args.Canonicalize(trail);
if (IsCanonical()) {
// Canonicalizing type_args canonicalized this type as a side effect.
Expand Down
28 changes: 28 additions & 0 deletions tests/language_2/regress_32425_test.dart
Original file line number Diff line number Diff line change
@@ -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.

import 'package:expect/expect.dart';

class A<T> {}

final map = {};

class B<T> {
void foo() {
Expect.equals(map[new A<T>().runtimeType], 42);
}
}

class C<T, U> {
void build() {
new B<T>().foo();
new B<U>().foo();
Expect.equals(new B<T>().runtimeType, new B<U>().runtimeType);
}
}

void main() {
map[new A<String>().runtimeType] = 42;
new C<String, String>().build();
}

0 comments on commit 383517d

Please sign in to comment.