From 53d859f6b9d511b3d65b7db6b22c759834224b0a Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Tue, 13 Dec 2022 15:28:19 +0000 Subject: [PATCH] Revert "[VM/CLI] Remove dartdev.dill" This reverts commit c09f790d371c206f4a8fc10ec19c2766fc5c2e57. Reason for revert: CI failures Original change's description: > [VM/CLI] Remove dartdev.dill > > Incompatible VM flags will no longer break the CLI when running from an > AppJIT snapshot, so the fallback logic is no longer required. This CL > thus removes dartdev.dill and the fallback logic. > > Relevant past CLs: https://dart-review.googlesource.com/c/sdk/+/157601 > and https://dart-review.googlesource.com/c/sdk/+/178300 > > Fixes https://github.com/dart-lang/sdk/issues/50504 > > TEST=I tried running `out/ReleaseX64/dart --observe --sound-null-safety test.dart` > and `out/ReleaseX64/dart --observe --no-sound-null-safety test.dart` and > both worked. > > Change-Id: I5cdcfbccf71ec557964014fdb80733b4a7c76b4d > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274520 > Reviewed-by: Ben Konyi > Commit-Queue: Derek Xu TBR=bkonyi@google.com,derekx@google.com,dart-scoped@luci-project-accounts.iam.gserviceaccount.com Change-Id: I5117f990dfabf93f5a9bae56098831280845e84e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/275181 Reviewed-by: Ben Konyi Commit-Queue: Derek Xu --- pkg/dartdev/test/load_from_dill_test.dart | 29 ++++++++++ runtime/bin/dartdev_isolate.cc | 16 ++++-- runtime/bin/dartdev_isolate.h | 5 ++ runtime/bin/main.cc | 64 ++++++++++++++++++----- sdk/BUILD.gn | 13 +++++ utils/dartdev/BUILD.gn | 17 ++++++ 6 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 pkg/dartdev/test/load_from_dill_test.dart diff --git a/pkg/dartdev/test/load_from_dill_test.dart b/pkg/dartdev/test/load_from_dill_test.dart new file mode 100644 index 00000000000..1c4542bffe1 --- /dev/null +++ b/pkg/dartdev/test/load_from_dill_test.dart @@ -0,0 +1,29 @@ +// Copyright (c) 2021, 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. + +import 'dart:io'; + +import 'package:test/test.dart'; + +import 'utils.dart'; + +void main() { + late TestProject p; + + tearDown(() async => await p.dispose()); + + test("Fallback to dartdev.dill from dartdev.dart.snapshot for 'Hello World'", + () async { + p = project(mainSrc: "void main() { print('Hello World'); }"); + // The DartDev snapshot includes the --use_field_guards flag. If + // --no-use-field-guards is passed, the VM will fail to load the + // snapshot and should fall back to using the DartDev dill file. + ProcessResult result = + await p.run(['--no-use-field-guards', 'run', p.relativeFilePath]); + + expect(result.stdout, contains('Hello World')); + expect(result.stderr, isEmpty); + expect(result.exitCode, 0); + }); +} diff --git a/runtime/bin/dartdev_isolate.cc b/runtime/bin/dartdev_isolate.cc index f54c7380790..1406ef3ebb2 100644 --- a/runtime/bin/dartdev_isolate.cc +++ b/runtime/bin/dartdev_isolate.cc @@ -59,14 +59,14 @@ bool DartDevIsolate::ShouldParseCommand(const char* script_uri) { (strncmp(script_uri, "google3://", 10) != 0)); } -Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() { - const char* snapshot_filename = "dartdev.dart.snapshot"; +Utils::CStringUniquePtr DartDevIsolate::TryResolveArtifactPath( + const char* filename) { // |dir_prefix| includes the last path separator. auto dir_prefix = EXEUtils::GetDirectoryPrefixFromExeName(); // First assume we're in dart-sdk/bin. char* snapshot_path = - Utils::SCreate("%ssnapshots/%s", dir_prefix.get(), snapshot_filename); + Utils::SCreate("%ssnapshots/%s", dir_prefix.get(), filename); if (File::Exists(nullptr, snapshot_path)) { return Utils::CreateCStringUniquePtr(snapshot_path); } @@ -74,7 +74,7 @@ Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() { // If we're not in dart-sdk/bin, we might be in one of the $SDK/out/* // directories. Try to use a snapshot from a previously built SDK. - snapshot_path = Utils::SCreate("%s%s", dir_prefix.get(), snapshot_filename); + snapshot_path = Utils::SCreate("%s%s", dir_prefix.get(), filename); if (File::Exists(nullptr, snapshot_path)) { return Utils::CreateCStringUniquePtr(snapshot_path); } @@ -82,6 +82,14 @@ Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() { return Utils::CreateCStringUniquePtr(nullptr); } +Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() { + return TryResolveArtifactPath("dartdev.dart.snapshot"); +} + +Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevKernelPath() { + return TryResolveArtifactPath("dartdev.dill"); +} + void DartDevIsolate::DartDevRunner::Run( Dart_IsolateGroupCreateCallback create_isolate, char** packages_file, diff --git a/runtime/bin/dartdev_isolate.h b/runtime/bin/dartdev_isolate.h index 4992427e8d7..0d7f74ae539 100644 --- a/runtime/bin/dartdev_isolate.h +++ b/runtime/bin/dartdev_isolate.h @@ -46,6 +46,9 @@ class DartDevIsolate { static bool should_run_dart_dev() { return should_run_dart_dev_; } + // Attempts to find the path of the DartDev kernel file. + static Utils::CStringUniquePtr TryResolveDartDevKernelPath(); + // Attempts to find the path of the DartDev snapshot. static Utils::CStringUniquePtr TryResolveDartDevSnapshotPath(); @@ -96,6 +99,8 @@ class DartDevIsolate { }; private: + static Utils::CStringUniquePtr TryResolveArtifactPath(const char* filename); + static DartDevRunner runner_; static bool should_run_dart_dev_; static bool print_usage_error_; diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc index 84564b1c99a..04deaf5ccaa 100644 --- a/runtime/bin/main.cc +++ b/runtime/bin/main.cc @@ -581,35 +581,75 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri, char** error, int* exit_code) { int64_t start = Dart_TimelineGetMicros(); + auto dartdev_path = DartDevIsolate::TryResolveDartDevSnapshotPath(); - if (dartdev_path.get() == nullptr) { - Syslog::PrintErr( - "Failed to start the Dart CLI isolate. Could not resolve DartDev " - "snapshot.\n"); - return nullptr; - } + + Dart_Isolate isolate = nullptr; + const uint8_t* isolate_snapshot_data = core_isolate_snapshot_data; + const uint8_t* isolate_snapshot_instructions = + core_isolate_snapshot_instructions; + IsolateGroupData* isolate_group_data = nullptr; + IsolateData* isolate_data = nullptr; if (error != nullptr) { *error = nullptr; } - Dart_Isolate isolate = nullptr; AppSnapshot* app_snapshot = nullptr; - const bool isolate_run_app_snapshot = true; + bool isolate_run_app_snapshot = true; if (dartdev_path.get() != nullptr && (app_snapshot = Snapshot::TryReadAppSnapshot( dartdev_path.get(), /*force_load_elf_from_memory=*/false, /*decode_uri=*/false)) != nullptr) { - const uint8_t* isolate_snapshot_data = nullptr; - const uint8_t* isolate_snapshot_instructions = nullptr; + const uint8_t* isolate_snapshot_data = NULL; + const uint8_t* isolate_snapshot_instructions = NULL; const uint8_t* ignore_vm_snapshot_data; const uint8_t* ignore_vm_snapshot_instructions; app_snapshot->SetBuffers( &ignore_vm_snapshot_data, &ignore_vm_snapshot_instructions, &isolate_snapshot_data, &isolate_snapshot_instructions); - IsolateGroupData* isolate_group_data = + isolate_group_data = new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config, app_snapshot, isolate_run_app_snapshot); - IsolateData* isolate_data = new IsolateData(isolate_group_data); + isolate_data = new IsolateData(isolate_group_data); + isolate = Dart_CreateIsolateGroup( + DART_DEV_ISOLATE_NAME, DART_DEV_ISOLATE_NAME, isolate_snapshot_data, + isolate_snapshot_instructions, flags, isolate_group_data, isolate_data, + error); + } + + if (isolate == nullptr) { + isolate_run_app_snapshot = false; + dartdev_path = DartDevIsolate::TryResolveDartDevKernelPath(); + // Clear error from app snapshot and retry from kernel. + if (error != nullptr && *error != nullptr) { + free(*error); + *error = nullptr; + } + + if (app_snapshot != nullptr) { + delete app_snapshot; + } + + if (dartdev_path.get() == nullptr) { + Syslog::PrintErr( + "Failed to start the Dart CLI isolate. Could not resolve DartDev " + "snapshot or kernel.\n"); + delete isolate_data; + delete isolate_group_data; + return nullptr; + } + + isolate_group_data = + new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config, nullptr, + isolate_run_app_snapshot); + uint8_t* application_kernel_buffer = NULL; + intptr_t application_kernel_buffer_size = 0; + dfe.ReadScript(dartdev_path.get(), &application_kernel_buffer, + &application_kernel_buffer_size, /*decode_uri=*/false); + isolate_group_data->SetKernelBufferNewlyOwned( + application_kernel_buffer, application_kernel_buffer_size); + + isolate_data = new IsolateData(isolate_group_data); isolate = Dart_CreateIsolateGroup( DART_DEV_ISOLATE_NAME, DART_DEV_ISOLATE_NAME, isolate_snapshot_data, isolate_snapshot_instructions, flags, isolate_group_data, isolate_data, diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index f62bad80bbc..33abc7713ed 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -45,6 +45,7 @@ declare_args() { # ........dart2js.dart.snapshot # ........dart2wasm_product.snapshot (if not on ia32) # ........dartdev.dart.snapshot +# ........dartdev.dill # ........dartdevc.dart.snapshot # ........dds.dart.snapshot # ........frontend_server.dart.snapshot @@ -454,6 +455,17 @@ copy("copy_vm_dill_files") { [ "$root_out_dir/$dart_sdk_output/lib/_internal/{{source_file_part}}" ] } +copy("copy_dartdev_dill_files") { + visibility = [ ":create_common_sdk" ] + deps = [ + ":copy_libraries", + "../utils/dartdev:dartdev", + ] + sources = [ "$root_out_dir/dartdev.dill" ] + outputs = + [ "$root_out_dir/$dart_sdk_output/bin/snapshots/{{source_file_part}}" ] +} + copy("copy_dart2js_dill_files") { visibility = [ ":create_full_sdk" ] deps = [ @@ -753,6 +765,7 @@ group("create_common_sdk") { ":copy_analysis_summaries", ":copy_api_readme", ":copy_dart", + ":copy_dartdev_dill_files", ":copy_dartdoc_files", ":copy_headers", ":copy_libraries_dart", diff --git a/utils/dartdev/BUILD.gn b/utils/dartdev/BUILD.gn index 9e9dd6e73d4..e3818aaedf7 100644 --- a/utils/dartdev/BUILD.gn +++ b/utils/dartdev/BUILD.gn @@ -7,11 +7,28 @@ import("../application_snapshot.gni") group("dartdev") { public_deps = [ + ":copy_dartdev_kernel", ":copy_dartdev_snapshot", ":copy_prebuilt_devtools", ] } +copy("copy_dartdev_kernel") { + visibility = [ ":dartdev" ] + public_deps = [ ":generate_dartdev_kernel" ] + sources = [ "$root_gen_dir/dartdev.dill" ] + outputs = [ "$root_out_dir/dartdev.dill" ] +} + +application_snapshot("generate_dartdev_kernel") { + dart_snapshot_kind = "kernel" + main_dart = "../../pkg/dartdev/bin/dartdev.dart" + training_args = [] + deps = [ "../dds:dds" ] + vm_args = [ "--sound-null-safety" ] + output = "$root_gen_dir/dartdev.dill" +} + copy("copy_dartdev_snapshot") { visibility = [ ":dartdev" ] public_deps = [ ":generate_dartdev_snapshot" ]