From 8debbd07257622873dcbdfe83f7566ac9050d8ce Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Thu, 21 Oct 2021 19:39:15 +0000 Subject: [PATCH] [vm] Fix writing v8 snapshot profiles for core snapshots. Fixes a crash when SetObjectTypeAndName is called twice for the same object id with different types. Don't add Code objects to the profile for non-Code carrying snapshots, as references to the Code object are replaced with references to null. TEST=vm/dart{_2}/v8_snapshot_profile_writer_test Bug: https://github.com/dart-lang/sdk/issues/47463 Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try Change-Id: Id9abf66509d422551ad5797744265eeb2c2ba2b5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217643 Reviewed-by: Alexander Markov Commit-Queue: Tess Strickland --- .../dart/v8_snapshot_profile_writer_test.dart | 29 ++++++++++++------- .../v8_snapshot_profile_writer_test.dart | 29 ++++++++++++------- runtime/vm/app_snapshot.cc | 2 +- runtime/vm/v8_snapshot_writer.cc | 2 +- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart b/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart index 9b74eca4e62..526c4492f6a 100644 --- a/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart +++ b/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart @@ -59,8 +59,9 @@ Snapshot testProfile(String profilePath) { return profile; } -Future testJIT(String dillPath) async { - final description = 'jit'; +Future testJIT(String dillPath, String snapshotKind) async { + final includesCode = snapshotKind == 'core-jit'; + final description = snapshotKind; Expect.isTrue(_seenDescriptions.add(description), "test configuration $description would be run multiple times"); @@ -73,9 +74,11 @@ Future testJIT(String dillPath) async { final isolateDataPath = path.join(tempDir, 'isolate_data.bin'); await run(genSnapshot, [ - '--snapshot-kind=core-jit', - '--vm_snapshot_instructions=$vmTextPath', - '--isolate_snapshot_instructions=$isolateTextPath', + '--snapshot-kind=$snapshotKind', + if (includesCode) ...[ + '--vm_snapshot_instructions=$vmTextPath', + '--isolate_snapshot_instructions=$isolateTextPath', + ], '--vm_snapshot_data=$vmDataPath', '--isolate_snapshot_data=$isolateDataPath', "--write-v8-snapshot-profile-to=$profilePath", @@ -89,10 +92,12 @@ Future testJIT(String dillPath) async { // Verify that the total size of the snapshot text and data sections is // the same as the sum of the shallow sizes of all objects in the profile. // This ensures that all bytes are accounted for in some way. - final actualSize = await File(vmTextPath).length() + - await File(isolateTextPath).length() + - await File(vmDataPath).length() + - await File(isolateDataPath).length(); + int actualSize = + await File(vmDataPath).length() + await File(isolateDataPath).length(); + if (includesCode) { + actualSize += await File(vmTextPath).length() + + await File(isolateTextPath).length(); + } final expectedSize = profile.nodes.fold(0, (size, n) => size + n.selfSize); @@ -426,8 +431,10 @@ main() async { // extra information that needs stripping), so no need to specify // stripUtil for useAsm tests. - // Test profile generation with a core JIT snapshot. - await testJIT(jitDillPath); + // Test profile generation with a core snapshot (no code). + await testJIT(jitDillPath, 'core'); + // Test profile generation with a core JIT snapshot (with code). + await testJIT(jitDillPath, 'core-jit'); // Test unstripped ELF generation directly. await testAOT(aotDillPath); diff --git a/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart b/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart index 37057d8709d..a4d23af7742 100644 --- a/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart +++ b/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart @@ -61,8 +61,9 @@ Snapshot testProfile(String profilePath) { return profile; } -Future testJIT(String dillPath) async { - final description = 'jit'; +Future testJIT(String dillPath, String snapshotKind) async { + final includesCode = snapshotKind == 'core-jit'; + final description = snapshotKind; Expect.isTrue(_seenDescriptions.add(description), "test configuration $description would be run multiple times"); @@ -75,9 +76,11 @@ Future testJIT(String dillPath) async { final isolateDataPath = path.join(tempDir, 'isolate_data.bin'); await run(genSnapshot, [ - '--snapshot-kind=core-jit', - '--vm_snapshot_instructions=$vmTextPath', - '--isolate_snapshot_instructions=$isolateTextPath', + '--snapshot-kind=$snapshotKind', + if (includesCode) ...[ + '--vm_snapshot_instructions=$vmTextPath', + '--isolate_snapshot_instructions=$isolateTextPath', + ], '--vm_snapshot_data=$vmDataPath', '--isolate_snapshot_data=$isolateDataPath', "--write-v8-snapshot-profile-to=$profilePath", @@ -91,10 +94,12 @@ Future testJIT(String dillPath) async { // Verify that the total size of the snapshot text and data sections is // the same as the sum of the shallow sizes of all objects in the profile. // This ensures that all bytes are accounted for in some way. - final actualSize = await File(vmTextPath).length() + - await File(isolateTextPath).length() + - await File(vmDataPath).length() + - await File(isolateDataPath).length(); + int actualSize = + await File(vmDataPath).length() + await File(isolateDataPath).length(); + if (includesCode) { + actualSize += await File(vmTextPath).length() + + await File(isolateTextPath).length(); + } final expectedSize = profile.nodes.fold(0, (size, n) => size + n.selfSize); @@ -420,8 +425,10 @@ main() async { // extra information that needs stripping), so no need to specify // stripUtil for useAsm tests. - // Test profile generation with a core JIT snapshot. - await testJIT(jitDillPath); + // Test profile generation with a core snapshot (no code). + await testJIT(jitDillPath, 'core'); + // Test profile generation with a core JIT snapshot (with code). + await testJIT(jitDillPath, 'core-jit'); // Test unstripped ELF generation directly. await testAOT(aotDillPath); diff --git a/runtime/vm/app_snapshot.cc b/runtime/vm/app_snapshot.cc index e25f0bc2de0..8f2e5abeb9e 100644 --- a/runtime/vm/app_snapshot.cc +++ b/runtime/vm/app_snapshot.cc @@ -5527,7 +5527,7 @@ class VMSerializationRoots : public SerializationRoots { if (!Snapshot::IncludesCode(s->kind())) { for (intptr_t i = 0; i < StubCode::NumEntries(); i++) { - s->AddBaseObject(StubCode::EntryAt(i).ptr(), "Code", ""); + s->AddBaseObject(StubCode::EntryAt(i).ptr()); } } } diff --git a/runtime/vm/v8_snapshot_writer.cc b/runtime/vm/v8_snapshot_writer.cc index d8861221752..1a224df4bd4 100644 --- a/runtime/vm/v8_snapshot_writer.cc +++ b/runtime/vm/v8_snapshot_writer.cc @@ -42,7 +42,7 @@ void V8SnapshotProfileWriter::SetObjectTypeAndName(const ObjectId& object_id, const intptr_t type_index = node_types_.Add(type); if (info->type != kInvalidString && info->type != type_index) { FATAL("Attempting to assign mismatching type %s to node %s", type, - info->ToCString(nullptr, zone_)); + info->ToCString(this, zone_)); } info->type = type_index; // Don't overwrite any existing name.