1. Figure out the modified libraries from the specifeid kernel file during

   reload instead of computing it based on the source modification information.

2. Fix for failure in Unit test case #30322

3. Fix crash in test that was being skipped in #30109 and turn that test on.

This CL is work towards completing issue #28051

[email protected]

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

Author: a-siva

pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart

@@ -136,7 +136,7 @@ main() {
     await hotReload();
     await programIsDone;
     expect(await lines[2], "part1 part4");
-  }, skip: true /* VM crashes on reload */);
+  });
 
   test('reload after whole program modification', () async {
     await startProgram(1);

runtime/bin/dfe.cc

@@ -52,7 +52,8 @@ void DFE::SetKernelBinaries(const char* name) {
            File::PathSeparator(), kVMServiceIOBinaryName);
 }
 
-Dart_Handle DFE::ReloadScript(Dart_Isolate isolate, const char* url_string) {
+Dart_Handle DFE::ReadKernelBinary(Dart_Isolate isolate,
+                                  const char* url_string) {
   ASSERT(!Dart_IsServiceIsolate(isolate) && !Dart_IsKernelIsolate(isolate));
   // First check if the URL points to a Kernel IR file in which case we
   // skip the compilation step and directly reload the file.
@@ -75,17 +76,7 @@ Dart_Handle DFE::ReloadScript(Dart_Isolate isolate, const char* url_string) {
   }
   void* kernel_program = Dart_ReadKernelBinary(kernel_ir, kernel_ir_size);
   ASSERT(kernel_program != NULL);
-  Dart_Handle result = Dart_LoadKernel(kernel_program);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  // Finalize loading. This will complete any futures for completed deferred
-  // loads.
-  result = Dart_FinalizeLoading(true);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  return Dart_Null();
+  return Dart_NewExternalTypedData(Dart_TypedData_kUint64, kernel_program, 1);
 }
 
 void* DFE::CompileAndReadScript(const char* script_uri,

runtime/bin/dfe.h

@@ -38,11 +38,11 @@ class DFE {
   bool kernel_file_specified() const { return kernel_file_specified_; }
   void set_kernel_file_specified(bool value) { kernel_file_specified_ = value; }
 
-  // Method to reload a script into a running a isolate.
+  // Method to read a kernel file into a kernel program blob.
   // If the specified script [url] is not a kernel IR, compile it first using
-  // DFE and then reload the resulting kernel IR into the isolate.
+  // DFE and then read the resulting kernel file into a kernel program blob.
   // Returns Dart_Null if successful, otherwise an error object is returned.
-  Dart_Handle ReloadScript(Dart_Isolate isolate, const char* url_string);
+  Dart_Handle ReadKernelBinary(Dart_Isolate isolate, const char* url_string);
 
   // Compiles a script and reads the resulting kernel file.
   // If the compilation is successful, returns a valid in memory kernel

runtime/bin/loader.cc

@@ -673,20 +673,14 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
   if (Dart_IsError(result)) {
     return result;
   }
-  if (tag == Dart_kScriptTag) {
-    if (dfe.UseDartFrontend() || dfe.kernel_file_specified()) {
-      Dart_Isolate current = Dart_CurrentIsolate();
-      // Check if we are trying to reload a kernel file or if the '--dfe'
-      // option was specified and we need to compile sources using DFE.
-      if (!Dart_IsServiceIsolate(current) && !Dart_IsKernelIsolate(current)) {
-        // When using DFE the library tag handler should be called only when
-        // we are reloading scripts.
-        return dfe.ReloadScript(current, url_string);
-      }
-    }
-    // TODO(asiva) We need to ensure that the kernel and service isolates
-    // are always loaded from a kernel IR and do not use this path.
-  } else {
+  if (tag == Dart_kKernelTag) {
+    ASSERT(dfe.UseDartFrontend() || dfe.kernel_file_specified());
+    Dart_Isolate current = Dart_CurrentIsolate();
+    ASSERT(!Dart_IsServiceIsolate(current) && !Dart_IsKernelIsolate(current));
+    return dfe.ReadKernelBinary(current, url_string);
+  }
+  ASSERT(!dfe.UseDartFrontend() && !dfe.kernel_file_specified());
+  if (tag != Dart_kScriptTag) {
     // Special case for handling dart: imports and parts.
     // Grab the library's url.
     Dart_Handle library_url = Dart_LibraryUrl(library);
@@ -707,7 +701,6 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
                                         url_string);
     }
   }
-
   if (DartUtils::IsDartExtensionSchemeURL(url_string)) {
     // Handle early error cases for dart-ext: imports.
     if (tag != Dart_kImportTag) {

runtime/include/dart_api.h

@@ -2686,6 +2686,7 @@ typedef enum {
   Dart_kScriptTag,
   Dart_kSourceTag,
   Dart_kImportTag,
+  Dart_kKernelTag,
 } Dart_LibraryTag;
 
 /**
@@ -2725,6 +2726,15 @@ typedef enum {
  * "import" directive.  Once the script is loaded, the embedder should
  * call Dart_LoadLibrary to provide the script source to the VM.  The
  * return value should be an error or null.
+ *
+ * Dart_kKernelTag
+ *
+ * This tag is used to load the intermediate file (kernel) generated by
+ * the Dart front end. This tag is typically used when a 'hot-reload'
+ * of an application is needed and the VM is 'use dart front end' mode.
+ * The dart front end typically compiles all the scripts, imports and part
+ * files into one intermediate file hence we don't use the source/import or
+ * script tags.
  */
 typedef Dart_Handle (*Dart_LibraryTagHandler)(
     Dart_LibraryTag tag,

runtime/tests/vm/vm.status

@@ -327,7 +327,6 @@ cc/IsolateReload_EnumToNotEnum: Skip
 cc/IsolateReload_ExportedLibModified: Crash
 cc/IsolateReload_ImportedLibModified: Crash
 cc/IsolateReload_ImportedMixinFunction: Crash
-cc/IsolateReload_KernelIncrementalCompileGenerics: Crash # Issue 30322
 cc/IsolateReload_LibraryHide: Crash
 cc/IsolateReload_LibraryLookup: Fail
 cc/IsolateReload_LibraryShow: Crash

runtime/vm/isolate_reload.cc

@@ -10,6 +10,7 @@
 #include "vm/dart_api_impl.h"
 #include "vm/hash_table.h"
 #include "vm/isolate.h"
+#include "vm/kernel_reader.h"
 #include "vm/log.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
@@ -485,6 +486,7 @@ class Aborted : public ReasonForCancelling {
   }
 };
 
+#if !defined(DART_PRECOMPILED_RUNTIME)
 static intptr_t CommonSuffixLength(const char* a, const char* b) {
   const intptr_t a_length = strlen(a);
   const intptr_t b_length = strlen(b);
@@ -533,8 +535,57 @@ void IsolateReloadContext::Reload(bool force_reload,
                           old_root_lib_url.Length() - common_suffix_length + 1);
   }
 
-  // Check to see which libraries have been modified.
-  modified_libs_ = FindModifiedLibraries(force_reload);
+  Object& result = Object::Handle(thread->zone());
+  kernel::Program* kernel_program = NULL;
+  String& packages_url = String::Handle();
+  if (packages_url_ != NULL) {
+    packages_url = String::New(packages_url_);
+  }
+  if (isolate()->use_dart_frontend()) {
+    // Load the kernel program and figure out the modified libraries.
+    const GrowableObjectArray& libs =
+        GrowableObjectArray::Handle(object_store()->libraries());
+    intptr_t num_libs = libs.Length();
+    modified_libs_ = new (Z) BitVector(Z, num_libs);
+    TIR_Print("---- ENTERING TAG HANDLER\n");
+    {
+      TransitionVMToNative transition(thread);
+      Api::Scope api_scope(thread);
+      Dart_Handle retval = (I->library_tag_handler())(
+          Dart_kKernelTag, Api::NewHandle(thread, packages_url.raw()),
+          Api::NewHandle(thread, root_lib_url.raw()));
+      if (Dart_IsError(retval)) {
+        // Compilation of the new sources failed, abort reload and report
+        // error.
+        result = Api::UnwrapHandle(retval);
+      } else {
+        uint64_t data;
+        intptr_t data_len = 0;
+        Dart_TypedData_Type data_type;
+        ASSERT(Dart_IsTypedData(retval));
+        Dart_Handle val = Dart_TypedDataAcquireData(
+            retval, &data_type, reinterpret_cast<void**>(&data), &data_len);
+        ASSERT(!Dart_IsError(val));
+        ASSERT(data_type == Dart_TypedData_kUint64);
+        ASSERT(data_len == 1);
+        kernel_program = reinterpret_cast<kernel::Program*>(data);
+        Dart_TypedDataReleaseData(retval);
+        kernel::KernelReader reader(kernel_program);
+        reader.FindModifiedLibraries(I, modified_libs_, force_reload);
+      }
+    }
+    if (result.IsError()) {
+      TIR_Print("---- LOAD FAILED, ABORTING RELOAD\n");
+      AddReasonForCancelling(new Aborted(zone_, ApiError::Cast(result)));
+      ReportReasonsForCancelling();
+      CommonFinalizeTail();
+      return;
+    }
+    TIR_Print("---- EXITED TAG HANDLER\n");
+  } else {
+    // Check to see which libraries have been modified.
+    modified_libs_ = FindModifiedLibraries(force_reload);
+  }
   if (!modified_libs_->Contains(old_root_lib.index())) {
     ASSERT(modified_libs_->IsEmpty());
     reload_skipped_ = true;
@@ -589,26 +640,33 @@ void IsolateReloadContext::Reload(bool force_reload,
   // returned by the tag handler. The tag handler can return other errors,
   // for example, top level parse errors. We want to capture these errors while
   // propagating the UnwindError or an UnhandledException error.
-  Object& result = Object::Handle(thread->zone());
 
-  String& packages_url = String::Handle();
-  if (packages_url_ != NULL) {
-    packages_url = String::New(packages_url_);
-  }
-
-  TIR_Print("---- ENTERING TAG HANDLER\n");
-  {
+  if (isolate()->use_dart_frontend()) {
+    // Read the kernel program.
+    kernel::KernelReader reader(kernel_program);
+    const Object& tmp = reader.ReadProgram();
+    if (!tmp.IsError()) {
+      Library& lib = Library::Handle(thread->zone());
+      lib ^= tmp.raw();
+      isolate()->object_store()->set_root_library(lib);
+      FinalizeLoading();
+      result = Object::null();
+    } else {
+      result = tmp.raw();
+    }
+  } else {
+    TIR_Print("---- ENTERING TAG HANDLER\n");
     TransitionVMToNative transition(thread);
     Api::Scope api_scope(thread);
 
     Dart_Handle retval = (I->library_tag_handler())(
         Dart_kScriptTag, Api::NewHandle(thread, packages_url.raw()),
         Api::NewHandle(thread, root_lib_url.raw()));
     result = Api::UnwrapHandle(retval);
+    TIR_Print("---- EXITED TAG HANDLER\n");
   }
   //
   // WEIRD CONTROL FLOW ENDS.
-  TIR_Print("---- EXITED TAG HANDLER\n");
 
   // Re-enable the background compiler. Do this before propagating any errors.
   BackgroundCompiler::Enable();
@@ -633,6 +691,12 @@ void IsolateReloadContext::Reload(bool force_reload,
     FinalizeFailedLoad(Error::Cast(result));
   }
 }
+#else
+// NOTE: This function returns *after* FinalizeLoading is called.
+void IsolateReloadContext::Reload(bool force_reload,
+                                  const char* root_script_url,
+                                  const char* packages_url_) {}
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
 void IsolateReloadContext::RegisterClass(const Class& new_cls) {
   const Class& old_cls = Class::Handle(OldClassOrNull(new_cls));
@@ -663,10 +727,9 @@ void IsolateReloadContext::RegisterClass(const Class& new_cls) {
 // FinalizeLoading will be called *before* Reload() returns but will not be
 // called if the embedder fails to load sources.
 void IsolateReloadContext::FinalizeLoading() {
-  if (reload_skipped_) {
+  if (reload_skipped_ || reload_finalized_) {
     return;
   }
-  ASSERT(!reload_finalized_);
   BuildLibraryMapping();
   TIR_Print("---- LOAD SUCCEEDED\n");
   if (ValidateReload()) {

runtime/vm/kernel_reader.cc

@@ -211,6 +211,43 @@ Object& KernelReader::ReadProgram() {
   return error;
 }
 
+void KernelReader::FindModifiedLibraries(Isolate* isolate,
+                                         BitVector* modified_libs,
+                                         bool force_reload) {
+  LongJumpScope jump;
+  if (setjmp(*jump.Set()) == 0) {
+    if (force_reload) {
+      // If a reload is being forced we mark all libraries as having
+      // been modified.
+      const GrowableObjectArray& libs =
+          GrowableObjectArray::Handle(isolate->object_store()->libraries());
+      intptr_t num_libs = libs.Length();
+      Library& lib = dart::Library::Handle(Z);
+      for (intptr_t i = 0; i < num_libs; i++) {
+        lib ^= libs.At(i);
+        if (!lib.is_dart_scheme()) {
+          modified_libs->Add(lib.index());
+        }
+      }
+      return;
+    }
+    // Now go through all the libraries that are present in the incremental
+    // kernel files, these will constitute the modified libraries.
+    intptr_t length = program_->library_count();
+    for (intptr_t i = 0; i < length; i++) {
+      intptr_t kernel_offset = library_offset(i);
+      builder_.SetOffset(kernel_offset);
+      LibraryHelper library_helper(&builder_);
+      library_helper.ReadUntilIncluding(LibraryHelper::kCanonicalName);
+      dart::Library& lib = LookupLibrary(library_helper.canonical_name_);
+      if (!lib.IsNull() && !lib.is_dart_scheme()) {
+        // This is a library that already exists so mark it as being modified.
+        modified_libs->Add(lib.index());
+      }
+    }
+  }
+}
+
 void KernelReader::ReadLibrary(intptr_t kernel_offset) {
   builder_.SetOffset(kernel_offset);
   LibraryHelper library_helper(&builder_);

runtime/vm/kernel_reader.h

@@ -60,6 +60,12 @@ class KernelReader {
   // was no main procedure, or a failure object if there was an error.
   Object& ReadProgram();
 
+  // Finds all libraries that have been modified in this incremental
+  // version of the kernel program file.
+  void FindModifiedLibraries(Isolate* isolate,
+                             BitVector* modified_libs,
+                             bool force_reload);
+
   void ReadLibrary(intptr_t kernel_offset);
 
   const String& DartSymbol(StringIndex index) {

runtime/vm/unit_test.cc

@@ -215,6 +215,7 @@ static Dart_Handle LibraryTagHandler(Dart_LibraryTag tag,
   if (FLAG_use_dart_frontend) {
     // Reload request.
 
+    ASSERT(tag == Dart_kKernelTag);
     const char* urlstr = NULL;
     Dart_Handle result = Dart_StringToCString(url, &urlstr);
     if (Dart_IsError(result)) {
@@ -238,11 +239,10 @@ static Dart_Handle LibraryTagHandler(Dart_LibraryTag tag,
       ASSERT(kernel_reload_key != kUnsetThreadLocalKey);
       kernel_pgm =
           reinterpret_cast<void*>(OSThread::GetThreadLocal(kernel_reload_key));
-      ASSERT(kernel_pgm != NULL);
       OSThread::SetThreadLocal(kernel_reload_key, 0);
     }
-    return Dart_LoadScript(url, Dart_Null(),
-                           reinterpret_cast<Dart_Handle>(kernel_pgm), 0, 0);
+    ASSERT(kernel_pgm != NULL);
+    return Dart_NewExternalTypedData(Dart_TypedData_kUint64, kernel_pgm, 1);
   }
   if (tag == Dart_kCanonicalizeUrl) {
     Dart_Handle library_url = Dart_LibraryUrl(library);