VM: [Kernel] Fix bootstraping when Kernel isolate is used.

runtime/bin/main.cc

Index: runtime/bin/main.cc
diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc
index 384931c55556f578d567363adf67a4f59b870767..9c3ddc300128d6f53e86caefaa3187fe03c13d92 100644
--- a/runtime/bin/main.cc
+++ b/runtime/bin/main.cc
@@ -127,6 +127,8 @@ static int vm_service_server_port = -1;
 // checks are disabled.
 static bool vm_service_dev_mode = false;
 
+// Exit code indicating an internal Dart Frontend error.
+static const int kDartFrontendErrorExitCode = 252;
 // Exit code indicating an API error.
 static const int kApiErrorExitCode = 253;
 // Exit code indicating a compilation error.
@@ -821,7 +823,9 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
                                                 char** error,
                                                 int* exit_code) {
   ASSERT(script_uri != NULL);
-  if (strcmp(script_uri, DART_KERNEL_ISOLATE_NAME) == 0) {
+  const bool is_kernel_isolate =
+      strcmp(script_uri, DART_KERNEL_ISOLATE_NAME) == 0;
+  if (is_kernel_isolate) {
     if (!use_dart_frontend) {
       *error = strdup("Kernel isolate not supported.");
       return NULL;
@@ -849,13 +853,35 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
   }
 #endif
 
-  // If the script is a Kernel binary, then we will try to bootstrap from the
-  // script.
   const uint8_t* kernel_file = NULL;
   intptr_t kernel_length = -1;
-  const bool is_kernel =
-      !isolate_run_app_snapshot &&
-      TryReadKernel(script_uri, &kernel_file, &kernel_length);
+  bool is_kernel = false;
+
+  if (use_dart_frontend && !is_kernel_isolate && !is_service_isolate) {
+    Dart_KernelCompilationResult result = Dart_CompileToKernel(script_uri);

[hausner, 2017/01/24 00:50:55]

Calling the front end here only works because it returns a fully linked Kernel
binary, containing the entire core library. Correct?

We want the Dart/Kernel front end to return an incremental Kernel binary
eventually, i.e. a binary that does not contain the core libraries (only
references to the names used in the script). The core libraries will then have
to be loaded from a snapshot again, possibly a new snapshot format which
contains Kernel binary blobs.
 

[Vyacheslav Egorov (Google), 2017/01/30 19:34:12]

Yep, your understanding of the current situation is correct. We are slowly
moving towards application and core split.

+    *error = result.error;  // Copy error message (if any).
+    switch (result.status) {
+      case Dart_KernelCompilationStatus_Ok:
+        is_kernel = true;
+        kernel_file = result.kernel;
+        kernel_length = result.kernel_size;
+        break;
+      case Dart_KernelCompilationStatus_Error:
+        *exit_code = kCompilationErrorExitCode;
+        return NULL;
+      case Dart_KernelCompilationStatus_Crash:
+        *exit_code = kDartFrontendErrorExitCode;
+        return NULL;
+      case Dart_KernelCompilationStatus_Unknown:

[siva, 2017/01/24 02:38:02]

This return code seems to happen only when the kernel isolate itself doesn't
start up. Why not set this up as
*exit_code = kErrorExitCode;


(otherwise exit code ends up being 0 in this case)

[Vyacheslav Egorov (Google), 2017/01/30 19:34:13]

Done.

+        return NULL;
+    }
+  }
+
+  // If the script is a Kernel binary, then we will try to bootstrap from the
+  // script.
+  if (!is_kernel && !isolate_run_app_snapshot) {
+    is_kernel = TryReadKernel(script_uri, &kernel_file, &kernel_length);
+  }
 
   void* kernel_program = NULL;
   if (is_kernel) {
@@ -944,6 +970,8 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
       *error = strdup("Error while initializing Kernel isolate");
       return NULL;
     }
+
+    Loader::InitForSnapshot(script_uri);

[hausner, 2017/01/24 00:50:55]

Why is this call needed here? This seems to be app snapshot specific. Maybe the
name of the function should change if it does something that is necessary when
otherwise the loader is not invoked?

[Vyacheslav Egorov (Google), 2017/01/30 19:34:13]

Some parts of core-libraries (e.g. Isolate methods) query loader state to
resolve packages mapping, package relative uris, etc, so we need loader in
initialized state.

I am adding a comment. 

   }
 
   if (isolate_run_app_snapshot) {