diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 4be42f80ecf..f22bfea4048 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -1004,8 +1004,12 @@ void IsolateGroup::RehashConstants(Become* become) { ASSERT(!old_value.IsNull()); if (become == nullptr) { - ASSERT(old_value.IsCanonical()); - cls.InsertCanonicalConstant(zone, old_value); + if (old_value.IsCanonical()) { + cls.InsertCanonicalConstant(zone, old_value); + } else { + // The deleted enum value sentinel is not marked canonical. + ASSERT(cls.is_enum_class()); + } } else { new_value = old_value.Canonicalize(thread); if (old_value.ptr() != new_value.ptr()) { diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index e8ab78683e9..f867a593cf1 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -1790,16 +1790,24 @@ void ProgramReloadContext::CommitBeforeInstanceMorphing() { } void ProgramReloadContext::CommitAfterInstanceMorphing() { - // Rehash constants map for all classes. Constants are hashed by content, and - // content may have changed from fields being added or removed. { + // Rehash constants map for all classes. Constants are hashed by content, + // and content may have changed from fields being added or removed. TIMELINE_SCOPE(RehashConstants); IG->RehashConstants(&become_); } { + // Forward old enum values to new enum values. Note this is a nop if the + // become operation is empty. TIMELINE_SCOPE(ForwardEnums); become_.Forward(); } + { + // Rehash again, since the become operation may have merged some constants + // and various things are unhappy with duplicates in the canonical tables. + TIMELINE_SCOPE(RehashConstants); + IG->RehashConstants(nullptr); + } if (FLAG_identity_reload) { const auto& saved_libs = GrowableObjectArray::Handle(saved_libraries_); diff --git a/runtime/vm/isolate_reload_test.cc b/runtime/vm/isolate_reload_test.cc index 74330889517..45c212a54ef 100644 --- a/runtime/vm/isolate_reload_test.cc +++ b/runtime/vm/isolate_reload_test.cc @@ -2259,6 +2259,49 @@ TEST_CASE(IsolateReload_EnumIdentityReload) { SimpleInvokeStr(lib, "main")); } +TEST_CASE(IsolateReload_EnumDeleteMultiple) { + // See https://github.com/dart-lang/sdk/issues/56583. + // Accessing Fruit.values will cause a const array with all the enum values + // to stick around in the canonical table for _List. + const char* kScript = + "enum Fruit { Apple, Banana, Cherry }\n" + "var retained;\n" + "main() {\n" + " retained = Fruit.values[0];\n" + " return retained.toString();\n" + "}\n"; + + Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr); + EXPECT_VALID(lib); + EXPECT_STREQ("Fruit.Apple", SimpleInvokeStr(lib, "main")); + + // Both Banana and Cherry forwarded to the deleted-enum sentinel, and + // two copies of the sentinel remain in Fruit's canonical table. + const char* kReloadScript0 = + "enum Fruit { Apple }\n" + "var retained;\n" + "main() {\n" + " return retained.toString();\n" + "}\n"; + + lib = TestCase::ReloadTestScript(kReloadScript0); + EXPECT_VALID(lib); + EXPECT_STREQ("Fruit.Apple", SimpleInvokeStr(lib, "main")); + + // When visiting Fruit's canonical table, we try to forward both entries of + // the old sentinel to the new sentinel, creating a become conflict. + const char* kReloadScript1 = + "enum Fruit { Apple }\n" + "var retained;\n" + "main() {\n" + " return retained.toString();\n" + "}\n"; + + lib = TestCase::ReloadTestScript(kReloadScript1); + EXPECT_VALID(lib); + EXPECT_STREQ("Fruit.Apple", SimpleInvokeStr(lib, "main")); +} + TEST_CASE(IsolateReload_EnumShapeChange) { const char* kScript = "enum Fruit { Apple, Banana }\n"