dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Account for changes in the implicit getters due to load guards

Browse Source 
Hot reload may mark certain fields as 'needs_load_guard', meaning
types of their values should be checked on access, in the implicit
getters.

Unoptimized code for implicit getters of such fields should be
forcefully recompiled during hot reload in order to update their
code. This ensures that optimized code in future can deoptimize
into the updated unoptimized code of implicit getters.

TEST=vm/cc/IsolateReload_ImplicitGetterWithLoadGuard
TEST=corelib/string_fromcharcodes_test, lib/convert/json_utf8_chunk_test
and vm/dart/string_equals_test on vm-reload-linux-{debug,release}-x64
configurations.

Fixes https://github.com/dart-lang/sdk/issues/51215
Fixes https://github.com/flutter/flutter/issues/103268

Change-Id: I1afbeabb4a60b7a5f69ed055f40beeb6d3dcf359
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280401
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Alexander Markov
2023-02-03 22:23:22 +00:00
committed by Commit Queue
parent 3787601dd0
commit 87810072db
4 changed files with 123 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/compiler/frontend/kernel_to_il.cc
+35 -35
View File
@@ -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
}
}
runtime/vm/isolate_reload.cc
+18 -3
View File
@@ -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.
runtime/vm/isolate_reload_test.cc
+57
View File
@@ -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
runtime/vm/unit_test.cc
+13 -11
View File
@@ -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}};