From d77fff73075426306d2c3d77ccf61afd3eba9ab1 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Thu, 8 Oct 2020 19:59:15 +0000 Subject: [PATCH] [vm/nnbd] Add separate Snapshot::Kind for core snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core snapshots should be agnostic to the sound null safety mode (so they can be used both in weak and strong modes), and snapshot writer verifies that. Snapshot::kFull was previously used both for core snapshots and app snapshots on ia32. However, app snapshots are not guaranteed to be agnostic, which appeared as failures on a few test on ia32. Also, VM should be able to detect null safety mode from app snapshots, even if they do not contain code, but null safety mode was not written into features string of kFull snapshots. In order to disambiguate core snapshots, a new Snapshot::Kind is added. Snapshot::kFullCore works exactly as Snapshot::kFull, except for verification of agnostic null safety and snapshot features string omitting null safety mode. All snapshots except kFullCore now have null safety mode included into their features string. Fixes https://github.com/dart-lang/sdk/issues/43626 Issue https://github.com/dart-lang/sdk/issues/43613 Change-Id: I8cd3b049ef4e428dd5e1ce666d4c7aa3b596d70c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166308 Reviewed-by: Régis Crelier Commit-Queue: Alexander Markov --- runtime/bin/gen_snapshot.cc | 5 +++-- runtime/bin/snapshot_utils.cc | 4 ++-- runtime/include/dart_api.h | 5 ++++- runtime/vm/benchmark_test.cc | 8 ++++---- runtime/vm/clustered_snapshot.cc | 9 +++++---- runtime/vm/dart.cc | 6 ++++-- runtime/vm/dart_api_impl.cc | 7 +++++-- runtime/vm/object_store.h | 1 + runtime/vm/raw_object.h | 8 ++++++++ runtime/vm/snapshot.cc | 2 ++ runtime/vm/snapshot.h | 20 ++++++++++++-------- runtime/vm/snapshot_test.cc | 3 ++- 12 files changed, 52 insertions(+), 26 deletions(-) diff --git a/runtime/bin/gen_snapshot.cc b/runtime/bin/gen_snapshot.cc index 187cf925e7c..eba26b589a8 100644 --- a/runtime/bin/gen_snapshot.cc +++ b/runtime/bin/gen_snapshot.cc @@ -446,7 +446,8 @@ static void CreateAndWriteCoreSnapshot() { // First create a snapshot. result = Dart_CreateSnapshot(&vm_snapshot_data_buffer, &vm_snapshot_data_size, &isolate_snapshot_data_buffer, - &isolate_snapshot_data_size); + &isolate_snapshot_data_size, + /*is_core=*/true); CHECK_RESULT(result); // Now write the vm isolate and isolate snapshots out to the @@ -539,7 +540,7 @@ static void CreateAndWriteAppSnapshot() { intptr_t isolate_snapshot_data_size = 0; result = Dart_CreateSnapshot(NULL, NULL, &isolate_snapshot_data_buffer, - &isolate_snapshot_data_size); + &isolate_snapshot_data_size, /*is_core=*/false); CHECK_RESULT(result); WriteFile(isolate_snapshot_data_filename, isolate_snapshot_data_buffer, diff --git a/runtime/bin/snapshot_utils.cc b/runtime/bin/snapshot_utils.cc index d31be2d21ec..4a19ba20e9b 100644 --- a/runtime/bin/snapshot_utils.cc +++ b/runtime/bin/snapshot_utils.cc @@ -489,8 +489,8 @@ void Snapshot::GenerateAppJIT(const char* snapshot_filename) { uint8_t* isolate_buffer = NULL; intptr_t isolate_size = 0; - Dart_Handle result = - Dart_CreateSnapshot(NULL, NULL, &isolate_buffer, &isolate_size); + Dart_Handle result = Dart_CreateSnapshot(NULL, NULL, &isolate_buffer, + &isolate_size, /*is_core=*/false); if (Dart_IsError(result)) { ErrorExit(kErrorExitCode, "%s\n", Dart_GetError(result)); } diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index 5268f5d62e6..5d7d117489e 100644 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -1242,6 +1242,8 @@ DART_EXPORT void Dart_ExitIsolate(); * snapshot. This buffer is scope allocated and is only valid * until the next call to Dart_ExitScope. * \param size Returns the size of the buffer. + * \param is_core Create a snapshot containing core libraries. + * Such snapshot should be agnostic to null safety mode. * * \return A valid handle if no error occurs during the operation. */ @@ -1249,7 +1251,8 @@ DART_EXPORT DART_WARN_UNUSED_RESULT Dart_Handle Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, intptr_t* vm_snapshot_data_size, uint8_t** isolate_snapshot_data_buffer, - intptr_t* isolate_snapshot_data_size); + intptr_t* isolate_snapshot_data_size, + bool is_core); /** * Returns whether the buffer contains a kernel file. diff --git a/runtime/vm/benchmark_test.cc b/runtime/vm/benchmark_test.cc index 2215c7d8baa..f7b12663486 100644 --- a/runtime/vm/benchmark_test.cc +++ b/runtime/vm/benchmark_test.cc @@ -528,12 +528,12 @@ BENCHMARK_SIZE(CoreSnapshotSize) { MallocWriteStream vm_snapshot_data(FullSnapshotWriter::kInitialSize); MallocWriteStream isolate_snapshot_data(FullSnapshotWriter::kInitialSize); FullSnapshotWriter writer( - Snapshot::kFull, &vm_snapshot_data, &isolate_snapshot_data, + Snapshot::kFullCore, &vm_snapshot_data, &isolate_snapshot_data, /*vm_image_writer=*/nullptr, /*iso_image_writer=*/nullptr); writer.WriteFullSnapshot(); const Snapshot* snapshot = Snapshot::SetupFromBuffer(isolate_snapshot_data.buffer()); - ASSERT(snapshot->kind() == Snapshot::kFull); + ASSERT(snapshot->kind() == Snapshot::kFullCore); benchmark->set_score(snapshot->length()); } @@ -566,12 +566,12 @@ BENCHMARK_SIZE(StandaloneSnapshotSize) { MallocWriteStream vm_snapshot_data(FullSnapshotWriter::kInitialSize); MallocWriteStream isolate_snapshot_data(FullSnapshotWriter::kInitialSize); FullSnapshotWriter writer( - Snapshot::kFull, &vm_snapshot_data, &isolate_snapshot_data, + Snapshot::kFullCore, &vm_snapshot_data, &isolate_snapshot_data, /*vm_image_writer=*/nullptr, /*iso_image_writer=*/nullptr); writer.WriteFullSnapshot(); const Snapshot* snapshot = Snapshot::SetupFromBuffer(isolate_snapshot_data.buffer()); - ASSERT(snapshot->kind() == Snapshot::kFull); + ASSERT(snapshot->kind() == Snapshot::kFullCore); benchmark->set_score(snapshot->length()); } diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index 7fb5ecabdb3..27d263a5765 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -245,7 +245,8 @@ class ClassSerializationCluster : public SerializationCluster { s->UnexpectedObject(cls, "Class with illegal cid"); } s->WriteCid(class_id); - if (s->kind() == Snapshot::kFull && RequireLegacyErasureOfConstants(cls)) { + if (s->kind() == Snapshot::kFullCore && + RequireLegacyErasureOfConstants(cls)) { s->UnexpectedObject(cls, "Class with non mode agnostic constants"); } if (s->kind() != Snapshot::kFullAOT) { @@ -596,7 +597,7 @@ class FunctionSerializationCluster : public SerializationCluster { objects_.Add(func); PushFromTo(func); - if (kind == Snapshot::kFull) { + if ((kind == Snapshot::kFull) || (kind == Snapshot::kFullCore)) { NOT_IN_PRECOMPILED(s->Push(func->ptr()->bytecode_)); } else if (kind == Snapshot::kFullAOT) { s->Push(func->ptr()->code_); @@ -625,7 +626,7 @@ class FunctionSerializationCluster : public SerializationCluster { FunctionPtr func = objects_[i]; AutoTraceObjectName(func, MakeDisambiguatedFunctionName(s, func)); WriteFromTo(func); - if (kind == Snapshot::kFull) { + if ((kind == Snapshot::kFull) || (kind == Snapshot::kFullCore)) { NOT_IN_PRECOMPILED(WriteField(func, bytecode_)); } else if (kind == Snapshot::kFullAOT) { WriteField(func, code_); @@ -692,7 +693,7 @@ class FunctionDeserializationCluster : public DeserializationCluster { Function::InstanceSize()); ReadFromTo(func); - if (kind == Snapshot::kFull) { + if ((kind == Snapshot::kFull) || (kind == Snapshot::kFullCore)) { NOT_IN_PRECOMPILED(func->ptr()->bytecode_ = static_cast(d->ReadRef())); } else if (kind == Snapshot::kFullAOT) { diff --git a/runtime/vm/dart.cc b/runtime/vm/dart.cc index f271e57e2d4..dac8c1839b9 100644 --- a/runtime/vm/dart.cc +++ b/runtime/vm/dart.cc @@ -782,12 +782,12 @@ bool Dart::DetectNullSafety(const char* script_uri, // when generating the snapshot. ASSERT(FLAG_sound_null_safety == kNullSafetyOptionUnspecified); - // If snapshot is an appJIT/AOT snapshot we will figure out the mode by + // If snapshot is not a core snapshot we will figure out the mode by // sniffing the feature string in the snapshot. if (snapshot_data != nullptr) { // Read the snapshot and check for null safety option. const Snapshot* snapshot = Snapshot::SetupFromBuffer(snapshot_data); - if (Snapshot::IncludesCode(snapshot->kind())) { + if (!Snapshot::IsAgnosticToNullSafety(snapshot->kind())) { return SnapshotHeaderReader::NullSafetyFromSnapshot(snapshot); } } @@ -1034,7 +1034,9 @@ const char* Dart::FeaturesString(Isolate* isolate, #else #error What architecture? #endif + } + if (!Snapshot::IsAgnosticToNullSafety(kind)) { if (isolate != NULL) { if (isolate->null_safety()) { buffer.AddString(" null-safety"); diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 291f8f711e3..6fdb1975da9 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1934,7 +1934,8 @@ DART_EXPORT Dart_Handle Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, intptr_t* vm_snapshot_data_size, uint8_t** isolate_snapshot_data_buffer, - intptr_t* isolate_snapshot_data_size) { + intptr_t* isolate_snapshot_data_size, + bool is_core) { #if defined(DART_PRECOMPILED_RUNTIME) return Api::NewError("Cannot create snapshots on an AOT runtime."); #else @@ -1966,8 +1967,10 @@ Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, FullSnapshotWriter::kInitialSize); ZoneWriteStream isolate_snapshot_data(Api::TopScope(T)->zone(), FullSnapshotWriter::kInitialSize); + const Snapshot::Kind snapshot_kind = + is_core ? Snapshot::kFullCore : Snapshot::kFull; FullSnapshotWriter writer( - Snapshot::kFull, &vm_snapshot_data, &isolate_snapshot_data, + snapshot_kind, &vm_snapshot_data, &isolate_snapshot_data, nullptr /* vm_image_writer */, nullptr /* isolate_image_writer */); writer.WriteFullSnapshot(); if (vm_snapshot_data_buffer != nullptr) { diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h index 05f290c5343..fa649a5db30 100644 --- a/runtime/vm/object_store.h +++ b/runtime/vm/object_store.h @@ -479,6 +479,7 @@ class ObjectStore { ObjectPtr* to_snapshot(Snapshot::Kind kind) { switch (kind) { case Snapshot::kFull: + case Snapshot::kFullCore: return reinterpret_cast(&global_object_pool_); case Snapshot::kFullJIT: case Snapshot::kFullAOT: diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index 9dd7f8f3550..5f03f711464 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -762,6 +762,7 @@ class ClassLayout : public ObjectLayout { case Snapshot::kFullAOT: return reinterpret_cast(&allocation_stub_); case Snapshot::kFull: + case Snapshot::kFullCore: return reinterpret_cast(&direct_subclasses_); case Snapshot::kFullJIT: return reinterpret_cast(&dependent_code_); @@ -836,6 +837,7 @@ class PatchClassLayout : public ObjectLayout { case Snapshot::kFullAOT: return reinterpret_cast(&script_); case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return reinterpret_cast(&library_kernel_data_); case Snapshot::kMessage: @@ -1026,6 +1028,7 @@ class FunctionLayout : public ObjectLayout { switch (kind) { case Snapshot::kFullAOT: case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return reinterpret_cast(&data_); case Snapshot::kMessage: @@ -1194,6 +1197,7 @@ class FieldLayout : public ObjectLayout { ObjectPtr* to_snapshot(Snapshot::Kind kind) { switch (kind) { case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: case Snapshot::kFullAOT: return reinterpret_cast(&initializer_function_); @@ -1270,6 +1274,7 @@ class ScriptLayout : public ObjectLayout { case Snapshot::kFullAOT: return reinterpret_cast(&url_); case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return reinterpret_cast(&kernel_program_info_); case Snapshot::kMessage: @@ -1343,6 +1348,7 @@ class LibraryLayout : public ObjectLayout { case Snapshot::kFullAOT: return reinterpret_cast(&exports_); case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return reinterpret_cast(&kernel_data_); case Snapshot::kMessage: @@ -2067,6 +2073,7 @@ class ICDataLayout : public CallSiteDataLayout { case Snapshot::kFullAOT: return reinterpret_cast(&entries_); case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return to(); case Snapshot::kMessage: @@ -2174,6 +2181,7 @@ class LibraryPrefixLayout : public InstanceLayout { case Snapshot::kFullAOT: return reinterpret_cast(&imports_); case Snapshot::kFull: + case Snapshot::kFullCore: case Snapshot::kFullJIT: return reinterpret_cast(&importer_); case Snapshot::kMessage: diff --git a/runtime/vm/snapshot.cc b/runtime/vm/snapshot.cc index 9403472227b..5c0d911c15f 100644 --- a/runtime/vm/snapshot.cc +++ b/runtime/vm/snapshot.cc @@ -229,6 +229,8 @@ const char* Snapshot::KindToCString(Kind kind) { switch (kind) { case kFull: return "full"; + case kFullCore: + return "full-core"; case kFullJIT: return "full-jit"; case kFullAOT: diff --git a/runtime/vm/snapshot.h b/runtime/vm/snapshot.h index d685fb2cca5..94aa3e7e272 100644 --- a/runtime/vm/snapshot.h +++ b/runtime/vm/snapshot.h @@ -91,11 +91,12 @@ enum SerializeState { class Snapshot { public: enum Kind { - kFull, // Full snapshot of core libraries or an application. - kFullJIT, // Full + JIT code - kFullAOT, // Full + AOT code - kMessage, // A partial snapshot used only for isolate messaging. - kNone, // gen_snapshot + kFull, // Full snapshot of an application. + kFullCore, // Full snapshot of core libraries. Agnostic to null safety. + kFullJIT, // Full + JIT code + kFullAOT, // Full + AOT code + kMessage, // A partial snapshot used only for isolate messaging. + kNone, // gen_snapshot kInvalid }; static const char* KindToCString(Kind kind); @@ -130,13 +131,15 @@ class Snapshot { void set_kind(Kind value) { return Write(kKindOffset, value); } static bool IsFull(Kind kind) { - return (kind == kFull) || (kind == kFullJIT) || (kind == kFullAOT); + return (kind == kFull) || (kind == kFullCore) || (kind == kFullJIT) || + (kind == kFullAOT); } + static bool IsAgnosticToNullSafety(Kind kind) { return (kind == kFullCore); } static bool IncludesCode(Kind kind) { return (kind == kFullJIT) || (kind == kFullAOT); } static bool IncludesBytecode(Kind kind) { - return (kind == kFull) || (kind == kFullJIT); + return (kind == kFull) || (kind == kFullCore) || (kind == kFullJIT); } const uint8_t* Addr() const { return reinterpret_cast(this); } @@ -171,7 +174,8 @@ class Snapshot { inline static bool IsSnapshotCompatible(Snapshot::Kind vm_kind, Snapshot::Kind isolate_kind) { if (vm_kind == isolate_kind) return true; - if (vm_kind == Snapshot::kFull && isolate_kind == Snapshot::kFullJIT) + if (((vm_kind == Snapshot::kFull) || (vm_kind == Snapshot::kFullCore)) && + isolate_kind == Snapshot::kFullJIT) return true; return Snapshot::IsFull(isolate_kind); } diff --git a/runtime/vm/snapshot_test.cc b/runtime/vm/snapshot_test.cc index 4ff7ccfea71..ed0383c4981 100644 --- a/runtime/vm/snapshot_test.cc +++ b/runtime/vm/snapshot_test.cc @@ -2073,7 +2073,8 @@ VM_UNIT_TEST_CASE(LegacyErasureDetectionInFullSnapshot) { // Write snapshot with object content. MallocWriteStream isolate_snapshot_data(FullSnapshotWriter::kInitialSize); FullSnapshotWriter writer( - Snapshot::kFull, /*vm_snapshot_data=*/nullptr, &isolate_snapshot_data, + Snapshot::kFullCore, /*vm_snapshot_data=*/nullptr, + &isolate_snapshot_data, /*vm_image_writer=*/nullptr, /*iso_image_writer=*/nullptr); writer.WriteFullSnapshot(); }