[vm] Fix parsing of DART_VM_OPTIONS environment variable

This change fixes the following problems when parsing DART_VM_OPTIONS
environment variable:

* Resize vm_options to accomodate for extra flags from
  DART_VM_OPTIONS.

* Prepend executable name to the argv to be compatible with
  implementation of Platform.executableArguments.

* Use strchr instead of strtok_r to avoid modifying value of
  DART_VM_OPTIONS environment variable in place.

TEST=pkg/dartdev/test/commands/compile_test.dart

Fixes https://github.com/dart-lang/sdk/issues/62957
Fixes https://github.com/dart-lang/sdk/issues/62958

Change-Id: I86e0ece20844888333b2a0765c82a5ad1fba54b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491001
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>

pkg/dartdev/test/commands/compile_test.dart

@@ -680,6 +680,70 @@ void main() {
     expect(result.exitCode, 0);
   }, skip: isRunningOnIA32);
 
+  test(
+    'Compile and run exe with DART_VM_OPTIONS - multiple options',
+    () async {
+      final p = project(
+        mainSrc: '''
+        import 'dart:io';
+
+        void main() {
+          print(Platform.executableArguments);
+        }''',
+      );
+      final inFile = path.canonicalize(
+        path.join(p.dirPath, p.relativeFilePath),
+      );
+      final outFile = path.canonicalize(path.join(p.dirPath, 'myexe'));
+
+      var result = await p.run(['compile', 'exe', '-o', outFile, inFile]);
+
+      expect(result.stdout, isNot(contains(soundNullSafetyMessage)));
+      expect(result.stderr, isEmpty);
+      expect(result.exitCode, 0);
+      expect(
+        File(outFile).existsSync(),
+        true,
+        reason: 'File not found: $outFile',
+      );
+
+      void testVmOptions(List<String> vmOptions) {
+        var result = Process.runSync(
+          outFile,
+          [],
+          environment: <String, String>{'DART_VM_OPTIONS': vmOptions.join(',')},
+        );
+
+        expect(result.stderr, isEmpty);
+        expect(result.stdout, contains(vmOptions.toString()));
+        expect(result.exitCode, 0);
+      }
+
+      testVmOptions([
+        '--use_compactor',
+        '--force_evacuation',
+        '--idle_duration_micros=0',
+        '--compactor_tasks=5',
+        '--marker_tasks=5',
+        '--scavenger_tasks=5',
+        '--old_gen_growth_time_ratio=50',
+        '--old_gen_growth_space_ratio=10',
+      ]);
+      testVmOptions([
+        '--use_compactor',
+        '--force_evacuation',
+        '--idle_duration_micros=0',
+        '--compactor_tasks=5',
+        '--marker_tasks=5',
+        '--scavenger_tasks=5',
+        '--mark_when_idle',
+        '--old_gen_growth_time_ratio=50',
+        '--old_gen_growth_space_ratio=10',
+      ]);
+    },
+    skip: isRunningOnIA32,
+  );
+
   test('Compile exe without info', () async {
     final p = project(mainSrc: '''void main() {}''');
     final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath));

runtime/bin/dartdev_options.cc

@@ -369,7 +369,7 @@ void Options::DestroyEnvironment() {
   }
 }
 
-char** Options::GetEnvArguments(int* argc) {
+char** Options::GetEnvArguments(const char* executable_name, int* argc) {
   return nullptr;
 }
 

runtime/bin/dartdev_options.h

@@ -91,7 +91,7 @@ class Options {
 
 #if defined(DART_PRECOMPILED_RUNTIME)
   // Get the list of options in DART_VM_OPTIONS.
-  static char** GetEnvArguments(int* argc);
+  static char** GetEnvArguments(const char* executable_name, int* argc);
 #endif  // defined(DART_PRECOMPILED_RUNTIME)
 
  private:

runtime/bin/dartutils.h

@@ -64,8 +64,8 @@ class CommandLineOptions {
  public:
   explicit CommandLineOptions(int max_count)
       : count_(0), max_count_(max_count), arguments_(nullptr) {
-    const int kWordSize = sizeof(intptr_t);
-    arguments_ = reinterpret_cast<const char**>(malloc(max_count * kWordSize));
+    arguments_ =
+        reinterpret_cast<const char**>(malloc(max_count * sizeof(const char*)));
     if (arguments_ == nullptr) {
       max_count_ = 0;
     }
@@ -104,6 +104,17 @@ class CommandLineOptions {
     }
   }
 
+  void EnsureCapacity(int max_count) {
+    if (max_count > max_count_) {
+      max_count_ = max_count;
+      arguments_ = reinterpret_cast<const char**>(
+          realloc(arguments_, max_count * sizeof(const char*)));
+      if (arguments_ == nullptr) {
+        max_count_ = 0;
+      }
+    }
+  }
+
   Dart_Handle CreateRuntimeOptions();
 
 #if defined(DEBUG)

runtime/bin/main_impl.cc

@@ -1236,11 +1236,12 @@ void main(int argc, char** argv) {
 
       // Parse DART_VM_OPTIONS options.
       int env_argc = 0;
-      char** env_argv = Options::GetEnvArguments(&env_argc);
+      char** env_argv = Options::GetEnvArguments(argv[0], &env_argc);
       if (env_argv != nullptr) {
         // Any Dart options that are generated based on parsing DART_VM_OPTIONS
         // are useless, so we'll throw them away rather than passing them along.
         CommandLineOptions tmp_options(env_argc + EXTRA_VM_ARGUMENTS);
+        vm_options.EnsureCapacity(env_argc + EXTRA_VM_ARGUMENTS);
         parse_arguments(env_argc, env_argv, &vm_options, &tmp_options,
                         /*parsing_dart_vm_options=*/true);
       }

runtime/bin/main_options.cc

@@ -131,15 +131,8 @@ bool Options::ParseArguments(int argc,
                              char** script_name,
                              CommandLineOptions* dart_options,
                              bool* print_flags_seen) {
-  int i = 0;
-#if !defined(DART_PRECOMPILED_RUNTIME)
-  // DART_VM_OPTIONS is only implemented for compiled executables.
-  ASSERT(!parsing_dart_vm_options);
-#endif  // !defined(DART_PRECOMPILED_RUNTIME)
-  if (!parsing_dart_vm_options) {
-    // Start processing arguments after argv[0] which would be the executable.
-    i = 1;
-  }
+  // Start processing arguments after argv[0] which would be the executable.
+  int i = 1;
 
   CommandLineOptions temp_vm_options(vm_options->max_count());
   // Parse out the vm options.
@@ -332,7 +325,7 @@ void Options::DestroyEnvironment() {
 // with no spaces. Options that support providing multiple values as
 // comma-separated lists (e.g., --timeline-streams=Dart,GC,Compiler,Microtask)
 // are not supported and will cause argument parsing to fail.
-char** Options::GetEnvArguments(int* argc) {
+char** Options::GetEnvArguments(const char* executable_name, int* argc) {
   ASSERT(argc != nullptr);
   const char* env_args_str = std::getenv("DART_VM_OPTIONS");
   if (env_args_str == nullptr) {
@@ -359,19 +352,28 @@ char** Options::GetEnvArguments(int* argc) {
     }
   }
 
+  // Account for executable name.
+  ++arg_count;
+
   env_argv_ = new char*[arg_count];
   env_argc_ = arg_count;
   *argc = arg_count;
 
-  int current_arg = 0;
-  char* token;
-  char* rest = const_cast<char*>(env_args_str);
+  env_argv_[0] = Utils::StrDup(executable_name);
 
   // Split out the individual arguments.
-  while ((token = strtok_r(rest, ",", &rest)) != nullptr) {
+  const char* token = env_args_str;
+  for (int current_arg = 1; current_arg < arg_count; ++current_arg) {
     // TODO(bkonyi): consider stripping leading/trailing whitespace from
     // arguments.
-    env_argv_[current_arg++] = Utils::StrNDup(token, rest - token);
+    const char* end = strchr(token, ',');
+    if (end != nullptr) {
+      env_argv_[current_arg] = Utils::StrNDup(token, end - token);
+      token = end + 1;
+    } else {
+      env_argv_[current_arg] = Utils::StrDup(token);
+      break;
+    }
   }
 
   return env_argv_;

runtime/bin/main_options.h

@@ -172,7 +172,7 @@ class Options {
 
 #if defined(DART_PRECOMPILED_RUNTIME)
   // Get the list of options in DART_VM_OPTIONS.
-  static char** GetEnvArguments(int* argc);
+  static char** GetEnvArguments(const char* executable_name, int* argc);
 #endif  // defined(DART_PRECOMPILED_RUNTIME)
 
  private: