Reland "[ VM / DartDev ] Added support for format and pub commands via standalone VM"

Fixes a malloc/delete[] mismatch.

This reverts commit b3396cbdca.

Change-Id: I18fe142bc60bedd8af4c588ba4039742e1d34606
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134570
Reviewed-by: Liam Appelbe <liama@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

runtime/bin/builtin_impl_sources.gni

@@ -17,6 +17,8 @@ builtin_impl_sources = [
   "crypto_win.cc",
   "dartutils.cc",
   "dartutils.h",
+  "dartdev_utils.cc",
+  "dartdev_utils.h",
   "directory.cc",
   "directory.h",
   "directory_android.cc",
@@ -24,6 +26,8 @@ builtin_impl_sources = [
   "directory_linux.cc",
   "directory_macos.cc",
   "directory_win.cc",
+  "exe_utils.h",
+  "exe_utils.cc",
   "extensions.h",
   "extensions.cc",
   "extensions_android.cc",

runtime/bin/dartdev_utils.cc

@@ -0,0 +1,81 @@
+// Copyright (c) 2020, 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.
+
+#include "bin/dartdev_utils.h"
+
+#include <memory>
+
+#include "bin/directory.h"
+#include "bin/exe_utils.h"
+#include "bin/file.h"
+#include "platform/utils.h"
+
+namespace dart {
+namespace bin {
+
+typedef struct {
+  const char* command;
+  const char* snapshot_name;
+} DartDevCommandMapping;
+
+static const DartDevCommandMapping dart_dev_commands[] = {
+    {"format", "dartfmt.dart.snapshot"},
+    {"pub", "pub.dart.snapshot"},
+};
+
+static const DartDevCommandMapping* FindCommandMapping(const char* command) {
+  intptr_t num_commands =
+      sizeof(dart_dev_commands) / sizeof(dart_dev_commands[0]);
+  for (intptr_t i = 0; i < num_commands; i++) {
+    const DartDevCommandMapping& command_mapping = dart_dev_commands[i];
+    if (strcmp(command, command_mapping.command) == 0) {
+      return &command_mapping;
+    }
+  }
+  return nullptr;
+}
+
+bool DartDevUtils::ShouldParseCommand(const char* script_uri) {
+  return !File::ExistsUri(nullptr, script_uri);
+}
+
+bool DartDevUtils::TryParseCommandFromScriptName(char** script_name) {
+  const DartDevCommandMapping* command = FindCommandMapping(*script_name);
+
+  // Either the command doesn't exist or we've been given an HTTP resource.
+  if (command == nullptr) {
+    return true;
+  }
+
+  // |dir_prefix| includes the last path seperator.
+  auto dir_prefix = std::unique_ptr<char, void (*)(void*)>(
+      EXEUtils::GetDirectoryPrefixFromExeName(), free);
+
+  // First assume we're in dart-sdk/bin.
+  char* snapshot_path = Utils::SCreate("%s/snapshots/%s", dir_prefix.get(),
+                                       command->snapshot_name);
+  if (File::Exists(nullptr, snapshot_path)) {
+    free(*script_name);
+    *script_name = snapshot_path;
+    return true;
+  }
+  free(snapshot_path);
+
+  // If we're not in dart-sdk/bin, we might be in one of the $SDK/out/*
+  // directories. Try to use a snapshot from a previously built SDK.
+  snapshot_path = Utils::SCreate("%s/dart-sdk/bin/snapshots/%s",
+                                 dir_prefix.get(), command->snapshot_name);
+  if (File::Exists(nullptr, snapshot_path)) {
+    free(*script_name);
+    *script_name = snapshot_path;
+    return true;
+  }
+  free(snapshot_path);
+  Syslog::PrintErr("Could not find snapshot for command '%s': %s\n",
+                   *script_name, command->snapshot_name);
+  return false;
+}
+
+}  // namespace bin
+}  // namespace dart

runtime/bin/dartdev_utils.h

@@ -0,0 +1,31 @@
+// Copyright (c) 2020, 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.
+
+#ifndef RUNTIME_BIN_DARTDEV_UTILS_H_
+#define RUNTIME_BIN_DARTDEV_UTILS_H_
+
+#include "platform/globals.h"
+
+namespace dart {
+namespace bin {
+
+class DartDevUtils {
+ public:
+  // Returns true if there does not exist a file at |script_uri|.
+  static bool ShouldParseCommand(const char* script_uri);
+
+  // Returns true if we were successfully able to parse a DartDev command.
+  // Returns false if we were unable to find a matching command or a matching
+  // snapshot does not exist, in which case the VM should exit.
+  static bool TryParseCommandFromScriptName(char** script_name);
+
+ private:
+  DISALLOW_ALLOCATION();
+  DISALLOW_IMPLICIT_CONSTRUCTORS(DartDevUtils);
+};
+
+}  // namespace bin
+}  // namespace dart
+
+#endif  // RUNTIME_BIN_DARTDEV_UTILS_H_

runtime/bin/dfe.cc

@@ -10,6 +10,7 @@
 #include "bin/dartutils.h"
 #include "bin/directory.h"
 #include "bin/error_exit.h"
+#include "bin/exe_utils.h"
 #include "bin/file.h"
 #include "bin/main_options.h"
 #include "bin/platform.h"
@@ -41,49 +42,6 @@ DFE dfe;
 const char kKernelServiceSnapshot[] = "kernel-service.dart.snapshot";
 const char kSnapshotsDirectory[] = "snapshots";
 
-static char* GetDirectoryPrefixFromExeName() {
-  const char* name = nullptr;
-  const int kTargetSize = 4096;
-  char target[kTargetSize];
-  intptr_t target_size =
-      Platform::ResolveExecutablePathInto(target, kTargetSize);
-  if (target_size > 0 && target_size < kTargetSize - 1) {
-    target[target_size] = 0;
-    name = target;
-  }
-  if (name == nullptr) {
-    name = Platform::GetExecutableName();
-    target_size = strlen(name);
-  }
-  Namespace* namespc = Namespace::Create(Namespace::Default());
-  if (File::GetType(namespc, name, false) == File::kIsLink) {
-    // Resolve the link without creating Dart scope String.
-    name = File::LinkTarget(namespc, name, target, kTargetSize);
-    if (name == NULL) {
-      return strdup("");
-    }
-    target_size = strlen(name);
-  }
-  namespc->Release();
-  const char* sep = File::PathSeparator();
-  const intptr_t sep_length = strlen(sep);
-
-  for (intptr_t i = target_size - 1; i >= 0; --i) {
-    const char* str = name + i;
-    if (strncmp(str, sep, sep_length) == 0
-#if defined(HOST_OS_WINDOWS)
-        // TODO(aam): GetExecutableName doesn't work reliably on Windows,
-        // the code below is a workaround for that (we would be using
-        // just single Platform::Separator instead of both slashes if it did).
-        || *str == '/'
-#endif
-    ) {
-      return Utils::StrNDup(name, i + 1);
-    }
-  }
-  return strdup("");
-}
-
 DFE::DFE()
     : use_dfe_(false),
       use_incremental_compiler_(false),
@@ -156,7 +114,7 @@ bool DFE::InitKernelServiceAndPlatformDills(int target_abi_version) {
 
   // |dir_prefix| includes the last path seperator.
   auto dir_prefix = std::unique_ptr<char, void (*)(void*)>(
-      GetDirectoryPrefixFromExeName(), free);
+      EXEUtils::GetDirectoryPrefixFromExeName(), free);
 
   if (target_abi_version != Options::kAbiVersionUnset) {
     kernel_service_dill_ = nullptr;

runtime/bin/exe_utils.cc

@@ -0,0 +1,59 @@
+// Copyright (c) 2020, 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.
+
+#include "bin/exe_utils.h"
+
+#include "bin/directory.h"
+#include "bin/file.h"
+#include "bin/platform.h"
+#include "platform/utils.h"
+
+namespace dart {
+namespace bin {
+
+char* EXEUtils::GetDirectoryPrefixFromExeName() {
+  const char* name = nullptr;
+  const int kTargetSize = 4096;
+  char target[kTargetSize];
+  intptr_t target_size =
+      Platform::ResolveExecutablePathInto(target, kTargetSize);
+  if (target_size > 0 && target_size < kTargetSize - 1) {
+    target[target_size] = 0;
+    name = target;
+  }
+  if (name == nullptr) {
+    name = Platform::GetExecutableName();
+    target_size = strlen(name);
+  }
+  Namespace* namespc = Namespace::Create(Namespace::Default());
+  if (File::GetType(namespc, name, false) == File::kIsLink) {
+    // Resolve the link without creating Dart scope String.
+    name = File::LinkTarget(namespc, name, target, kTargetSize);
+    if (name == NULL) {
+      return strdup("");
+    }
+    target_size = strlen(name);
+  }
+  namespc->Release();
+  const char* sep = File::PathSeparator();
+  const intptr_t sep_length = strlen(sep);
+
+  for (intptr_t i = target_size - 1; i >= 0; --i) {
+    const char* str = name + i;
+    if (strncmp(str, sep, sep_length) == 0
+#if defined(HOST_OS_WINDOWS)
+        // TODO(aam): GetExecutableName doesn't work reliably on Windows,
+        // the code below is a workaround for that (we would be using
+        // just single Platform::Separator instead of both slashes if it did).
+        || *str == '/'
+#endif
+    ) {
+      return Utils::StrNDup(name, i + 1);
+    }
+  }
+  return strdup("");
+}
+
+}  // namespace bin
+}  // namespace dart

runtime/bin/exe_utils.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2020, 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.
+
+#ifndef RUNTIME_BIN_EXE_UTILS_H_
+#define RUNTIME_BIN_EXE_UTILS_H_
+
+#include <stdlib.h>
+#include <string.h>
+
+#include "include/dart_api.h"
+#include "platform/globals.h"
+
+namespace dart {
+namespace bin {
+
+class EXEUtils {
+ public:
+  // Returns the path to the directory the current executable resides in.
+  static char* GetDirectoryPrefixFromExeName();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(EXEUtils);
+};
+
+}  // namespace bin
+}  // namespace dart
+
+#endif  // RUNTIME_BIN_EXE_UTILS_H_

runtime/bin/file.h

@@ -220,6 +220,7 @@ class File : public ReferenceCounted<File> {
 #endif
 
   static bool Exists(Namespace* namespc, const char* path);
+  static bool ExistsUri(Namespace* namespc, const char* uri);
   static bool Create(Namespace* namespc, const char* path);
   static bool CreateLink(Namespace* namespc,
                          const char* path,

runtime/bin/file_android.cc

@@ -241,15 +241,23 @@ File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) {
   return new File(new FileHandle(fd));
 }
 
-File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+static Utils::CStringUniquePtr DecodeUri(const char* uri) {
   const char* path = (strlen(uri) >= 8 && strncmp(uri, "file:///", 8) == 0)
       ? uri + 7 : uri;
   UriDecoder uri_decoder(path);
-  if (uri_decoder.decoded() == NULL) {
+  if (uri_decoder.decoded() == nullptr) {
     errno = EINVAL;
-    return NULL;
+    return Utils::CreateCStringUniquePtr(nullptr);
   }
-  return File::Open(namespc, uri_decoder.decoded(), mode);
+  return Utils::CreateCStringUniquePtr(strdup(uri_decoder.decoded()));
+}
+
+File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return nullptr;
+  }
+  return File::Open(namespc, path.get(), mode);
 }
 
 File* File::OpenStdio(int fd) {
@@ -267,6 +275,14 @@ bool File::Exists(Namespace* namespc, const char* name) {
   }
 }
 
+bool File::ExistsUri(Namespace* namespc, const char* uri) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return false;
+  }
+  return File::Exists(namespc, path.get());
+}
+
 bool File::Create(Namespace* namespc, const char* name) {
   NamespaceScope ns(namespc, name);
   const int fd = TEMP_FAILURE_RETRY(

runtime/bin/file_fuchsia.cc

@@ -243,19 +243,23 @@ File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) {
   return OpenFD(fd);
 }
 
-File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+static Utils::CStringUniquePtr DecodeUri(const char* uri) {
   const char* path = (strlen(uri) >= 8 && strncmp(uri, "file:///", 8) == 0)
       ? uri + 7 : uri;
   UriDecoder uri_decoder(path);
-  if (uri_decoder.decoded() == NULL) {
+  if (uri_decoder.decoded() == nullptr) {
     errno = EINVAL;
-    return NULL;
+    return Utils::CreateCStringUniquePtr(nullptr);
   }
-  return File::Open(namespc, uri_decoder.decoded(), mode);
+  return Utils::CreateCStringUniquePtr(strdup(uri_decoder.decoded()));
 }
 
-File* File::OpenStdio(int fd) {
-  return new File(new FileHandle(fd));
+File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return nullptr;
+  }
+  return File::Open(namespc, path.get(), mode);
 }
 
 bool File::Exists(Namespace* namespc, const char* name) {
@@ -268,6 +272,14 @@ bool File::Exists(Namespace* namespc, const char* name) {
   return false;
 }
 
+bool File::ExistsUri(Namespace* namespc, const char* uri) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return false;
+  }
+  return File::Exists(namespc, path.get());
+}
+
 bool File::Create(Namespace* namespc, const char* name) {
   NamespaceScope ns(namespc, name);
   const int fd = NO_RETRY_EXPECTED(

runtime/bin/file_linux.cc

@@ -244,15 +244,23 @@ File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) {
   return OpenFD(fd);
 }
 
-File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+static Utils::CStringUniquePtr DecodeUri(const char* uri) {
   const char* path = (strlen(uri) >= 8 && strncmp(uri, "file:///", 8) == 0)
       ? uri + 7 : uri;
   UriDecoder uri_decoder(path);
-  if (uri_decoder.decoded() == NULL) {
+  if (uri_decoder.decoded() == nullptr) {
     errno = EINVAL;
-    return NULL;
+    return Utils::CreateCStringUniquePtr(nullptr);
   }
-  return File::Open(namespc, uri_decoder.decoded(), mode);
+  return Utils::CreateCStringUniquePtr(strdup(uri_decoder.decoded()));
+}
+
+File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return nullptr;
+  }
+  return File::Open(namespc, path.get(), mode);
 }
 
 File* File::OpenStdio(int fd) {
@@ -270,6 +278,14 @@ bool File::Exists(Namespace* namespc, const char* name) {
   }
 }
 
+bool File::ExistsUri(Namespace* namespc, const char* uri) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return false;
+  }
+  return File::Exists(namespc, path.get());
+}
+
 bool File::Create(Namespace* namespc, const char* name) {
   NamespaceScope ns(namespc, name);
   const int fd = TEMP_FAILURE_RETRY(

runtime/bin/file_macos.cc

@@ -286,15 +286,23 @@ File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) {
   return new File(new FileHandle(fd));
 }
 
-File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+static Utils::CStringUniquePtr DecodeUri(const char* uri) {
   const char* path = (strlen(uri) >= 8 && strncmp(uri, "file:///", 8) == 0)
       ? uri + 7 : uri;
   UriDecoder uri_decoder(path);
-  if (uri_decoder.decoded() == NULL) {
+  if (uri_decoder.decoded() == nullptr) {
     errno = EINVAL;
-    return NULL;
+    return Utils::CreateCStringUniquePtr(nullptr);
   }
-  return File::Open(namespc, uri_decoder.decoded(), mode);
+  return Utils::CreateCStringUniquePtr(strdup(uri_decoder.decoded()));
+}
+
+File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return nullptr;
+  }
+  return File::Open(namespc, path.get(), mode);
 }
 
 File* File::OpenStdio(int fd) {
@@ -311,6 +319,14 @@ bool File::Exists(Namespace* namespc, const char* name) {
   }
 }
 
+bool File::ExistsUri(Namespace* namespc, const char* uri) {
+  auto path = DecodeUri(uri);
+  if (path == nullptr) {
+    return false;
+  }
+  return File::Exists(namespc, path.get());
+}
+
 bool File::Create(Namespace* namespc, const char* name) {
   int fd = TEMP_FAILURE_RETRY(open(name, O_RDONLY | O_CREAT, 0666));
   if (fd < 0) {

runtime/bin/file_win.cc

@@ -353,6 +353,15 @@ bool File::Exists(Namespace* namespc, const char* name) {
   return StatHelper(system_name.wide(), &st);
 }
 
+bool File::ExistsUri(Namespace* namespc, const char* uri) {
+  UriDecoder uri_decoder(uri);
+  if (uri_decoder.decoded() == nullptr) {
+    SetLastError(ERROR_INVALID_NAME);
+    return false;
+  }
+  return File::Exists(namespc, uri_decoder.decoded());
+}
+
 bool File::Create(Namespace* namespc, const char* name) {
   Utf8ToWideScope system_name(name);
   int fd = _wopen(system_name.wide(), O_RDONLY | O_CREAT, 0666);

runtime/bin/main.cc

@@ -15,9 +15,9 @@
 #include "bin/builtin.h"
 #include "bin/console.h"
 #include "bin/crashpad.h"
+#include "bin/dartdev_utils.h"
 #include "bin/dartutils.h"
 #include "bin/dfe.h"
-#include "bin/directory.h"
 #include "bin/error_exit.h"
 #include "bin/eventhandler.h"
 #include "bin/extensions.h"
@@ -37,6 +37,7 @@
 #include "platform/hashmap.h"
 #include "platform/syslog.h"
 #include "platform/text_buffer.h"
+#include "platform/utils.h"
 
 extern "C" {
 extern const uint8_t kDartVmSnapshotData[];
@@ -1072,31 +1073,47 @@ void main(int argc, char** argv) {
 #endif
 
   // Parse command line arguments.
-  if (app_snapshot == nullptr &&
-      Options::ParseArguments(argc, argv, vm_run_app_snapshot, &vm_options,
-                              &script_name, &dart_options, &print_flags_seen,
-                              &verbose_debug_seen) < 0) {
-    if (Options::help_option()) {
-      Options::PrintUsage();
-      Platform::Exit(0);
-    } else if (Options::version_option()) {
-      Options::PrintVersion();
-      Platform::Exit(0);
-    } else if (print_flags_seen) {
-      // Will set the VM flags, print them out and then we exit as no
-      // script was specified on the command line.
-      char* error = Dart_SetVMFlags(vm_options.count(), vm_options.arguments());
-      if (error != NULL) {
-        Syslog::PrintErr("Setting VM flags failed: %s\n", error);
-        free(error);
+  if (app_snapshot == nullptr) {
+    int result = Options::ParseArguments(
+        argc, argv, vm_run_app_snapshot, &vm_options, &script_name,
+        &dart_options, &print_flags_seen, &verbose_debug_seen);
+    if (result < 0) {
+      if (Options::help_option()) {
+        Options::PrintUsage();
+        Platform::Exit(0);
+      } else if (Options::version_option()) {
+        Options::PrintVersion();
+        Platform::Exit(0);
+      } else if (print_flags_seen) {
+        // Will set the VM flags, print them out and then we exit as no
+        // script was specified on the command line.
+        char* error =
+            Dart_SetVMFlags(vm_options.count(), vm_options.arguments());
+        if (error != NULL) {
+          Syslog::PrintErr("Setting VM flags failed: %s\n", error);
+          free(error);
+          Platform::Exit(kErrorExitCode);
+        }
+        Platform::Exit(0);
+      } else {
+        Options::PrintUsage();
         Platform::Exit(kErrorExitCode);
       }
-      Platform::Exit(0);
-    } else {
-      Options::PrintUsage();
+    }
+
+    // Try to parse a DartDev command if script_name doesn't point to a valid
+    // file. If the command isn't valid we fall through to handle the
+    // possibility that the script_name points to a HTTP resource. If the
+    // relevant snapshot can't be found we abort execution.
+    if (DartDevUtils::ShouldParseCommand(script_name) &&
+        !DartDevUtils::TryParseCommandFromScriptName(&script_name)) {
       Platform::Exit(kErrorExitCode);
     }
   }
+
+  // At this point, script_name now points to either a script or a snapshot
+  // determined by DartDevUtils above.
+
   DartUtils::SetEnvironment(Options::environment());
 
   if (Options::suppress_core_dump()) {

runtime/bin/main_options.cc

@@ -440,7 +440,7 @@ int Options::ParseArguments(int argc,
 
   // Get the script name.
   if (i < argc) {
-    *script_name = argv[i];
+    *script_name = strdup(argv[i]);
     i++;
   } else {
     return -1;

runtime/platform/utils.cc

@@ -295,4 +295,8 @@ char* Utils::VSCreate(const char* format, va_list args) {
   return buffer;
 }
 
+Utils::CStringUniquePtr Utils::CreateCStringUniquePtr(char* str) {
+  return std::unique_ptr<char, decltype(std::free)*>{str, std::free};
+}
+
 }  // namespace dart

runtime/platform/utils.h

@@ -6,6 +6,7 @@
 #define RUNTIME_PLATFORM_UTILS_H_
 
 #include <limits>
+#include <memory>
 #include <type_traits>
 
 #include "platform/assert.h"
@@ -416,6 +417,11 @@ class Utils {
   // Allocate a string and print formatted output into a malloc'd buffer.
   static char* SCreate(const char* format, ...) PRINTF_ATTRIBUTE(1, 2);
   static char* VSCreate(const char* format, va_list args);
+
+  typedef std::unique_ptr<char, decltype(std::free)*> CStringUniquePtr;
+
+  // Returns str in a unique_ptr with free used as its deleter.
+  static CStringUniquePtr CreateCStringUniquePtr(char* str);
 };
 
 }  // namespace dart