[vm] Don't use ApiError to represent errors during startup.

With the removal of the VM isolate and less bootstrapping being repeated in the AOT runtime, ApiError cannot be instantiated until after snapshot loading succeeds.

TEST=ci
Change-Id: I5be295e87e6d6351ada40d2837825b1f93672ac7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501262
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

runtime/vm/app_snapshot.cc

@@ -685,9 +685,9 @@ class Deserializer : public ThreadStackResource {
 
   // Verifies the image alignment.
   //
-  // Returns ApiError::null() on success and an ApiError with an an appropriate
-  // message otherwise.
-  ApiErrorPtr VerifyImageAlignment();
+  // On success, returns nullptr. On failure, returns an error message that the
+  // caller must free.
+  char* VerifyImageAlignment();
 
   ObjectPtr Allocate(intptr_t size);
   static void InitializeHeader(ObjectPtr raw, intptr_t cid, intptr_t size) {
@@ -9720,11 +9720,11 @@ void Deserializer::ReadDispatchTable(
 #endif
 }
 
-ApiErrorPtr Deserializer::VerifyImageAlignment() {
+char* Deserializer::VerifyImageAlignment() {
   if (image_reader_ != nullptr) {
     return image_reader_->VerifyAlignment();
   }
-  return ApiError::null();
+  return nullptr;
 }
 
 void SnapshotHeaderReader::SetCoverageFromSnapshotFeatures(
@@ -9846,17 +9846,6 @@ char* SnapshotHeaderReader::BuildError(const char* message) {
   return Utils::StrDup(message);
 }
 
-ApiErrorPtr FullSnapshotReader::ConvertToApiError(char* message) {
-  // This can also fail while bringing up the VM isolate, so make sure to
-  // allocate the error message in old space.
-  const String& msg = String::Handle(String::New(message, Heap::kOld));
-
-  // The [message] was constructed with [BuildError] and needs to be freed.
-  free(message);
-
-  return ApiError::New(msg, Heap::kOld);
-}
-
 void Deserializer::ReadInstructions(CodePtr code, bool deferred) {
 #if defined(DART_PRECOMPILED_RUNTIME)
   if (deferred) {
@@ -10384,14 +10373,14 @@ char* SnapshotHeaderReader::InitializeGlobalVMFlagsFromSnapshot(
   return nullptr;
 }
 
-ApiErrorPtr FullSnapshotReader::ReadVMSnapshot() {
+char* FullSnapshotReader::ReadVMSnapshot() {
   SnapshotHeaderReader header_reader(kind_, buffer_, size_);
 
   intptr_t offset = 0;
   char* error = header_reader.VerifyVersionAndFeatures(
       /*isolate_group=*/nullptr, &offset);
   if (error != nullptr) {
-    return ConvertToApiError(error);
+    return error;
   }
 
   // Even though there's no concurrent threads we have to guard agains, some
@@ -10402,9 +10391,9 @@ ApiErrorPtr FullSnapshotReader::ReadVMSnapshot() {
   Deserializer deserializer(thread_, kind_, buffer_, size_, data_image_,
                             instructions_image_, /*is_non_root_unit=*/false,
                             offset);
-  ApiErrorPtr api_error = deserializer.VerifyImageAlignment();
-  if (api_error != ApiError::null()) {
-    return api_error;
+  error = deserializer.VerifyImageAlignment();
+  if (error != nullptr) {
+    return error;
   }
 
   if (Snapshot::IncludesCode(kind_)) {
@@ -10428,17 +10417,17 @@ ApiErrorPtr FullSnapshotReader::ReadVMSnapshot() {
   }
 #endif  // defined(DART_PRECOMPILED_RUNTIME)
 
-  return ApiError::null();
+  return nullptr;
 }
 
-ApiErrorPtr FullSnapshotReader::ReadProgramSnapshot() {
+char* FullSnapshotReader::ReadProgramSnapshot() {
   SnapshotHeaderReader header_reader(kind_, buffer_, size_);
   header_reader.SetCoverageFromSnapshotFeatures(thread_->isolate_group());
   intptr_t offset = 0;
   char* error =
       header_reader.VerifyVersionAndFeatures(thread_->isolate_group(), &offset);
   if (error != nullptr) {
-    return ConvertToApiError(error);
+    return error;
   }
 
   // Even though there's no concurrent threads we have to guard agains, some
@@ -10449,9 +10438,9 @@ ApiErrorPtr FullSnapshotReader::ReadProgramSnapshot() {
   Deserializer deserializer(thread_, kind_, buffer_, size_, data_image_,
                             instructions_image_, /*is_non_root_unit=*/false,
                             offset);
-  ApiErrorPtr api_error = deserializer.VerifyImageAlignment();
-  if (api_error != ApiError::null()) {
-    return api_error;
+  error = deserializer.VerifyImageAlignment();
+  if (error != nullptr) {
+    return error;
   }
 
   if (Snapshot::IncludesCode(kind_)) {
@@ -10482,24 +10471,24 @@ ApiErrorPtr FullSnapshotReader::ReadProgramSnapshot() {
 
   InitializeBSS();
 
-  return ApiError::null();
+  return nullptr;
 }
 
-ApiErrorPtr FullSnapshotReader::ReadUnitSnapshot(const LoadingUnit& unit) {
+char* FullSnapshotReader::ReadUnitSnapshot(const LoadingUnit& unit) {
   SnapshotHeaderReader header_reader(kind_, buffer_, size_);
   intptr_t offset = 0;
   char* error =
       header_reader.VerifyVersionAndFeatures(thread_->isolate_group(), &offset);
   if (error != nullptr) {
-    return ConvertToApiError(error);
+    return error;
   }
 
   Deserializer deserializer(
       thread_, kind_, buffer_, size_, data_image_, instructions_image_,
       /*is_non_root_unit=*/unit.id() != LoadingUnit::kRootId, offset);
-  ApiErrorPtr api_error = deserializer.VerifyImageAlignment();
-  if (api_error != ApiError::null()) {
-    return api_error;
+  error = deserializer.VerifyImageAlignment();
+  if (error != nullptr) {
+    return nullptr;
   }
   {
     Array& units =
@@ -10507,9 +10496,9 @@ ApiErrorPtr FullSnapshotReader::ReadUnitSnapshot(const LoadingUnit& unit) {
     uint32_t main_program_hash = Smi::Value(Smi::RawCast(units.At(0)));
     uint32_t unit_program_hash = deserializer.Read<uint32_t>();
     if (main_program_hash != unit_program_hash) {
-      return ApiError::New(String::Handle(
-          String::New("Deferred loading unit is from a different "
-                      "program than the main loading unit")));
+      return Utils::StrDup(
+          "Deferred loading unit is from a different "
+          "program than the main loading unit");
     }
   }
 
@@ -10528,7 +10517,7 @@ ApiErrorPtr FullSnapshotReader::ReadUnitSnapshot(const LoadingUnit& unit) {
 
   InitializeBSS();
 
-  return ApiError::null();
+  return nullptr;
 }
 
 void FullSnapshotReader::InitializeBSS() {

runtime/vm/app_snapshot.h

@@ -167,14 +167,15 @@ class FullSnapshotReader {
                      Thread* thread);
   ~FullSnapshotReader() {}
 
-  ApiErrorPtr ReadVMSnapshot();
-  ApiErrorPtr ReadProgramSnapshot();
-  ApiErrorPtr ReadUnitSnapshot(const LoadingUnit& unit);
+  // On success, returns nullptr. On failure, returns an error message that the
+  // caller must free.
+  char* ReadVMSnapshot();
+  char* ReadProgramSnapshot();
+  char* ReadUnitSnapshot(const LoadingUnit& unit);
 
  private:
   IsolateGroup* isolate_group() const { return thread_->isolate_group(); }
 
-  ApiErrorPtr ConvertToApiError(char* message);
   void InitializeBSS();
 
   Snapshot::Kind kind_;

runtime/vm/dart.cc

@@ -442,10 +442,9 @@ char* Dart::DartInit(const Dart_InitializeParams* params) {
         return Utils::StrDup("Invalid vm isolate snapshot seen");
       }
       FullSnapshotReader reader(snapshot, params->vm_snapshot_instructions, T);
-      const Error& error = Error::Handle(reader.ReadVMSnapshot());
-      if (!error.IsNull()) {
-        // Must copy before leaving the zone.
-        return Utils::StrDup(error.ToErrorCString());
+      char* error = reader.ReadVMSnapshot();
+      if (error != nullptr) {
+        return error;
       }
 
       Object::FinishInit(vm_isolate_->group());
@@ -849,17 +848,16 @@ Isolate* Dart::CreateIsolate(const char* name_prefix,
   return isolate;
 }
 
-ErrorPtr Dart::InitIsolateGroupFromSnapshot(
-    Thread* T,
-    const uint8_t* snapshot_data,
-    const uint8_t* snapshot_instructions,
-    const uint8_t* kernel_buffer,
-    intptr_t kernel_buffer_size) {
+char* Dart::InitIsolateGroupFromSnapshot(Thread* T,
+                                         const uint8_t* snapshot_data,
+                                         const uint8_t* snapshot_instructions,
+                                         const uint8_t* kernel_buffer,
+                                         intptr_t kernel_buffer_size) {
   auto IG = T->isolate_group();
   Error& error = Error::Handle(T->zone());
   error = Object::Init(IG, kernel_buffer, kernel_buffer_size);
   if (!error.IsNull()) {
-    return error.ptr();
+    return Utils::StrDup(error.ToCString());
   }
   if (snapshot_data != nullptr && kernel_buffer == nullptr) {
     // Read the snapshot and setup the initial state.
@@ -869,23 +867,21 @@ ErrorPtr Dart::InitIsolateGroupFromSnapshot(
 #endif  // defined(SUPPORT_TIMELINE)
     const Snapshot* snapshot = Snapshot::SetupFromBuffer(snapshot_data);
     if (snapshot == nullptr) {
-      const String& message = String::Handle(String::New("Invalid snapshot"));
-      return ApiError::New(message);
+      return Utils::StrDup("Invalid snapshot");
     }
     if (!IsSnapshotCompatible(vm_snapshot_kind_, snapshot->kind())) {
-      const String& message = String::Handle(String::NewFormatted(
-          "Incompatible snapshot kinds: vm '%s', isolate '%s'",
-          Snapshot::KindToCString(vm_snapshot_kind_),
-          Snapshot::KindToCString(snapshot->kind())));
-      return ApiError::New(message);
+      return OS::SCreate(nullptr,
+                         "Incompatible snapshot kinds: vm '%s', isolate '%s'",
+                         Snapshot::KindToCString(vm_snapshot_kind_),
+                         Snapshot::KindToCString(snapshot->kind()));
     }
     if (FLAG_trace_isolates) {
       OS::PrintErr("Size of isolate snapshot = %" Pd "\n", snapshot->length());
     }
     FullSnapshotReader reader(snapshot, snapshot_instructions, T);
-    const Error& error = Error::Handle(reader.ReadProgramSnapshot());
-    if (!error.IsNull()) {
-      return error.ptr();
+    char* error = reader.ReadProgramSnapshot();
+    if (error != nullptr) {
+      return error;
     }
     {
       // Initialize sentinel field table, which should have sentinel values for
@@ -913,16 +909,14 @@ ErrorPtr Dart::InitIsolateGroupFromSnapshot(
     }
   } else {
     if ((vm_snapshot_kind_ != Snapshot::kNone) && kernel_buffer == nullptr) {
-      const String& message =
-          String::Handle(String::New("Missing isolate snapshot"));
-      return ApiError::New(message);
+      return Utils::StrDup("Missing isolate snapshot");
     }
   }
 #if !defined(PRODUCT) || defined(FORCE_INCLUDE_SAMPLING_HEAP_PROFILER)
   IG->class_table()->PopulateUserVisibleNames();
 #endif
 
-  return Error::null();
+  return nullptr;
 }
 
 #if !defined(DART_PRECOMPILED_RUNTIME)
@@ -946,16 +940,16 @@ static void FinalizeBuiltinClasses(Thread* thread) {
 }
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
-ErrorPtr Dart::InitializeIsolateGroup(Thread* T,
-                                      const uint8_t* snapshot_data,
-                                      const uint8_t* snapshot_instructions,
-                                      const uint8_t* kernel_buffer,
-                                      intptr_t kernel_buffer_size) {
-  auto& error = Error::Handle(
+char* Dart::InitializeIsolateGroup(Thread* T,
+                                   const uint8_t* snapshot_data,
+                                   const uint8_t* snapshot_instructions,
+                                   const uint8_t* kernel_buffer,
+                                   intptr_t kernel_buffer_size) {
+  char* error =
       InitIsolateGroupFromSnapshot(T, snapshot_data, snapshot_instructions,
-                                   kernel_buffer, kernel_buffer_size));
-  if (!error.IsNull()) {
-    return error.ptr();
+                                   kernel_buffer, kernel_buffer_size);
+  if (error != nullptr) {
+    return error;
   }
 
   Object::VerifyBuiltinVtables();
@@ -975,9 +969,9 @@ ErrorPtr Dart::InitializeIsolateGroup(Thread* T,
 
   if (snapshot_data == nullptr || kernel_buffer != nullptr) {
     auto object_store = IG->object_store();
-    error ^= object_store->PreallocateObjects();
+    const Error& error = Error::Handle(object_store->PreallocateObjects());
     if (!error.IsNull()) {
-      return error.ptr();
+      return Utils::StrDup(error.ToErrorCString());
     }
   }
 
@@ -988,7 +982,7 @@ ErrorPtr Dart::InitializeIsolateGroup(Thread* T,
   IG->object_store()->set_tag_table(
       GrowableObjectArray::Handle(GrowableObjectArray::New()));
 
-  return Error::null();
+  return nullptr;
 }
 
 ErrorPtr Dart::InitializeIsolate(Thread* T,

runtime/vm/dart.h

@@ -46,12 +46,14 @@ class Dart : public AllStatic {
                                 IsolateGroup* isolate_group);
 
   // Initialize an isolate group either from a snapshot or from a Kernel binary.
-  static ErrorPtr InitializeIsolateGroup(Thread* T,
-                                         const uint8_t* snapshot_data,
-                                         const uint8_t* snapshot_instructions,
-                                         const uint8_t* kernel_buffer,
-                                         intptr_t kernel_buffer_size);
-  static ErrorPtr InitIsolateGroupFromSnapshot(
+  // On success, returns nullptr. On failure, returns an error message that the
+  // caller must free.
+  static char* InitializeIsolateGroup(Thread* T,
+                                      const uint8_t* snapshot_data,
+                                      const uint8_t* snapshot_instructions,
+                                      const uint8_t* kernel_buffer,
+                                      intptr_t kernel_buffer_size);
+  static char* InitIsolateGroupFromSnapshot(
       Thread* T,
       const uint8_t* snapshot_data,
       const uint8_t* snapshot_instructions,

runtime/vm/dart_api_impl.cc

@@ -1220,24 +1220,27 @@ static Dart_Isolate CreateIsolate(IsolateGroup* group,
     // bootstrap library files which call out to a tag handler that may create
     // Api Handles when an error is encountered.
     T->EnterApiScope();
-    auto& error_obj = Error::Handle(Z);
+    char* error_str = nullptr;
     if (is_new_group) {
-      error_obj = Dart::InitializeIsolateGroup(
+      error_str = Dart::InitializeIsolateGroup(
           T, source->snapshot_data, source->snapshot_instructions,
           source->kernel_buffer, source->kernel_buffer_size);
     }
-    if (error_obj.IsNull()) {
-      error_obj = Dart::InitializeIsolate(T, is_new_group, isolate_data);
-    }
-    if (error_obj.IsNull()) {
+    if (error_str == nullptr) {
+      const Error& error_obj = Error::Handle(
+          Z, Dart::InitializeIsolate(T, is_new_group, isolate_data));
+      if (error_obj.IsNull()) {
 #if defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME)
-      if (FLAG_check_function_fingerprints && !FLAG_precompiled_mode) {
-        Library::CheckFunctionFingerprints();
-      }
+        if (FLAG_check_function_fingerprints && !FLAG_precompiled_mode) {
+          Library::CheckFunctionFingerprints();
+        }
 #endif  // defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME).
-      success = true;
+        success = true;
+      } else if (error != nullptr) {
+        *error = Utils::StrDup(error_obj.ToErrorCString());
+      }
     } else if (error != nullptr) {
-      *error = Utils::StrDup(error_obj.ToErrorCString());
+      *error = error_str;
     }
     // We exit the API scope entered above.
     T->ExitApiScope();
@@ -5663,10 +5666,12 @@ Dart_LoadModuleSnapshot(const uint8_t* snapshot_data,
     return Api::NewError("Invalid snapshot kind");
   }
 
-  const Error& error = Error::Handle(
-      module_snapshot::ReadModuleSnapshot(T, snapshot, snapshot_instructions));
-  if (!error.IsNull()) {
-    return Api::NewHandle(T, error.ptr());
+  char* error =
+      module_snapshot::ReadModuleSnapshot(T, snapshot, snapshot_instructions);
+  if (error != nullptr) {
+    const String& message = String::Handle(String::New(error));
+    free(error);
+    return Api::NewHandle(T, ApiError::New(message));
   }
 
   return Api::Success();
@@ -6113,9 +6118,11 @@ static Dart_Handle DeferredLoadComplete(intptr_t loading_unit_id,
     }
 
     FullSnapshotReader reader(snapshot, snapshot_instructions, T);
-    const Error& error = Error::Handle(reader.ReadUnitSnapshot(unit));
-    if (!error.IsNull()) {
-      return Api::NewHandle(T, error.ptr());
+    char* error = reader.ReadUnitSnapshot(unit);
+    if (error != nullptr) {
+      const String& message = String::Handle(Z, String::New(error));
+      free(error);
+      return Api::NewHandle(T, ApiError::New(message));
     }
 
     return Api::NewHandle(T, unit.CompleteLoad(String::Handle(), false));

runtime/vm/image_snapshot.cc

@@ -2095,17 +2095,15 @@ ImageReader::ImageReader(const uint8_t* data_image,
     : data_image_(ASSERT_NOTNULL(data_image)),
       instructions_image_(ASSERT_NOTNULL(instructions_image)) {}
 
-ApiErrorPtr ImageReader::VerifyAlignment() const {
+char* ImageReader::VerifyAlignment() const {
   // If this changes, bin_to_assembly.py and bin_to_coff.py must also change.
   COMPILE_ASSERT(kObjectStartAlignment == 64);
 
   if (!Utils::IsAligned(data_image_, kObjectStartAlignment) ||
       !Utils::IsAligned(instructions_image_, kObjectStartAlignment)) {
-    return ApiError::New(
-        String::Handle(String::New("Snapshot is misaligned", Heap::kOld)),
-        Heap::kOld);
+    return Utils::StrDup("Snapshot is misaligned");
   }
-  return ApiError::null();
+  return nullptr;
 }
 
 #if defined(DART_PRECOMPILED_RUNTIME)

runtime/vm/image_snapshot.h

@@ -170,7 +170,7 @@ class ImageReader : public ZoneObject {
  public:
   ImageReader(const uint8_t* data_image, const uint8_t* instructions_image);
 
-  ApiErrorPtr VerifyAlignment() const;
+  char* VerifyAlignment() const;
 
   ONLY_IN_PRECOMPILED(uword GetBareInstructionsAt(uint32_t offset) const);
   ONLY_IN_PRECOMPILED(uword GetBareInstructionsEnd() const);

runtime/vm/module_snapshot.cc

@@ -155,7 +155,7 @@ class Deserializer : public ThreadStackResource {
                const uint8_t* instructions_buffer);
   ~Deserializer();
 
-  ApiErrorPtr VerifyVersionAndFeatures();
+  char* VerifyVersionAndFeatures();
 
   ObjectPtr Allocate(intptr_t size);
   static void InitializeHeader(ObjectPtr raw,
@@ -1338,14 +1338,15 @@ Deserializer::~Deserializer() {
   delete[] clusters_;
 }
 
-ApiErrorPtr Deserializer::VerifyVersionAndFeatures() {
+char* Deserializer::VerifyVersionAndFeatures() {
   stream_.SetPosition(Snapshot::kHeaderSize);
 
   const intptr_t format_version = stream_.ReadUnsigned();
   if (format_version != ModuleSnapshot::kFormatVersion) {
-    return ApiError::New(String::Handle(String::NewFormatted(
-        "Invalid module snapshot format version %" Pd " (expected %" Pd ")",
-        format_version, ModuleSnapshot::kFormatVersion)));
+    return OS::SCreate(nullptr,
+                       "Invalid module snapshot format version %" Pd
+                       " (expected %" Pd ")",
+                       format_version, ModuleSnapshot::kFormatVersion);
   }
 
   const char* features =
@@ -1353,19 +1354,18 @@ ApiErrorPtr Deserializer::VerifyVersionAndFeatures() {
   const intptr_t features_length =
       Utils::StrNLen(features, stream_.PendingBytes());
   if (features_length == stream_.PendingBytes()) {
-    return ApiError::New(
-        String::Handle(String::New("The features string in the module snapshot "
-                                   "was not zero-terminated.")));
+    return Utils::StrDup(
+        "The features string in the module snapshot was not zero-terminated.");
   }
   stream_.Advance(features_length + 1);
 
   const char* expected_features = kHostArchitectureName;
   if (strcmp(expected_features, features) != 0) {
-    return ApiError::New(String::Handle(String::NewFormatted(
-        "Invalid module snapshot configuration '%s' (expected '%s')", features,
-        expected_features)));
+    return OS::SCreate(
+        nullptr, "Invalid module snapshot configuration '%s' (expected '%s')",
+        features, expected_features);
   }
-  return ApiError::null();
+  return nullptr;
 }
 
 DeserializationCluster* Deserializer::ReadCluster() {
@@ -1581,22 +1581,22 @@ void Deserializer::Deserialize() {
   }
 }
 
-ApiErrorPtr ReadModuleSnapshot(Thread* thread,
-                               const Snapshot* snapshot,
-                               const uint8_t* instructions_buffer) {
+char* ReadModuleSnapshot(Thread* thread,
+                         const Snapshot* snapshot,
+                         const uint8_t* instructions_buffer) {
   ASSERT(snapshot->kind() == Snapshot::kModule);
 
   Deserializer deserializer(thread, snapshot->Addr(), snapshot->length(),
                             instructions_buffer);
 
-  ApiErrorPtr api_error = deserializer.VerifyVersionAndFeatures();
-  if (api_error != ApiError::null()) {
-    return api_error;
+  char* error = deserializer.VerifyVersionAndFeatures();
+  if (error != nullptr) {
+    return error;
   }
 
   deserializer.Deserialize();
 
-  return ApiError::null();
+  return nullptr;
 }
 
 }  // namespace module_snapshot

runtime/vm/module_snapshot.h

@@ -16,9 +16,11 @@
 namespace dart {
 namespace module_snapshot {
 
-ApiErrorPtr ReadModuleSnapshot(Thread* thread,
-                               const Snapshot* snapshot,
-                               const uint8_t* instructions_buffer);
+// On success, returns nullptr. On failure, returns an error message that the
+// caller must free.
+char* ReadModuleSnapshot(Thread* thread,
+                         const Snapshot* snapshot,
+                         const uint8_t* instructions_buffer);
 
 }  // namespace module_snapshot
 }  // namespace dart