From c09f790d371c206f4a8fc10ec19c2766fc5c2e57 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Mon, 12 Dec 2022 18:02:47 +0000 Subject: [PATCH] [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 --- 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, 16 insertions(+), 128 deletions(-) delete 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 deleted file mode 100644 index 1c4542bffe1..00000000000 --- a/pkg/dartdev/test/load_from_dill_test.dart +++ /dev/null @@ -1,29 +0,0 @@ -// 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 1406ef3ebb2..f54c7380790 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::TryResolveArtifactPath( - const char* filename) { +Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() { + const char* snapshot_filename = "dartdev.dart.snapshot"; // |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(), filename); + Utils::SCreate("%ssnapshots/%s", dir_prefix.get(), snapshot_filename); if (File::Exists(nullptr, snapshot_path)) { return Utils::CreateCStringUniquePtr(snapshot_path); } @@ -74,7 +74,7 @@ Utils::CStringUniquePtr DartDevIsolate::TryResolveArtifactPath( // 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(), filename); + snapshot_path = Utils::SCreate("%s%s", dir_prefix.get(), snapshot_filename); if (File::Exists(nullptr, snapshot_path)) { return Utils::CreateCStringUniquePtr(snapshot_path); } @@ -82,14 +82,6 @@ Utils::CStringUniquePtr DartDevIsolate::TryResolveArtifactPath( 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 0d7f74ae539..4992427e8d7 100644 --- a/runtime/bin/dartdev_isolate.h +++ b/runtime/bin/dartdev_isolate.h @@ -46,9 +46,6 @@ 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(); @@ -99,8 +96,6 @@ 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 04deaf5ccaa..84564b1c99a 100644 --- a/runtime/bin/main.cc +++ b/runtime/bin/main.cc @@ -581,75 +581,35 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri, char** error, int* exit_code) { int64_t start = Dart_TimelineGetMicros(); - auto dartdev_path = DartDevIsolate::TryResolveDartDevSnapshotPath(); - - 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 (dartdev_path.get() == nullptr) { + Syslog::PrintErr( + "Failed to start the Dart CLI isolate. Could not resolve DartDev " + "snapshot.\n"); + return nullptr; + } if (error != nullptr) { *error = nullptr; } + Dart_Isolate isolate = nullptr; AppSnapshot* app_snapshot = nullptr; - bool isolate_run_app_snapshot = true; + const 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 = NULL; - const uint8_t* isolate_snapshot_instructions = NULL; + const uint8_t* isolate_snapshot_data = nullptr; + const uint8_t* isolate_snapshot_instructions = nullptr; 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); - isolate_group_data = + IsolateGroupData* isolate_group_data = new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config, app_snapshot, isolate_run_app_snapshot); - 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); + IsolateData* 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 33abc7713ed..f62bad80bbc 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -45,7 +45,6 @@ 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 @@ -455,17 +454,6 @@ 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 = [ @@ -765,7 +753,6 @@ 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 e3818aaedf7..9e9dd6e73d4 100644 --- a/utils/dartdev/BUILD.gn +++ b/utils/dartdev/BUILD.gn @@ -7,28 +7,11 @@ 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" ]