From 0b8ab17feb1609de5ff5a0c9a743d239ca12cc2f Mon Sep 17 00:00:00 2001 From: Chris Evans Date: Tue, 24 Jan 2023 14:07:26 +0000 Subject: [PATCH] Fix analyze_snapshot tests and build configuration Updated strictness on build configuration for analyze_snapshot Will run only under Linux/Android 64 bit arm, arm64c, simarm64, simarm64c and x64 architectures. analyze_snapshot just has a single target that links into product precompiled programatically Testing suite updated to comply with null-safety requirements and fix for simulated platform failing to use gen_snapshot with assembly. TEST=ci Change-Id: I3d58400db2e26a441a40fe7197b22510a52732b6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279391 Reviewed-by: Slava Egorov Reviewed-by: Martin Kustermann Commit-Queue: Chris Evans --- BUILD.gn | 13 ++-- .../lib/src/build_configurations.dart | 13 ++++ runtime/bin/BUILD.gn | 64 +++++++++---------- runtime/runtime_args.gni | 12 ++-- .../vm/dart/analyze_snapshot_binary_test.dart | 48 +++++--------- .../dart_2/analyze_snapshot_binary_test.dart | 54 ++++++---------- runtime/tests/vm/vm.status | 11 ++-- sdk/BUILD.gn | 2 +- tools/gn.py | 3 +- 9 files changed, 97 insertions(+), 123 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index a25f379b0ff..380fc57909e 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -60,14 +60,11 @@ group("runtime") { ] } - # We do not support AOT on ia32 and should therefore not provide native - # snapshot tooling. - if (dart_target_arch != "ia32" && dart_target_arch != "x86") { - if (dart_runtime_mode == "release") { - deps += [ "runtime/bin:analyze_snapshot_product" ] - } else { - deps += [ "runtime/bin:analyze_snapshot" ] - } + # This flag is set in runtime/runtime_args.gni + # The analyze_snapshot tool is only supported on 64 bit AOT builds running + # under linux and android platforms + if (build_analyze_snapshot) { + deps += [ "runtime/bin:analyze_snapshot($host_toolchain)" ] } if (is_linux || is_android) { diff --git a/pkg/test_runner/lib/src/build_configurations.dart b/pkg/test_runner/lib/src/build_configurations.dart index 6ceccbcb81f..cefde157a4b 100644 --- a/pkg/test_runner/lib/src/build_configurations.dart +++ b/pkg/test_runner/lib/src/build_configurations.dart @@ -95,6 +95,19 @@ List _selectBuildTargets(Configuration inner) { .contains(inner.architecture)) { result.add('gen_snapshot'); } + if ([Mode.release, Mode.product].contains(inner.mode) && + [Compiler.dartkp, Compiler.dartk].contains(compiler) && + [ + Architecture.arm64, + Architecture.x64, + Architecture.arm64c, + Architecture.x64c, + Architecture.simarm64, + Architecture.simarm64c + ].contains(inner.architecture) && + [System.linux, System.android].contains(inner.system)) { + result.add('analyze_snapshot'); + } if (compiler == Compiler.dartdevk && !inner.useSdk) { result diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn index e15715fce4d..04547177e7c 100644 --- a/runtime/bin/BUILD.gn +++ b/runtime/bin/BUILD.gn @@ -891,45 +891,41 @@ dart_executable("dart_precompiled_runtime_product") { extra_deps += [ ":elf_loader_product" ] } -dart_executable("analyze_snapshot") { - extra_configs = [ "..:dart_precompiled_runtime_config" ] - extra_deps = [ - "..:libdart_precompiled_runtime", - "../platform:libdart_platform_precompiled_runtime", - ] +# This flag is set set in runtime/runtime_args.gni +# The analyze_snapshot tool is only supported on 64 bit AOT builds running under +# linux and android platforms +if (build_analyze_snapshot) { + dart_executable("analyze_snapshot") { + use_product_mode = dart_runtime_mode == "release" + extra_configs = [ "..:dart_precompiled_runtime_config" ] - extra_sources = [ - "analyze_snapshot.cc", - "builtin.cc", - "loader.cc", - "loader.h", - ] + if (use_product_mode) { + extra_deps = [ + "..:libdart_precompiled_runtime_product", + "../platform:libdart_platform_precompiled_runtime_product", + ] + } else { + extra_deps = [ + "..:libdart_precompiled_runtime", + "../platform:libdart_platform_precompiled_runtime", + ] + } - if (dart_runtime_mode == "release") { - extra_deps += [ ":elf_loader_product" ] - } else { - extra_deps += [ ":elf_loader" ] + extra_sources = [ + "analyze_snapshot.cc", + "builtin.cc", + "loader.cc", + "loader.h", + ] + + if (use_product_mode) { + extra_deps += [ ":elf_loader_product" ] + } else { + extra_deps += [ ":elf_loader" ] + } } } -dart_executable("analyze_snapshot_product") { - use_product_mode = true - extra_configs = [ "..:dart_precompiled_runtime_config" ] - extra_deps = [ - "..:libdart_precompiled_runtime_product", - "../platform:libdart_platform_precompiled_runtime_product", - ] - - extra_sources = [ - "analyze_snapshot.cc", - "builtin.cc", - "loader.cc", - "loader.h", - ] - - extra_deps += [ ":elf_loader_product" ] -} - executable("process_test") { sources = [ "process_test.cc" ] } diff --git a/runtime/runtime_args.gni b/runtime/runtime_args.gni index 8219c5140b9..0c4f1ea6f11 100644 --- a/runtime/runtime_args.gni +++ b/runtime/runtime_args.gni @@ -87,9 +87,11 @@ declare_args() { # We create a kernel service app-jit snapshot only for when the target # architecture is x64 for other cases we will use the '.dill' file # which is already linked in the VM. - if (dart_target_arch == "x64") { - create_kernel_service_snapshot = true - } else { - create_kernel_service_snapshot = false - } + create_kernel_service_snapshot = dart_target_arch == "x64" + + # The analyze_snapshot tool is only supported on 64 bit AOT builds running + # under linux and android platforms + build_analyze_snapshot = + (is_linux || is_android) && + (dart_target_arch == "x64" || dart_target_arch == "arm64") } diff --git a/runtime/tests/vm/dart/analyze_snapshot_binary_test.dart b/runtime/tests/vm/dart/analyze_snapshot_binary_test.dart index 52152d122b2..816f53262f6 100644 --- a/runtime/tests/vm/dart/analyze_snapshot_binary_test.dart +++ b/runtime/tests/vm/dart/analyze_snapshot_binary_test.dart @@ -25,11 +25,7 @@ Future testAOT(String dillPath, Expect.isFalse(disassemble, 'no use of disassembler in PRODUCT mode'); } - final analyzeSnapshot = path.join( - buildDir, - bool.fromEnvironment('dart.vm.product') - ? 'analyze_snapshot_product' - : 'analyze_snapshot'); + final analyzeSnapshot = path.join(buildDir, 'analyze_snapshot'); // For assembly, we can't test the sizes of the snapshot sections, since we // don't have a Mach-O reader for Mac snapshots and for ELF, the assembler @@ -143,7 +139,7 @@ main() async { await withTempDir('analyze_snapshot_binary', (String tempDir) async { // We only need to generate the dill file once for all JIT tests. final _thisTestPath = path.join(sdkDir, 'runtime', 'tests', 'vm', 'dart', - 'analyze_snapshot_binary_test.dart'); + 'use_save_debugging_info_flag_program.dart'); // We only need to generate the dill file once for all AOT tests. final aotDillPath = path.join(tempDir, 'aot_test.dill'); @@ -179,37 +175,23 @@ main() async { // they have lots of output and so the log will be truncated. if (!const bool.fromEnvironment('dart.vm.product')) { // Regression test for dartbug.com/41149. - await testAOT(aotDillPath, disassemble: true); + await Future.wait([testAOT(aotDillPath, disassemble: true)]); } - // We neither generate assembly nor have a stripping utility on Windows. - if (Platform.isWindows) { - printSkip('external stripping and assembly tests'); - return; - } - - // The native strip utility on Mac OS X doesn't recognize ELF files. - if (Platform.isMacOS && clangBuildToolsDir == null) { - printSkip('ELF external stripping test'); - } else { - // Test unstripped ELF generation that is then externally stripped. - await testAOT(aotDillPath, stripUtil: true); - } - - // TODO(sstrickl): Currently we can't assemble for SIMARM64 on MacOSX. - // For example, the test runner still uses blobs for - // dartkp-mac-*-simarm64. Change assembleSnapshot and remove this check - // when we can. - if (Platform.isMacOS && buildDir.endsWith('SIMARM64')) { - printSkip('assembly tests'); - return; - } + // Test unstripped ELF generation that is then externally stripped. await Future.wait([ - // Test unstripped assembly generation that is then externally stripped. - testAOT(aotDillPath, useAsm: true), - // Test stripped assembly generation that is then externally stripped. - testAOT(aotDillPath, useAsm: true, stripFlag: true), + testAOT(aotDillPath, stripUtil: true), ]); + + // Dont test assembled snapshot for simulated platforms + if (!buildDir.endsWith("SIMARM64") && !buildDir.endsWith("SIMARM64C")) { + await Future.wait([ + // Test unstripped assembly generation that is then externally stripped. + testAOT(aotDillPath, useAsm: true), + // Test stripped assembly generation that is then externally stripped. + testAOT(aotDillPath, useAsm: true, stripFlag: true), + ]); + } }); } diff --git a/runtime/tests/vm/dart_2/analyze_snapshot_binary_test.dart b/runtime/tests/vm/dart_2/analyze_snapshot_binary_test.dart index 7affeec1d6b..91aa6f57fec 100644 --- a/runtime/tests/vm/dart_2/analyze_snapshot_binary_test.dart +++ b/runtime/tests/vm/dart_2/analyze_snapshot_binary_test.dart @@ -1,7 +1,6 @@ // Copyright (c) 2018, 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. - // @dart = 2.9 import 'dart:convert'; @@ -27,11 +26,7 @@ Future testAOT(String dillPath, Expect.isFalse(disassemble, 'no use of disassembler in PRODUCT mode'); } - final analyzeSnapshot = path.join( - buildDir, - bool.fromEnvironment('dart.vm.product') - ? 'analyze_snapshot_product' - : 'analyze_snapshot'); + final analyzeSnapshot = path.join(buildDir, 'analyze_snapshot'); // For assembly, we can't test the sizes of the snapshot sections, since we // don't have a Mach-O reader for Mac snapshots and for ELF, the assembler @@ -79,6 +74,7 @@ Future testAOT(String dillPath, final assemblyPath = path.join(tempDir, 'test.S'); await run(genSnapshot, [ + '--no-sound-null-safety', '--snapshot-kind=app-aot-assembly', '--assembly=$assemblyPath', ...commonSnapshotArgs, @@ -87,6 +83,7 @@ Future testAOT(String dillPath, await assembleSnapshot(assemblyPath, snapshotPath); } else { await run(genSnapshot, [ + '--no-sound-null-safety', '--snapshot-kind=app-aot-elf', '--elf=$snapshotPath', ...commonSnapshotArgs, @@ -144,12 +141,13 @@ main() async { await withTempDir('analyze_snapshot_binary', (String tempDir) async { // We only need to generate the dill file once for all JIT tests. - final _thisTestPath = path.join(sdkDir, 'runtime', 'tests', 'vm', 'dart_2', - 'analyze_snapshot_binary_test.dart'); + final _thisTestPath = path.join(sdkDir, 'runtime', 'tests', 'vm', 'dart', + 'use_save_debugging_info_flag_program.dart'); // We only need to generate the dill file once for all AOT tests. final aotDillPath = path.join(tempDir, 'aot_test.dill'); await run(genKernel, [ + '--no-sound-null-safety', '--aot', '--platform', platformDill, @@ -177,37 +175,23 @@ main() async { // they have lots of output and so the log will be truncated. if (!const bool.fromEnvironment('dart.vm.product')) { // Regression test for dartbug.com/41149. - await testAOT(aotDillPath, disassemble: true); + await Future.wait([testAOT(aotDillPath, disassemble: true)]); } - // We neither generate assembly nor have a stripping utility on Windows. - if (Platform.isWindows) { - printSkip('external stripping and assembly tests'); - return; - } - - // The native strip utility on Mac OS X doesn't recognize ELF files. - if (Platform.isMacOS && clangBuildToolsDir == null) { - printSkip('ELF external stripping test'); - } else { - // Test unstripped ELF generation that is then externally stripped. - await testAOT(aotDillPath, stripUtil: true); - } - - // TODO(sstrickl): Currently we can't assemble for SIMARM64 on MacOSX. - // For example, the test runner still uses blobs for - // dartkp-mac-*-simarm64. Change assembleSnapshot and remove this check - // when we can. - if (Platform.isMacOS && buildDir.endsWith('SIMARM64')) { - printSkip('assembly tests'); - return; - } + // Test unstripped ELF generation that is then externally stripped. await Future.wait([ - // Test unstripped assembly generation that is then externally stripped. - testAOT(aotDillPath, useAsm: true), - // Test stripped assembly generation that is then externally stripped. - testAOT(aotDillPath, useAsm: true, stripFlag: true), + testAOT(aotDillPath, stripUtil: true), ]); + + // Dont test assembled snapshot for simulated platforms + if (!buildDir.endsWith("SIMARM64") && !buildDir.endsWith("SIMARM64C")) { + await Future.wait([ + // Test unstripped assembly generation that is then externally stripped. + testAOT(aotDillPath, useAsm: true), + // Test stripped assembly generation that is then externally stripped. + testAOT(aotDillPath, useAsm: true, stripFlag: true), + ]); + } }); } diff --git a/runtime/tests/vm/vm.status b/runtime/tests/vm/vm.status index 5b31fce69d1..8d1a2decff3 100644 --- a/runtime/tests/vm/vm.status +++ b/runtime/tests/vm/vm.status @@ -452,11 +452,6 @@ dart_2/causal_stacks/zone_callback_stack_traces_test: SkipByDesign # Asserts exa dart/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer. dart_2/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer. -# Currently this is only supported on 64-bit linux systems with precompilation -[ $compiler != dartk || $mode == debug || $runtime != dart_precompiled || $arch != arm64 && $arch != x64 || $system != android && $system != linux ] -dart/analyze_snapshot_binary_test: SkipByDesign # Only run on 64bit AOT on standard architectures -dart_2/analyze_snapshot_binary_test: SkipByDesign # Only run on 64bit AOT on standard architectures - [ $mode == debug || $runtime != dart_precompiled || $system == android ] dart/emit_aot_size_info_flag_test: SkipByDesign # This test is for VM AOT only and is quite slow (so we don't run it in debug mode). dart/split_aot_kernel_generation2_test: SkipByDesign # This test is for VM AOT only and is quite slow (so we don't run it in debug mode). @@ -512,3 +507,9 @@ dart_2/stack_overflow_shared_test: SkipSlow # Too slow with --shared-slow-path-t [ $hot_reload || $hot_reload_rollback || $compiler != dartk && $compiler != dartkp ] dart/entrypoints/*: SkipByDesign # These tests are for compiler optimizations and very sensitive to when functions are optimized, so they are disabled on hotreload and optcounter bots. dart_2/entrypoints/*: SkipByDesign # These tests are for compiler optimizations and very sensitive to when functions are optimized, so they are disabled on hotreload and optcounter bots. + +# The analyze_snapshot tool tests are only supported on 64 bit AOT builds running under +# release or product runtimes for linux and android platforms +[ $arch != arm64 && $arch != arm64c && $arch != simarm64 && $arch != simarm64c && $arch != x64 || $compiler != dartk && $compiler != dartkp || $mode != product && $mode != release || $runtime != dart_precompiled && $runtime != vm || $system != android && $system != linux ] +dart/analyze_snapshot_binary_test: SkipByDesign # Only run on 64bit AOT on standard architectures +dart_2/analyze_snapshot_binary_test: SkipByDesign # Only run on 64bit AOT on standard architectures diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 90bb16febaa..e81d00d13ca 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -26,7 +26,7 @@ declare_args() { dart_stripped_binary = "dart" dart_precompiled_runtime_stripped_binary = "dart_precompiled_runtime_product" gen_snapshot_stripped_binary = "gen_snapshot_product" - analyze_snapshot_binary = "analyze_snapshot_product" + analyze_snapshot_binary = "analyze_snapshot" } # The directory layout of the SDK is as follows: diff --git a/tools/gn.py b/tools/gn.py index 4f6e2efd13f..04085827b5c 100755 --- a/tools/gn.py +++ b/tools/gn.py @@ -297,8 +297,7 @@ def ToGnArgs(args, mode, arch, target_os, sanitizer, verify_sdk_hash): 'exe.stripped/dart_precompiled_runtime_product') gn_args['gen_snapshot_stripped_binary'] = ( 'exe.stripped/gen_snapshot_product') - gn_args['analyze_snapshot_binary'] = ( - 'exe.stripped/analyze_snapshot_product') + gn_args['analyze_snapshot_binary'] = ('exe.stripped/analyze_snapshot') # Setup the user-defined sysroot. if UseSysroot(args, gn_args):