[vm] Fix finalization of classes when instances are allocated
Classes should be allocate-finalized before instance can be allocated
in order to properly update class hierarchy information and invalidate
code which was speculatively optimized.
TEST=runtime/tests/vm/dart/regress_44342_test.dart
Fixes https://github.com/dart-lang/sdk/issues/44342
Change-Id: I52a415f4cf25010eb23be339266a91ae6b7d7261
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175106
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/lib/ffi.cc b/runtime/lib/ffi.cc
index bb85a96..2144be8 100644
--- a/runtime/lib/ffi.cc
+++ b/runtime/lib/ffi.cc
@@ -224,7 +224,7 @@
const auto& typed_data_class =
Class::Handle(zone, isolate->class_table()->At(cid));
const auto& error =
- Error::Handle(zone, typed_data_class.EnsureIsFinalized(thread));
+ Error::Handle(zone, typed_data_class.EnsureIsAllocateFinalized(thread));
if (!error.IsNull()) {
Exceptions::PropagateError(error);
}
diff --git a/runtime/tests/vm/dart/regress_44342_test.dart b/runtime/tests/vm/dart/regress_44342_test.dart
new file mode 100644
index 0000000..98514fc
--- /dev/null
+++ b/runtime/tests/vm/dart/regress_44342_test.dart
@@ -0,0 +1,40 @@
+// Copyright (c) 2020, 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=100
+
+// Verifies that class of a constant participates in CHA decisions.
+// Regression test for https://github.com/dart-lang/sdk/issues/44342.
+
+import 'package:expect/expect.dart';
+
+class A {
+ const A();
+
+ int foo() => 3;
+
+ @pragma("vm:never-inline")
+ int bar() => foo();
+}
+
+class B extends A {}
+
+class C extends A {
+ const C();
+
+ int foo() => 4;
+}
+
+A x = B();
+A y = const C();
+
+main() {
+ for (int i = 0; i < 200; ++i) {
+ // Optimize A.bar() with inlined A.foo().
+ int result = x.bar();
+ Expect.equals(3, result);
+ }
+ int result = y.bar();
+ Expect.equals(4, result);
+}
diff --git a/runtime/tests/vm/dart_2/regress_44342_test.dart b/runtime/tests/vm/dart_2/regress_44342_test.dart
new file mode 100644
index 0000000..98514fc
--- /dev/null
+++ b/runtime/tests/vm/dart_2/regress_44342_test.dart
@@ -0,0 +1,40 @@
+// Copyright (c) 2020, 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=100
+
+// Verifies that class of a constant participates in CHA decisions.
+// Regression test for https://github.com/dart-lang/sdk/issues/44342.
+
+import 'package:expect/expect.dart';
+
+class A {
+ const A();
+
+ int foo() => 3;
+
+ @pragma("vm:never-inline")
+ int bar() => foo();
+}
+
+class B extends A {}
+
+class C extends A {
+ const C();
+
+ int foo() => 4;
+}
+
+A x = B();
+A y = const C();
+
+main() {
+ for (int i = 0; i < 200; ++i) {
+ // Optimize A.bar() with inlined A.foo().
+ int result = x.bar();
+ Expect.equals(3, result);
+ }
+ int result = y.bar();
+ Expect.equals(4, result);
+}
diff --git a/runtime/tests/vm/vm.status b/runtime/tests/vm/vm.status
index cdc1186..802cc8b 100644
--- a/runtime/tests/vm/vm.status
+++ b/runtime/tests/vm/vm.status
@@ -115,6 +115,7 @@
[ $mode == debug ]
cc/CorelibIsolateStartup: SkipByDesign # This is a benchmark that is not informative in debug mode.
+cc/SixtyThousandDartClasses: SkipSlow # Finalization of 64K classes is too slow in debug mode.
cc/VerifyExplicit_Crash: Crash # Negative tests of VerifiedMemory should crash iff in DEBUG mode. TODO(koda): Improve support for negative tests.
cc/VerifyImplicit_Crash: Crash # Negative tests of VerifiedMemory should crash iff in DEBUG mode. TODO(koda): Improve support for negative tests.
dart/appjit_cha_deopt_test: Pass, Slow # Quite slow in debug mode, uses --optimization-counter-threshold=100
diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc
index ea2fc4a..61cd91d 100644
--- a/runtime/vm/compiler/aot/precompiler.cc
+++ b/runtime/vm/compiler/aot/precompiler.cc
@@ -1122,7 +1122,7 @@
class_count_++;
cls.set_is_allocated(true);
- error_ = cls.EnsureIsFinalized(T);
+ error_ = cls.EnsureIsAllocateFinalized(T);
if (!error_.IsNull()) {
Jump(error_);
}
diff --git a/runtime/vm/compiler/frontend/constant_reader.cc b/runtime/vm/compiler/frontend/constant_reader.cc
index d6bff45..6eb0a03 100644
--- a/runtime/vm/compiler/frontend/constant_reader.cc
+++ b/runtime/vm/compiler/frontend/constant_reader.cc
@@ -233,7 +233,8 @@
"%s is not loaded yet.",
klass.ToCString());
}
- const auto& obj = Object::Handle(Z, klass.EnsureIsFinalized(H.thread()));
+ const auto& obj =
+ Object::Handle(Z, klass.EnsureIsAllocateFinalized(H.thread()));
ASSERT(obj.IsNull());
ASSERT(klass.is_enum_class() || klass.is_const());
instance = Instance::New(klass, Heap::kOld);
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index 2b0c369..49c32bc 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -4627,7 +4627,7 @@
return Api::NewError("Precompilation dropped '%s'", cls.ToCString());
}
#endif
- CHECK_ERROR_HANDLE(cls.EnsureIsFinalized(T));
+ CHECK_ERROR_HANDLE(cls.EnsureIsAllocateFinalized(T));
return Api::NewHandle(T, AllocateObject(T, cls));
}
@@ -4653,7 +4653,7 @@
return Api::NewError("Precompilation dropped '%s'", cls.ToCString());
}
#endif
- CHECK_ERROR_HANDLE(cls.EnsureIsFinalized(T));
+ CHECK_ERROR_HANDLE(cls.EnsureIsAllocateFinalized(T));
if (num_native_fields != cls.num_native_fields()) {
return Api::NewError(
"%s: invalid number of native fields %" Pd " passed in, expected %d",
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 7870499..f11ffea 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -4310,6 +4310,10 @@
UNREACHABLE();
}
}
+ // May be allocate-finalized recursively during EnsureIsFinalized.
+ if (is_allocate_finalized()) {
+ return Error::null();
+ }
error ^= ClassFinalizer::AllocateFinalizeClass(*this);
return error.raw();
}
@@ -18756,7 +18760,7 @@
InstancePtr Instance::New(const Class& cls, Heap::Space space) {
Thread* thread = Thread::Current();
- if (cls.EnsureIsFinalized(thread) != Error::null()) {
+ if (cls.EnsureIsAllocateFinalized(thread) != Error::null()) {
return Instance::null();
}
intptr_t instance_size = cls.host_instance_size();
@@ -23912,7 +23916,7 @@
const Class& cls =
Class::Handle(Isolate::Current()->class_table()->At(kFfiPointerCid));
- cls.EnsureIsFinalized(Thread::Current());
+ cls.EnsureIsAllocateFinalized(Thread::Current());
Pointer& result = Pointer::Handle(zone);
result ^= Object::Allocate(kFfiPointerCid, Pointer::InstanceSize(), space);
