Skip to content

Commit fe37377

Browse files
mkustermanncommit-bot@chromium.org
mkustermann
authored and
commit-bot@chromium.org
committed
[vm/constants-2019] Fix crashing VM by ensuring constants table is preserved across snapshots
When constants are lazily created in VM from the constants table present in the Kernel file, we have to ensure to preserve the constants table across snapshot serialization/deserialization. Also there is no need to use zone handles in the constant evaluator for local handles. Also removes some commented code and adds an assertion that we never try to partially instantiate a closure with non-null function type arguments. Issue #37357 Change-Id: I49588fd18d981b7aa07c61e845cd2e2161b122bf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107283 Commit-Queue: Martin Kustermann <kustermann@google.com> Auto-Submit: Martin Kustermann <kustermann@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
1 parent 9de3e7c commit fe37377

File tree

2 files changed

+53
-59
lines changed

2 files changed

+53
-59
lines changed

‎runtime/vm/compiler/frontend/constant_evaluator.cc‎

Lines changed: 52 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,9 @@ RawInstance* ConstantEvaluator::EvaluateConstant(intptr_t constant_offset) {
381381
case kSymbolConstant: {
382382
Library& library = Library::Handle(Z);
383383
library = Library::InternalLibrary();
384-
const Class& symbol_class =
384+
const auto& symbol_class =
385385
Class::Handle(Z, library.LookupClass(Symbols::Symbol()));
386-
const Field& symbol_name_field = Field::Handle(
386+
const auto& symbol_name_field = Field::Handle(
387387
Z, symbol_class.LookupInstanceFieldAllowPrivate(Symbols::_name()));
388388
ASSERT(!symbol_name_field.IsNull());
389389
const NameIndex index = reader.ReadCanonicalNameReference();
@@ -399,13 +399,13 @@ RawInstance* ConstantEvaluator::EvaluateConstant(intptr_t constant_offset) {
399399
break;
400400
}
401401
case kListConstant: {
402-
const Library& corelib = Library::Handle(Z, Library::CoreLibrary());
403-
const Class& list_class =
402+
const auto& corelib = Library::Handle(Z, Library::CoreLibrary());
403+
const auto& list_class =
404404
Class::Handle(Z, corelib.LookupClassAllowPrivate(Symbols::_List()));
405405
// Build type from the raw bytes (needs temporary translator).
406406
TypeTranslator type_translator(&reader, active_class_, true);
407-
TypeArguments& type_arguments =
408-
TypeArguments::ZoneHandle(Z, TypeArguments::New(1, Heap::kOld));
407+
auto& type_arguments =
408+
TypeArguments::Handle(Z, TypeArguments::New(1, Heap::kOld));
409409
AbstractType& type = type_translator.BuildType();
410410
type_arguments.SetTypeAt(0, type);
411411
// Instantiate class.
@@ -432,22 +432,21 @@ RawInstance* ConstantEvaluator::EvaluateConstant(intptr_t constant_offset) {
432432
}
433433
case kInstanceConstant: {
434434
const NameIndex index = reader.ReadCanonicalNameReference();
435-
const Class& klass = Class::Handle(Z, H.LookupClassByKernelClass(index));
436-
const Object& obj =
437-
Object::Handle(Z, klass.EnsureIsFinalized(H.thread()));
435+
const auto& klass = Class::Handle(Z, H.LookupClassByKernelClass(index));
436+
const auto& obj = Object::Handle(Z, klass.EnsureIsFinalized(H.thread()));
438437
ASSERT(obj.IsNull());
439438
instance = Instance::New(klass, Heap::kOld);
440439
// Build type from the raw bytes (needs temporary translator).
441440
TypeTranslator type_translator(&reader, active_class_, true);
442441
const intptr_t number_of_type_arguments = reader.ReadUInt();
443442
if (klass.NumTypeArguments() > 0) {
444-
TypeArguments& type_arguments = TypeArguments::ZoneHandle(
443+
auto& type_arguments = TypeArguments::Handle(
445444
Z, TypeArguments::New(number_of_type_arguments, Heap::kOld));
446445
for (intptr_t j = 0; j < number_of_type_arguments; ++j) {
447446
type_arguments.SetTypeAt(j, type_translator.BuildType());
448447
}
449448
// Instantiate class.
450-
AbstractType& type = AbstractType::Handle(
449+
auto& type = AbstractType::Handle(
451450
Z, Type::New(klass, type_arguments, TokenPosition::kNoSource));
452451
type = ClassFinalizer::FinalizeType(*active_class_->klass, type,
453452
ClassFinalizer::kCanonicalize);
@@ -476,20 +475,15 @@ RawInstance* ConstantEvaluator::EvaluateConstant(intptr_t constant_offset) {
476475
// needed to evaluate the current constant.
477476
const intptr_t entry_offset = reader.ReadUInt();
478477
ASSERT(entry_offset < constant_offset); // DAG!
479-
Instance& constant =
478+
const auto& constant =
480479
Instance::Handle(Z, EvaluateConstantExpression(entry_offset));
481-
// Happens if the tearoff was in the vmservice library and we have
482-
// [skip_vm_service_library] enabled.
483-
// TODO(ajcbik): probably ASSERT that this no longer happens
484-
if (constant.IsNull()) {
485-
instance = Instance::null();
486-
break;
487-
}
480+
ASSERT(!constant.IsNull());
481+
488482
// Build type from the raw bytes (needs temporary translator).
489483
TypeTranslator type_translator(&reader, active_class_, true);
490484
const intptr_t number_of_type_arguments = reader.ReadUInt();
491485
ASSERT(number_of_type_arguments > 0);
492-
TypeArguments& type_arguments = TypeArguments::ZoneHandle(
486+
auto& type_arguments = TypeArguments::Handle(
493487
Z, TypeArguments::New(number_of_type_arguments, Heap::kOld));
494488
for (intptr_t j = 0; j < number_of_type_arguments; ++j) {
495489
type_arguments.SetTypeAt(j, type_translator.BuildType());
@@ -498,11 +492,12 @@ RawInstance* ConstantEvaluator::EvaluateConstant(intptr_t constant_offset) {
498492
// Make a copy of the old closure, and set delayed type arguments.
499493
Closure& closure = Closure::Handle(Z, Closure::RawCast(constant.raw()));
500494
Function& function = Function::Handle(Z, closure.function());
501-
TypeArguments& type_arguments2 =
502-
TypeArguments::ZoneHandle(Z, closure.instantiator_type_arguments());
503-
// TODO(ajcbik): why was this here in original reader?
504-
// TypeArguments& type_arguments3 =
505-
// TypeArguments::ZoneHandle(Z, closure.function_type_arguments());
495+
const auto& type_arguments2 =
496+
TypeArguments::Handle(Z, closure.instantiator_type_arguments());
497+
// The function type arguments are used for type parameters from enclosing
498+
// closures. Though inner closures cannot be constants. We should
499+
// therefore see `null here.
500+
ASSERT(closure.function_type_arguments() == TypeArguments::null());
506501
Context& context = Context::Handle(Z, closure.context());
507502
instance = Closure::New(type_arguments2, Object::null_type_arguments(),
508503
type_arguments, function, context, Heap::kOld);
@@ -577,7 +572,7 @@ void ConstantEvaluator::EvaluateGetStringLength(intptr_t expression_offset,
577572
TokenPosition position) {
578573
EvaluateExpression(expression_offset);
579574
if (result_.IsString()) {
580-
const String& str = String::Handle(Z, String::RawCast(result_.raw()));
575+
const auto& str = String::Handle(Z, String::RawCast(result_.raw()));
581576
result_ = Integer::New(str.Length(), H.allocation_space());
582577
} else {
583578
H.ReportError(
@@ -628,7 +623,7 @@ void ConstantEvaluator::EvaluateStaticGet() {
628623
ASSERT(Error::Handle(Z, H.thread()->sticky_error()).IsNull());
629624

630625
if (H.IsField(target)) {
631-
const Field& field = Field::Handle(Z, H.LookupFieldByKernelField(target));
626+
const auto& field = Field::Handle(Z, H.LookupFieldByKernelField(target));
632627
if (!field.is_const()) {
633628
H.ReportError(script_, position, "Not a constant field.");
634629
}
@@ -640,7 +635,7 @@ void ConstantEvaluator::EvaluateStaticGet() {
640635
H.ReportError(script_, position, "Not a constant expression.");
641636
} else if (field.StaticValue() == Object::sentinel().raw()) {
642637
field.SetStaticValue(Object::transition_sentinel());
643-
const Object& value = Object::Handle(Z, field.EvaluateInitializer());
638+
const auto& value = Object::Handle(Z, field.EvaluateInitializer());
644639
if (value.IsError()) {
645640
field.SetStaticValue(Object::null_instance());
646641
H.ReportError(Error::Cast(value), script_, position,
@@ -664,12 +659,12 @@ void ConstantEvaluator::EvaluateStaticGet() {
664659
result_ = field.StaticValue();
665660
}
666661
} else if (H.IsProcedure(target)) {
667-
const Function& function =
668-
Function::ZoneHandle(Z, H.LookupStaticMethodByKernelProcedure(target));
662+
const auto& function =
663+
Function::Handle(Z, H.LookupStaticMethodByKernelProcedure(target));
669664

670665
if (H.IsMethod(target)) {
671-
Function& closure_function =
672-
Function::ZoneHandle(Z, function.ImplicitClosureFunction());
666+
const auto& closure_function =
667+
Function::Handle(Z, function.ImplicitClosureFunction());
673668
result_ = closure_function.ImplicitStaticClosure();
674669
result_ = H.Canonicalize(result_);
675670
} else if (H.IsGetter(target)) {
@@ -683,7 +678,7 @@ void ConstantEvaluator::EvaluateStaticGet() {
683678
void ConstantEvaluator::EvaluateMethodInvocation() {
684679
TokenPosition position = helper_->ReadPosition(); // read position.
685680
// This method call wasn't cached, so receiver et al. isn't cached either.
686-
const Instance& receiver = Instance::Handle(
681+
const auto& receiver = Instance::Handle(
687682
Z, EvaluateExpression(helper_->ReaderOffset(), false)); // read receiver.
688683
Class& klass =
689684
Class::Handle(Z, isolate_->class_table()->At(receiver.GetClassId()));
@@ -709,13 +704,13 @@ void ConstantEvaluator::EvaluateMethodInvocation() {
709704
void ConstantEvaluator::EvaluateDirectMethodInvocation() {
710705
TokenPosition position = helper_->ReadPosition(); // read position.
711706

712-
const Instance& receiver = Instance::Handle(
707+
const auto& receiver = Instance::Handle(
713708
Z, EvaluateExpression(helper_->ReaderOffset(), false)); // read receiver.
714709

715710
NameIndex kernel_name =
716711
helper_->ReadCanonicalNameReference(); // read target_reference.
717712

718-
const Function& function = Function::ZoneHandle(
713+
const auto& function = Function::Handle(
719714
Z, H.LookupMethodByMember(kernel_name, H.DartProcedureName(kernel_name)));
720715

721716
// Read arguments, run the method and canonicalize the result.
@@ -739,7 +734,7 @@ void ConstantEvaluator::EvaluateSuperMethodInvocation() {
739734
ASSERT(!klass.IsNull());
740735

741736
const String& method_name = helper_->ReadNameAsMethodName(); // read name.
742-
Function& function =
737+
const auto& function =
743738
Function::Handle(Z, H.LookupDynamicFunction(klass, method_name));
744739

745740
// The frontend should guarantee that [MethodInvocation]s inside constant
@@ -759,7 +754,7 @@ void ConstantEvaluator::EvaluateStaticInvocation() {
759754
NameIndex procedure_reference =
760755
helper_->ReadCanonicalNameReference(); // read procedure reference.
761756

762-
const Function& function = Function::ZoneHandle(
757+
const auto& function = Function::Handle(
763758
Z, H.LookupStaticMethodByKernelProcedure(procedure_reference));
764759
Class& klass = Class::Handle(Z, function.Owner());
765760

@@ -781,7 +776,7 @@ void ConstantEvaluator::EvaluateConstructorInvocationInternal() {
781776
TokenPosition position = helper_->ReadPosition(); // read position.
782777

783778
NameIndex target = helper_->ReadCanonicalNameReference(); // read target.
784-
const Function& constructor =
779+
const auto& constructor =
785780
Function::Handle(Z, H.LookupConstructorByKernelConstructor(target));
786781
Class& klass = Class::Handle(Z, constructor.Owner());
787782

@@ -793,12 +788,12 @@ void ConstantEvaluator::EvaluateConstructorInvocationInternal() {
793788
TranslateTypeArguments(constructor, &klass); // read argument types.
794789

795790
if (klass.NumTypeArguments() > 0 && !klass.IsGeneric()) {
796-
Type& type = Type::ZoneHandle(Z, T.ReceiverType(klass).raw());
791+
auto& type = Type::Handle(Z, T.ReceiverType(klass).raw());
797792
// TODO(27590): Can we move this code into [ReceiverType]?
798793
type ^= ClassFinalizer::FinalizeType(*active_class_->klass, type,
799794
ClassFinalizer::kFinalize);
800-
TypeArguments& canonicalized_type_arguments =
801-
TypeArguments::ZoneHandle(Z, type.arguments());
795+
auto& canonicalized_type_arguments =
796+
TypeArguments::Handle(Z, type.arguments());
802797
canonicalized_type_arguments = canonicalized_type_arguments.Canonicalize();
803798
type_arguments = &canonicalized_type_arguments;
804799
}
@@ -873,22 +868,22 @@ void ConstantEvaluator::EvaluateAsExpression() {
873868

874869
const AbstractType& type = T.BuildType();
875870
if (!type.IsInstantiated()) {
876-
const String& type_str = String::Handle(type.UserVisibleName());
871+
const auto& type_str = String::Handle(type.UserVisibleName());
877872
H.ReportError(
878873
script_, position,
879874
"Not a constant expression: right hand side of an implicit "
880875
"as-expression is expected to be an instantiated type, got %s",
881876
type_str.ToCString());
882877
}
883878

884-
const TypeArguments& instantiator_type_arguments = TypeArguments::Handle();
885-
const TypeArguments& function_type_arguments = TypeArguments::Handle();
879+
const auto& instantiator_type_arguments = TypeArguments::Handle();
880+
const auto& function_type_arguments = TypeArguments::Handle();
886881
if (!result_.IsInstanceOf(type, instantiator_type_arguments,
887882
function_type_arguments)) {
888883
const AbstractType& rtype =
889884
AbstractType::Handle(result_.GetType(Heap::kNew));
890-
const String& result_str = String::Handle(rtype.UserVisibleName());
891-
const String& type_str = String::Handle(type.UserVisibleName());
885+
const auto& result_str = String::Handle(rtype.UserVisibleName());
886+
const auto& type_str = String::Handle(type.UserVisibleName());
892887
H.ReportError(
893888
script_, position,
894889
"Not a constant expression: Type '%s' is not a subtype of type '%s'",
@@ -929,13 +924,13 @@ void ConstantEvaluator::EvaluateStringConcatenation() {
929924
const Class& cls =
930925
Class::Handle(Z, Library::LookupCoreClass(Symbols::StringBase()));
931926
ASSERT(!cls.IsNull());
932-
const Function& func = Function::Handle(
927+
const auto& func = Function::Handle(
933928
Z, cls.LookupStaticFunction(
934929
Library::PrivateCoreLibName(Symbols::Interpolate())));
935930
ASSERT(!func.IsNull());
936931

937932
// Build argument array to pass to the interpolation function.
938-
const Array& interpolate_arg = Array::Handle(Z, Array::New(1, Heap::kOld));
933+
const auto& interpolate_arg = Array::Handle(Z, Array::New(1, Heap::kOld));
939934
interpolate_arg.SetAt(0, strings);
940935

941936
// Run and canonicalize.
@@ -950,9 +945,9 @@ void ConstantEvaluator::EvaluateSymbolLiteral() {
950945
const Library& lib = Library::Handle(Z, owner.library());
951946
String& symbol_value = H.DartIdentifier(lib, helper_->ReadStringReference());
952947
const Class& symbol_class =
953-
Class::ZoneHandle(Z, I->object_store()->symbol_class());
948+
Class::Handle(Z, I->object_store()->symbol_class());
954949
ASSERT(!symbol_class.IsNull());
955-
const Function& symbol_constructor = Function::ZoneHandle(
950+
const auto& symbol_constructor = Function::Handle(
956951
Z, symbol_class.LookupConstructor(Symbols::SymbolCtor()));
957952
ASSERT(!symbol_constructor.IsNull());
958953
result_ ^= EvaluateConstConstructorCall(
@@ -968,8 +963,7 @@ void ConstantEvaluator::EvaluateListLiteralInternal() {
968963
helper_->ReadPosition(); // read position.
969964
const TypeArguments& type_arguments = T.BuildTypeArguments(1); // read type.
970965
intptr_t length = helper_->ReadListLength(); // read list length.
971-
const Array& const_list =
972-
Array::ZoneHandle(Z, Array::New(length, Heap::kOld));
966+
const auto& const_list = Array::Handle(Z, Array::New(length, Heap::kOld));
973967
const_list.SetTypeArguments(type_arguments);
974968
Instance& expression = Instance::Handle(Z);
975969
for (intptr_t i = 0; i < length; ++i) {
@@ -1043,7 +1037,7 @@ void ConstantEvaluator::EvaluateLet() {
10431037

10441038
void ConstantEvaluator::EvaluatePartialTearoffInstantiation() {
10451039
// This method call wasn't cached, so receiver et al. isn't cached either.
1046-
const Instance& receiver = Instance::Handle(
1040+
const auto& receiver = Instance::Handle(
10471041
Z, EvaluateExpression(helper_->ReaderOffset(), false)); // read receiver.
10481042
if (!receiver.IsClosure()) {
10491043
H.ReportError(script_, TokenPosition::kNoSource, "Expected closure.");
@@ -1133,7 +1127,7 @@ const Object& ConstantEvaluator::RunFunction(TokenPosition position,
11331127
(receiver != NULL ? 1 : 0) + (type_args != NULL ? 1 : 0);
11341128

11351129
// Build up arguments.
1136-
const Array& arguments = Array::Handle(
1130+
const auto& arguments = Array::Handle(
11371131
Z, Array::New(extra_arguments + argument_count, H.allocation_space()));
11381132
intptr_t pos = 0;
11391133
if (receiver != NULL) {
@@ -1173,9 +1167,9 @@ const Object& ConstantEvaluator::RunFunction(const TokenPosition position,
11731167
const Array& names) {
11741168
// We do not support generic methods yet.
11751169
const int kTypeArgsLen = 0;
1176-
const Array& args_descriptor = Array::Handle(
1170+
const auto& args_descriptor = Array::Handle(
11771171
Z, ArgumentsDescriptor::New(kTypeArgsLen, arguments.Length(), names));
1178-
const Object& result = Object::Handle(
1172+
const auto& result = Object::Handle(
11791173
Z, DartEntry::InvokeFunction(function, arguments, args_descriptor));
11801174
if (result.IsError()) {
11811175
H.ReportError(Error::Cast(result), script_, position,
@@ -1235,7 +1229,7 @@ RawObject* ConstantEvaluator::EvaluateConstConstructorCall(
12351229
const Array& args_descriptor =
12361230
Array::Handle(Z, ArgumentsDescriptor::New(kTypeArgsLen, argument_count,
12371231
Object::empty_array()));
1238-
const Object& result = Object::Handle(
1232+
const auto& result = Object::Handle(
12391233
Z, DartEntry::InvokeFunction(constructor, arg_values, args_descriptor));
12401234
ASSERT(!result.IsError());
12411235
if (constructor.IsFactory()) {
@@ -1325,7 +1319,7 @@ void ConstantEvaluator::CacheConstantValue(intptr_t kernel_offset,
13251319
const intptr_t kInitialConstMapSize = 16;
13261320
ASSERT(!script_.InVMIsolateHeap());
13271321
if (script_.compile_time_constants() == Array::null()) {
1328-
const Array& array = Array::Handle(
1322+
const auto& array = Array::Handle(
13291323
HashTables::New<KernelConstantsMap>(kInitialConstMapSize, Heap::kNew));
13301324
script_.set_compile_time_constants(array);
13311325
}

‎runtime/vm/raw_object.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,7 @@ class RawKernelProgramInfo : public RawObject {
12661266
VISIT_TO(RawObject*, classes_cache_);
12671267

12681268
RawObject** to_snapshot(Snapshot::Kind kind) {
1269-
return reinterpret_cast<RawObject**>(&ptr()->potential_natives_);
1269+
return reinterpret_cast<RawObject**>(&ptr()->constants_table_);
12701270
}
12711271
};
12721272

0 commit comments

Comments
 (0)