diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index 052bb7399e9..f685e530c73 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -3862,44 +3862,44 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFieldAccessor( } } body += NullConstant(); - } else if (is_getter && is_method) { - ASSERT(!field.needs_load_guard() - NOT_IN_PRODUCT(|| IG->HasAttemptedReload())); - body += LoadLocal(parsed_function_->ParameterVariable(0)); - body += LoadField( - field, /*calls_initializer=*/field.NeedsInitializationCheckOnLoad()); - if (field.needs_load_guard()) { -#if defined(PRODUCT) - UNREACHABLE(); -#else - body += CheckAssignable(AbstractType::Handle(Z, field.type()), - Symbols::FunctionResult()); -#endif - } - } else if (field.is_const()) { - const auto& value = Object::Handle(Z, field.StaticConstFieldValue()); - if (value.IsError()) { - Report::LongJump(Error::Cast(value)); - } - body += Constant(Instance::ZoneHandle(Z, Instance::RawCast(value.ptr()))); } else { - // Static fields - // - with trivial initializer - // - without initializer if they are not late - // are initialized eagerly and do not have implicit getters. - // Static fields with non-trivial initializer need getter to perform - // lazy initialization. Late fields without initializer need getter - // to make sure they are already initialized. - ASSERT(field.has_nontrivial_initializer() || - (field.is_late() && !field.has_initializer())); - body += LoadStaticField(field, /*calls_initializer=*/true); - if (field.needs_load_guard()) { + ASSERT(is_getter); + if (is_method) { + body += LoadLocal(parsed_function_->ParameterVariable(0)); + body += LoadField( + field, /*calls_initializer=*/field.NeedsInitializationCheckOnLoad()); + } else if (field.is_const()) { + const auto& value = Object::Handle(Z, field.StaticConstFieldValue()); + if (value.IsError()) { + Report::LongJump(Error::Cast(value)); + } + body += Constant(Instance::ZoneHandle(Z, Instance::RawCast(value.ptr()))); + } else { + // Static fields + // - with trivial initializer + // - without initializer if they are not late + // are initialized eagerly and do not have implicit getters. + // Static fields with non-trivial initializer need getter to perform + // lazy initialization. Late fields without initializer need getter + // to make sure they are already initialized. + ASSERT(field.has_nontrivial_initializer() || + (field.is_late() && !field.has_initializer())); + body += LoadStaticField(field, /*calls_initializer=*/true); + } + + if (is_method || !field.is_const()) { #if defined(PRODUCT) - UNREACHABLE(); + RELEASE_ASSERT(!field.needs_load_guard()); #else - ASSERT(IsolateGroup::Current()->HasAttemptedReload()); - body += CheckAssignable(AbstractType::Handle(Z, field.type()), - Symbols::FunctionResult()); + // Always build fragment for load guard to maintain stable deopt_id + // numbering, but link it into the graph only if field actually + // needs load guard. + Fragment load_guard = CheckAssignable( + AbstractType::Handle(Z, field.type()), Symbols::FunctionResult()); + if (field.needs_load_guard()) { + ASSERT(IG->HasAttemptedReload()); + body += load_guard; + } #endif } } diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index f902c84584f..43e1de9f201 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -2005,8 +2005,11 @@ void ProgramReloadContext::RunInvalidationVisitors() { InvalidateKernelInfos(zone, kernel_infos); InvalidateSuspendStates(zone, suspend_states); - InvalidateFunctions(zone, functions); InvalidateFields(zone, fields, instances); + + // After InvalidateFields in order to invalidate + // implicit getters which need load guards. + InvalidateFunctions(zone, functions); } void ProgramReloadContext::InvalidateKernelInfos( @@ -2051,6 +2054,7 @@ void ProgramReloadContext::InvalidateFunctions( Class& owning_class = Class::Handle(zone); Library& owning_lib = Library::Handle(zone); Code& code = Code::Handle(zone); + Field& field = Field::Handle(zone); SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); for (intptr_t i = 0; i < functions.length(); i++) { const Function& func = *functions[i]; @@ -2062,9 +2066,20 @@ void ProgramReloadContext::InvalidateFunctions( code = func.CurrentCode(); ASSERT(!code.IsNull()); + // Force recompilation of unoptimized code of implicit getters + // in order to add load guards. This is needed for future + // deoptimizations which will expect load guard in the unoptimized code. + bool recompile_for_load_guard = false; + if (func.IsImplicitGetterFunction() || + func.IsImplicitStaticGetterFunction()) { + field = func.accessor_field(); + recompile_for_load_guard = field.needs_load_guard(); + } + owning_class = func.Owner(); owning_lib = owning_class.library(); - const bool clear_code = IsDirty(owning_lib); + const bool clear_unoptimized_code = + IsDirty(owning_lib) || recompile_for_load_guard; const bool stub_code = code.IsStubCode(); // Zero edge counters, before clearing the ICDataArray, since that's where @@ -2073,7 +2088,7 @@ void ProgramReloadContext::InvalidateFunctions( if (stub_code) { // Nothing to reset. - } else if (clear_code) { + } else if (clear_unoptimized_code) { VTIR_Print("Marking %s for recompilation, clearing code\n", func.ToCString()); // Null out the ICData array and code. diff --git a/runtime/vm/isolate_reload_test.cc b/runtime/vm/isolate_reload_test.cc index 91f6a64bdd4..e09d4d2372b 100644 --- a/runtime/vm/isolate_reload_test.cc +++ b/runtime/vm/isolate_reload_test.cc @@ -5007,6 +5007,63 @@ TEST_CASE(IsolateReload_GenericConstructorTearOff) { EXPECT_STREQ("okay", SimpleInvokeStr(lib, "main")); } +// Regression test for https://github.com/dart-lang/sdk/issues/51215. +TEST_CASE(IsolateReload_ImplicitGetterWithLoadGuard) { + const char* kLibScript = R"( + import 'file:///test:isolate_reload_helper'; + + class A { + int x; + A(this.x); + A.withUinitializedObject(int Function() callback) : x = callback(); + } + + A a = A(3); + + main() { + int sum = 0; + // Trigger OSR and optimize this function. + for (int i = 0; i < 30000; ++i) { + sum += i; + } + // Make sure A.get:x is compiled. + int y = a.x; + // Reload while having an uninitialized + // object A on the stack. This should result in + // a load guard for A.x. + A.withUinitializedObject(() { + reloadTest(); + return 4; + }); + // Trigger OSR and optimize this function once again. + for (int i = 0; i < 30000; ++i) { + sum += i; + } + // Trigger deoptimization in A.get:x. + // It should correctly deoptimize into an implicit + // getter with a load guard. + a.x = 0x8070605040302010; + int z = a.x & 0xffff; + return "y: $y, z: $z"; + } + )"; + + Dart_Handle lib1 = + TestCase::LoadTestLibrary("test_lib1.dart", kLibScript, nullptr); + EXPECT_VALID(lib1); + + const char* kMainScript = R"( + main() {} + )"; + + // Trigger hot reload during execution of 'main' from test_lib1 + // without reloading test_lib1, so its unoptimized code is retained. + EXPECT_VALID(TestCase::LoadTestScript(kMainScript, nullptr)); + EXPECT_VALID(TestCase::SetReloadTestScript(kMainScript)); + + EXPECT_STREQ("y: 3, z: 8208", SimpleInvokeStr(lib1, "main")); +} + #endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) } // namespace dart diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index 57286f754cd..2b1e2578c40 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -284,13 +284,19 @@ void FUNCTION_NAME(Test_CollectOldSpace)(Dart_NativeArguments native_args) { GCTestHelper::CollectOldSpace(); } -static Dart_Handle LoadIsolateReloadTestLib() { - return TestCase::LoadTestLibrary(IsolateReloadTestLibUri(), - kIsolateReloadTestLibSource, - IsolateReloadTestNativeResolver); -} #endif // !PRODUCT +static void LoadIsolateReloadTestLibIfNeeded(const char* script) { +#ifndef PRODUCT + if (strstr(script, IsolateReloadTestLibUri()) != nullptr) { + Dart_Handle result = TestCase::LoadTestLibrary( + IsolateReloadTestLibUri(), kIsolateReloadTestLibSource, + IsolateReloadTestNativeResolver); + EXPECT_VALID(result); + } +#endif // ifndef PRODUCT +} + char* TestCase::CompileTestScriptWithDFE(const char* url, const char* source, const uint8_t** kernel_buffer, @@ -418,12 +424,7 @@ Dart_Handle TestCase::LoadTestScript(const char* script, const char* lib_url, bool finalize_classes, bool allow_compile_errors) { -#ifndef PRODUCT - if (strstr(script, IsolateReloadTestLibUri()) != NULL) { - Dart_Handle result = LoadIsolateReloadTestLib(); - EXPECT_VALID(result); - } -#endif // ifndef PRODUCT + LoadIsolateReloadTestLibIfNeeded(script); Dart_SourceFile* sourcefiles = NULL; intptr_t num_sources = BuildSourceFilesArray(&sourcefiles, script, lib_url); Dart_Handle result = @@ -440,6 +441,7 @@ static void MallocFinalizer(void* isolate_callback_data, void* peer) { Dart_Handle TestCase::LoadTestLibrary(const char* lib_uri, const char* script, Dart_NativeEntryResolver resolver) { + LoadIsolateReloadTestLibIfNeeded(script); const char* prefixed_lib_uri = OS::SCreate(Thread::Current()->zone(), "file:///%s", lib_uri); Dart_SourceFile sourcefiles[] = {{prefixed_lib_uri, script}};