Use assembly instead of C array literals to link the core snapshot into the VM.

Allows for including instructions.

R=zra@google.com

Review-Url: https://codereview.chromium.org/2901163002 .

build/toolchain/win/BUILD.gn

@@ -112,9 +112,14 @@ template("msvc_toolchain") {
     }
 
     tool("asm") {
-      # TODO(brettw): "/safeseh" assembler argument is hardcoded here. Extract
-      # assembler flags to a variable like cflags. crbug.com/418613
-      command = "$python_path gyp-win-tool asm-wrapper $env ml.exe {{defines}} {{include_dirs}} {{asmflags}} /safeseh /c /Fo {{output}} {{source}}"
+      if (invoker.current_cpu == "x64") {
+        ml = "ml64.exe"
+        x64 = "-D_ML64_X64"
+      } else {
+        ml = "ml.exe"
+        x64 = ""
+      }
+      command = "$python_path gyp-win-tool asm-wrapper $env $ml $x64 {{defines}} {{include_dirs}} {{asmflags}} /c /Fo {{output}} {{source}}"
       description = "ASM {{output}}"
       outputs = [
         "{{target_out_dir}}/{{target_output_name}}/{{source_name_part}}.obj",

runtime/bin/BUILD.gn

@@ -489,43 +489,90 @@ action("generate_snapshot_bin") {
   ]
 }
 
-action("generate_snapshot_file") {
+# Generates an assembly file defining a given symbol with the bytes from a
+# binary file. Places the symbol in a text section if 'executable' is true,
+# otherwise places the symbol in a read-only data section.
+template("bin_to_assembly") {
+  assert(defined(invoker.deps), "Must define deps")
+  assert(defined(invoker.input), "Must define input binary file")
+  assert(defined(invoker.output), "Must define output assembly file")
+  assert(defined(invoker.symbol), "Must define symbol name")
+  assert(defined(invoker.executable), "Must define boolean executable")
+
+  action(target_name) {
+    deps = invoker.deps
+    script = "../tools/bin_to_assembly.py"
+    args = [
+      "--input",
+      rebase_path(invoker.input),
+      "--output",
+      rebase_path(invoker.output),
+      "--symbol_name",
+      invoker.symbol,
+      "--target_os",
+      target_os,
+    ]
+    if (invoker.executable) {
+      args += [ "--executable" ]
+    }
+    inputs = [
+      script,
+      invoker.input,
+    ]
+    outputs = [
+      invoker.output,
+    ]
+  }
+}
+
+bin_to_assembly("vm_snapshot_data_assembly") {
   deps = [
     ":generate_snapshot_bin",
   ]
+  input = "$target_gen_dir/vm_isolate_snapshot.bin"
+  output = "$target_gen_dir/vm_snapshot_data.S"
+  symbol = "kDartVmSnapshotData"
+  executable = false
+}
 
-  snapshot_in_cc_file = "snapshot_in.cc"
-  inputs = [
-    "../tools/create_snapshot_file.py",
-    snapshot_in_cc_file,
-    "$target_gen_dir/vm_isolate_snapshot.bin",
-    "$target_gen_dir/isolate_snapshot.bin",
-  ]
-  output = "$root_gen_dir/dart_snapshot.cc"
-  outputs = [
-    output,
-  ]
+bin_to_assembly("vm_snapshot_instructions_assembly") {
+  deps = []
+  input = "../tools/empty.bin"
+  output = "$target_gen_dir/vm_snapshot_instructions.S"
+  symbol = "kDartVmSnapshotInstructions"
+  executable = true
+}
 
-  script = "../tools/create_snapshot_file.py"
-  args = [
-    "--vm_input_bin",
-    rebase_path("$target_gen_dir/vm_isolate_snapshot.bin"),
-    "--input_bin",
-    rebase_path("$target_gen_dir/isolate_snapshot.bin"),
-    "--input_cc",
-    rebase_path(snapshot_in_cc_file),
-    "--output",
-    rebase_path(output),
+bin_to_assembly("isolate_snapshot_data_assembly") {
+  deps = [
+    ":generate_snapshot_bin",
   ]
+  input = "$target_gen_dir/isolate_snapshot.bin"
+  output = "$target_gen_dir/isolate_snapshot_data.S"
+  symbol = "kDartCoreIsolateSnapshotData"
+  executable = false
+}
+
+bin_to_assembly("isolate_snapshot_instructions_assembly") {
+  deps = []
+  input = "../tools/empty.bin"
+  output = "$target_gen_dir/isolate_snapshot_instructions.S"
+  symbol = "kDartCoreIsolateSnapshotInstructions"
+  executable = true
 }
 
 source_set("dart_snapshot_cc") {
-  sources = [
-    "$root_gen_dir/dart_snapshot.cc",
-  ]
-
   deps = [
-    ":generate_snapshot_file",
+    ":isolate_snapshot_data_assembly",
+    ":isolate_snapshot_instructions_assembly",
+    ":vm_snapshot_data_assembly",
+    ":vm_snapshot_instructions_assembly",
+  ]
+  sources = [
+    "$target_gen_dir/isolate_snapshot_data.S",
+    "$target_gen_dir/isolate_snapshot_instructions.S",
+    "$target_gen_dir/vm_snapshot_data.S",
+    "$target_gen_dir/vm_snapshot_instructions.S",
   ]
 }
 

runtime/bin/bin.gypi

@@ -1328,6 +1328,8 @@
       ],
       'sources': [
         'run_vm_tests.cc',
+        'error_exit.cc',
+        'error_exit.h',
         'builtin_common.cc',
         'builtin_natives.cc',
         'builtin_nolib.cc',
@@ -1335,6 +1337,8 @@
         'io_natives.h',
         'loader.cc',
         'loader.h',
+        'snapshot_utils.cc',
+        'snapshot_utils.h',
         # Include generated source files.
         '<(snapshot_cc_file)',
         '<(builtin_cc_file)',
@@ -1353,8 +1357,10 @@
       'sources/': [
         ['exclude', '\\.(cc|h)$'],
         ['include', 'run_vm_tests.cc'],
+        ['include', 'error_exit.cc'],
         ['include', 'builtin_nolib.cc'],
         ['include', 'builtin_natives.cc'],
+        ['include', 'snapshot_utils.cc'],
         ['include', '_gen\\.cc$'],
         ['include', '_test\\.(cc|h)$'],
       ],

runtime/bin/main.cc

@@ -36,14 +36,28 @@
 
 #include "vm/kernel.h"
 
+extern "C" {
+extern const uint8_t kDartVmSnapshotData[];
+extern const uint8_t kDartVmSnapshotInstructions[];
+extern const uint8_t kDartCoreIsolateSnapshotData[];
+extern const uint8_t kDartCoreIsolateSnapshotInstructions[];
+}
+
 namespace dart {
 namespace bin {
 
 // Snapshot pieces if we link in a snapshot, otherwise initialized to NULL.
-extern const uint8_t* vm_snapshot_data;
-extern const uint8_t* vm_snapshot_instructions;
-extern const uint8_t* core_isolate_snapshot_data;
-extern const uint8_t* core_isolate_snapshot_instructions;
+#if defined(DART_NO_SNAPSHOT)
+const uint8_t* vm_snapshot_data = NULL;
+const uint8_t* vm_snapshot_instructions = NULL;
+const uint8_t* core_isolate_snapshot_data = NULL;
+const uint8_t* core_isolate_snapshot_instructions = NULL;
+#else
+const uint8_t* vm_snapshot_data = kDartVmSnapshotData;
+const uint8_t* vm_snapshot_instructions = kDartVmSnapshotInstructions;
+const uint8_t* core_isolate_snapshot_data = kDartCoreIsolateSnapshotData;
+const uint8_t* core_isolate_snapshot_instructions = kDartVmSnapshotInstructions;
+#endif
 
 /**
  * Global state used to control and store generation of application snapshots

runtime/bin/run_vm_tests.cc

@@ -14,6 +14,12 @@
 #include "vm/dart.h"
 #include "vm/unit_test.h"
 
+extern "C" {
+extern const uint8_t kDartVmSnapshotData[];
+extern const uint8_t kDartVmSnapshotInstructions[];
+extern const uint8_t kDartCoreIsolateSnapshotData[];
+extern const uint8_t kDartCoreIsolateSnapshotInstructions[];
+}
 
 // TODO(iposva, asiva): This is a placeholder for the real unittest framework.
 namespace dart {
@@ -21,10 +27,16 @@ namespace dart {
 // Defined in vm/os_thread_win.cc
 extern bool private_flag_windows_run_tls_destructors;
 
-// vm_snapshot_data_buffer points to a snapshot for the vm isolate if we
-// link in a snapshot otherwise it is initialized to NULL.
-extern const uint8_t* bin::vm_snapshot_data;
-extern const uint8_t* bin::vm_snapshot_instructions;
+// Snapshot pieces when we link in a snapshot.
+#if defined(DART_NO_SNAPSHOT)
+#error "run_vm_tests must be built with a snapshot"
+#else
+const uint8_t* bin::vm_snapshot_data = kDartVmSnapshotData;
+const uint8_t* bin::vm_snapshot_instructions = kDartVmSnapshotInstructions;
+const uint8_t* bin::core_isolate_snapshot_data = kDartCoreIsolateSnapshotData;
+const uint8_t* bin::core_isolate_snapshot_instructions =
+    kDartCoreIsolateSnapshotInstructions;
+#endif
 
 // Only run tests that match the filter string. The default does not match any
 // tests.

runtime/bin/snapshot_empty.cc

@@ -13,13 +13,9 @@ typedef unsigned __int8 uint8_t;
 #endif
 #include <stddef.h>
 
-namespace dart {
-namespace bin {
-
-const uint8_t* vm_snapshot_data = NULL;
-const uint8_t* vm_snapshot_instructions = NULL;
-const uint8_t* core_isolate_snapshot_data = NULL;
-const uint8_t* core_isolate_snapshot_instructions = NULL;
-
-}  // namespace bin
-}  // namespace dart
+extern "C" {
+const uint8_t* kDartVmSnapshotData = NULL;
+const uint8_t* kDartVmSnapshotInstructions = NULL;
+const uint8_t* kDartCoreIsolateSnapshotData = NULL;
+const uint8_t* kDartCoreIsolateSnapshotInstructions = NULL;
+}

runtime/bin/snapshot_in.cc

@@ -15,29 +15,24 @@ typedef unsigned __int8 uint8_t;
 #endif
 #include <stddef.h>
 
-namespace dart {
-namespace bin {
+extern "C" {
 
 // The string on the next line will be filled in with the contents of the
 // generated snapshot binary file for the vm isolate.
 // This string forms the content of a vm isolate snapshot which is loaded
 // into the vm isolate.
-static const uint8_t vm_snapshot_data_[] = {
+uint8_t kDartVmSnapshotData[] = {
   %s
 };
-const uint8_t* vm_snapshot_data = vm_snapshot_data_;
-const uint8_t* vm_snapshot_instructions = NULL;
-
+uint8_t kDartVmSnapshotInstructions[] = {};
 
 // The string on the next line will be filled in with the contents of the
 // generated snapshot binary file for a regular dart isolate.
 // This string forms the content of a regular dart isolate snapshot which is
 // loaded into an isolate when it is created.
-static const uint8_t core_isolate_snapshot_data_[] = {
+uint8_t kDartCoreIsolateSnapshotData[] = {
   %s
 };
-const uint8_t* core_isolate_snapshot_data = core_isolate_snapshot_data_;
-const uint8_t* core_isolate_snapshot_instructions = NULL;
+uint8_t kDartCoreIsolateSnapshotInstructions[] = {};
 
-}  // namespace bin
-}  // namespace dart
+}

runtime/tools/bin_to_assembly.py

@@ -0,0 +1,94 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2017, 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.
+
+# Generates an assembly source file the defines a symbol with the bytes from
+# a given file.
+
+import os
+import sys
+from optparse import OptionParser
+
+def Main():
+  parser = OptionParser()
+  parser.add_option("--output",
+                    action="store", type="string",
+                    help="output assembly file name")
+  parser.add_option("--input",
+                    action="store", type="string",
+                    help="input binary blob file")
+  parser.add_option("--symbol_name",
+                    action="store", type="string")
+  parser.add_option("--executable",
+                    action="store_true", default=False)
+  parser.add_option("--target_os",
+                    action="store", type="string")
+
+  (options, args) = parser.parse_args()
+  if not options.output:
+    sys.stderr.write("--output not specified\n")
+    parser.print_help();
+    return -1
+  if not options.input:
+    sys.stderr.write("--input not specified\n")
+    parser.print_help();
+    return -1
+  if not os.path.isfile(options.input):
+    sys.stderr.write("input file does not exist: %s\n" % options.input)
+    parser.print_help();
+    return -1
+  if not options.symbol_name:
+    sys.stderr.write("--symbol_name not specified\n")
+    parser.print_help();
+    return -1
+  if not options.target_os:
+    sys.stderr.write("--target_os not specified\n")
+    parser.print_help();
+    return -1
+
+  with open(options.output, "w") as output_file:
+    if options.target_os in ["mac", "ios"]:
+      if options.executable:
+        output_file.write(".text\n")
+      else:
+        output_file.write(".const\n")
+      output_file.write(".global _%s\n" % options.symbol_name)
+      output_file.write(".balign 32\n")
+      output_file.write("_%s:\n" % options.symbol_name)
+    elif options.target_os in ["linux", "android", "fuchsia"]:
+      if options.executable:
+        output_file.write(".text\n")
+      else:
+        output_file.write(".section .rodata\n")
+      output_file.write(".global %s\n" % options.symbol_name)
+      output_file.write(".balign 32\n")
+      output_file.write("%s:\n" % options.symbol_name)
+    elif options.target_os in ["win"]:
+      output_file.write("ifndef _ML64_X64\n")
+      output_file.write(".model flat, C\n")
+      output_file.write("endif\n")
+      if options.executable:
+        output_file.write(".code\n")
+      else:
+        output_file.write(".const\n")
+      output_file.write("public %s\n" % options.symbol_name)
+      output_file.write("%s label byte\n" % options.symbol_name)
+    else:
+      sys.stderr.write("Unknown target_os: %s" % options.target_os)
+      return -1
+
+    with open(options.input, "rb") as input_file:
+      if options.target_os in ["win"]:
+        for byte in input_file.read():
+          output_file.write("byte %d\n" % ord(byte))
+        output_file.write("end\n")
+      else:
+        for byte in input_file.read():
+          output_file.write(".byte %d\n" % ord(byte))
+
+  return 0
+
+if __name__ == "__main__":
+  sys.exit(Main())

runtime/vm/clustered_snapshot.cc

@@ -4958,8 +4958,7 @@ Deserializer::Deserializer(Thread* thread,
       clusters_(NULL) {
   if (Snapshot::IncludesCode(kind)) {
     ASSERT(instructions_buffer != NULL);
-  }
-  if (instructions_buffer != NULL) {
+    ASSERT(data_buffer != NULL);
     image_reader_ = new (zone_) ImageReader(instructions_buffer, data_buffer);
   }
 }

tools/gyp/configurations_make.gypi

@@ -14,7 +14,6 @@
           '_FORTIFY_SOURCE=2',
         ],
         'cflags': [
-          '-Werror',
           '<@(common_gcc_warning_flags)',
           '-Wnon-virtual-dtor',
           '-Wvla',