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 <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Chris Evans <cmevans@google.com>

BUILD.gn

@@ -60,14 +60,11 @@ group("runtime") {
     ]
   }
 
-  # We do not support AOT on ia32 and should therefore not provide native
+  # This flag is set in runtime/runtime_args.gni
-  # snapshot tooling.
+  # The analyze_snapshot tool is only supported on 64 bit AOT builds running
-  if (dart_target_arch != "ia32" && dart_target_arch != "x86") {
+  # under linux and android platforms
-    if (dart_runtime_mode == "release") {
+  if (build_analyze_snapshot) {
-      deps += [ "runtime/bin:analyze_snapshot_product" ]
+    deps += [ "runtime/bin:analyze_snapshot($host_toolchain)" ]
-    } else {
-      deps += [ "runtime/bin:analyze_snapshot" ]
-    }
   }
 
   if (is_linux || is_android) {

pkg/test_runner/lib/src/build_configurations.dart

@@ -95,6 +95,19 @@ List<String> _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

runtime/bin/BUILD.gn

@@ -891,34 +891,25 @@ dart_executable("dart_precompiled_runtime_product") {
   extra_deps += [ ":elf_loader_product" ]
 }
 
-dart_executable("analyze_snapshot") {
+# 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_deps = [
-    "..:libdart_precompiled_runtime",
-    "../platform:libdart_platform_precompiled_runtime",
-  ]
 
-  extra_sources = [
+    if (use_product_mode) {
-    "analyze_snapshot.cc",
-    "builtin.cc",
-    "loader.cc",
-    "loader.h",
-  ]
-
-  if (dart_runtime_mode == "release") {
-    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",
       ]
+    } else {
+      extra_deps = [
+        "..:libdart_precompiled_runtime",
+        "../platform:libdart_platform_precompiled_runtime",
+      ]
+    }
 
     extra_sources = [
       "analyze_snapshot.cc",
@@ -927,7 +918,12 @@ dart_executable("analyze_snapshot_product") {
       "loader.h",
     ]
 
+    if (use_product_mode) {
       extra_deps += [ ":elf_loader_product" ]
+    } else {
+      extra_deps += [ ":elf_loader" ]
+    }
+  }
 }
 
 executable("process_test") {

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 = dart_target_arch == "x64"
-    create_kernel_service_snapshot = true
+
-  } else {
+  # The analyze_snapshot tool is only supported on 64 bit AOT builds running
-    create_kernel_service_snapshot = false
+  # under linux and android platforms
-  }
+  build_analyze_snapshot =
+      (is_linux || is_android) &&
+      (dart_target_arch == "x64" || dart_target_arch == "arm64")
 }

runtime/tests/vm/dart/analyze_snapshot_binary_test.dart

@@ -25,11 +25,7 @@ Future<void> testAOT(String dillPath,
     Expect.isFalse(disassemble, 'no use of disassembler in PRODUCT mode');
   }
 
-  final analyzeSnapshot = path.join(
+  final analyzeSnapshot = path.join(buildDir, 'analyze_snapshot');
-      buildDir,
-      bool.fromEnvironment('dart.vm.product')
-          ? 'analyze_snapshot_product'
-          : '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);
+    await Future.wait([
-    }
+      testAOT(aotDillPath, stripUtil: true),
+    ]);
 
-    // TODO(sstrickl): Currently we can't assemble for SIMARM64 on MacOSX.
+    // Dont test assembled snapshot for simulated platforms
-    // For example, the test runner still uses blobs for
+    if (!buildDir.endsWith("SIMARM64") && !buildDir.endsWith("SIMARM64C")) {
-    // dartkp-mac-*-simarm64. Change assembleSnapshot and remove this check
-    // when we can.
-    if (Platform.isMacOS && buildDir.endsWith('SIMARM64')) {
-      printSkip('assembly tests');
-      return;
-    }
       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),
       ]);
+    }
   });
 }
 

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<void> testAOT(String dillPath,
     Expect.isFalse(disassemble, 'no use of disassembler in PRODUCT mode');
   }
 
-  final analyzeSnapshot = path.join(
+  final analyzeSnapshot = path.join(buildDir, 'analyze_snapshot');
-      buildDir,
-      bool.fromEnvironment('dart.vm.product')
-          ? 'analyze_snapshot_product'
-          : '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<void> testAOT(String dillPath,
       final assemblyPath = path.join(tempDir, 'test.S');
 
       await run(genSnapshot, <String>[
+        '--no-sound-null-safety',
         '--snapshot-kind=app-aot-assembly',
         '--assembly=$assemblyPath',
         ...commonSnapshotArgs,
@@ -87,6 +83,7 @@ Future<void> testAOT(String dillPath,
       await assembleSnapshot(assemblyPath, snapshotPath);
     } else {
       await run(genSnapshot, <String>[
+        '--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',
+    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');
     await run(genKernel, <String>[
+      '--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);
+    await Future.wait([
-    }
+      testAOT(aotDillPath, stripUtil: true),
+    ]);
 
-    // TODO(sstrickl): Currently we can't assemble for SIMARM64 on MacOSX.
+    // Dont test assembled snapshot for simulated platforms
-    // For example, the test runner still uses blobs for
+    if (!buildDir.endsWith("SIMARM64") && !buildDir.endsWith("SIMARM64C")) {
-    // dartkp-mac-*-simarm64. Change assembleSnapshot and remove this check
-    // when we can.
-    if (Platform.isMacOS && buildDir.endsWith('SIMARM64')) {
-      printSkip('assembly tests');
-      return;
-    }
       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),
       ]);
+    }
   });
 }
 

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

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:

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'] = (
+        gn_args['analyze_snapshot_binary'] = ('exe.stripped/analyze_snapshot')
-            'exe.stripped/analyze_snapshot_product')
 
     # Setup the user-defined sysroot.
     if UseSysroot(args, gn_args):