Fuchsia: Improves run_vm_tests wrapper program.

runtime/bin/run_vm_tests_fuchsia.cc

Index: runtime/bin/run_vm_tests_fuchsia.cc
diff --git a/runtime/bin/run_vm_tests_fuchsia.cc b/runtime/bin/run_vm_tests_fuchsia.cc
index fddba5e0b4920d61b2ad1de9b2eddb5a15e565a4..9823461f39578e354c49587fb0e6b96ce009410e 100644
--- a/runtime/bin/run_vm_tests_fuchsia.cc
+++ b/runtime/bin/run_vm_tests_fuchsia.cc
@@ -4,6 +4,7 @@
 
 #include <fcntl.h>
 #include <launchpad/launchpad.h>
+#include <launchpad/vmo.h>
 #include <magenta/syscalls.h>
 #include <mxio/util.h>
 #include <pthread.h>
@@ -17,16 +18,12 @@
 // This program runs Dart VM unit tests. The Dart VM unit tests are contained
 // in a separate binary whose location is defined in kRunVmTestsPath below.
 // That program accepts a command line argument --list to list all the available
-// tests, or the name of a single test to run. This program accepts a single
-// command line argument which is the path to a file containing a list of tests
-// to run, one per line.
+// tests, or the name of a single test to run. This program grabs the list of
+// tests, and then runs them.
 
 // TODO(zra): Make this a command line argument
 const char* kRunVmTestsPath = "/boot/bin/dart_vm_tests";
 
-// The simulator only has 512MB;
-const intptr_t kOldGenHeapSizeMB = 256;
-
 // Tests that are invalid, wedge, or cause panics.
 const char* kSkip[] = {
   // These expect a file to exist that we aren't putting in the image.
@@ -144,29 +141,143 @@ static bool isBug(const char* test) {
 }
 
 
-static int run_test(const char* test_name) {
-  const intptr_t kArgc = 3;
-  const char* argv[kArgc];
+// This is mostly taken from //magenta/system/uapp/mxsh with the addtion of
+// launchpad_add_pipe calls to setup pipes for stdout and stderr.
+static mx_status_t lp_setup(launchpad_t** lp_out, mx_handle_t binary_vmo,
+                            int argc, const char* const* argv,
+                            int *stdout_out, int *stderr_out) {
+  launchpad_t* lp;
+  mx_status_t status;
+  if ((status = launchpad_create(argv[0], &lp)) < 0) {
+    return status;
+  }
+  if ((status = launchpad_arguments(lp, argc, argv)) < 0) {
+    launchpad_destroy(lp);
+    return status;
+  }
+  if ((status = launchpad_clone_mxio_root(lp)) < 0) {
+    launchpad_destroy(lp);
+    return status;
+  }
+  if ((stdout_out != NULL) &&
+      (status = launchpad_add_pipe(lp, stdout_out, 1)) < 0) {
+    launchpad_destroy(lp);
+    return status;
+  }
+  if ((stderr_out != NULL) &&
+      (status = launchpad_add_pipe(lp, stderr_out, 2)) < 0) {
+    launchpad_destroy(lp);

[siva, 2016/08/18 00:38:46]

Missing
if (stdout_out != NULL) {
  close(*stdout_out);
}

[zra, 2016/08/18 15:49:31]

Moved error handling to the caller.

+    return status;
+  }
+  if ((status = launchpad_elf_load(lp, binary_vmo)) < 0) {
+    launchpad_destroy(lp);
+    if (stdout_out != NULL) {
+      close(*stdout_out);
+    }
+    if (stderr_out != NULL) {
+      close(*stderr_out);
+    }
+    return status;
+  }

[siva, 2016/08/18 00:38:46]

ASSERT(lp_out != NULL);

[zra, 2016/08/18 15:49:31]

Done.

+  *lp_out = lp;
+  return NO_ERROR;
+}
 
-  char old_gen_arg[64];
-  snprintf(old_gen_arg, sizeof(old_gen_arg), "--old_gen_heap_size=%ld",
-      kOldGenHeapSizeMB);
+
+// Start the test running and return file descriptors for the stdout and stderr
+// pipes.
+static mx_handle_t start_test(mx_handle_t binary_vmo, const char* test_name,
+                              int* stdout_out, int* stderr_out) {
+  const intptr_t kArgc = 2;
+  const char* argv[kArgc];
 
   argv[0] = kRunVmTestsPath;
-  argv[1] = old_gen_arg;
-  argv[2] = test_name;
+  argv[1] = test_name;
+
+  launchpad_t* lp;
+  int stdout_pipe = -1;
+  int stderr_pipe = -1;
+  mx_status_t r = lp_setup(
+      &lp, binary_vmo, kArgc, argv, &stdout_pipe, &stderr_pipe);
+  if (r != NO_ERROR) {
+    fprintf(stderr, "Failed to setup process\n");
+    return -1;
+  }
 
-  mx_handle_t p = launchpad_launch_mxio(argv[0], kArgc, argv);
+  mx_handle_t p = launchpad_start(lp);
+  launchpad_destroy(lp);
   if (p < 0) {
-    fprintf(stderr, "process failed to start\n");
+    close(stdout_pipe);
+    close(stderr_pipe);
+    fprintf(stderr, "Failed to start process\n");
+    fflush(0);
     return -1;
   }
 
+  if (stdout_out != NULL) {
+    *stdout_out = stdout_pipe;
+  } else {
+    close(stdout_pipe);
+  }
+  if (stderr_out != NULL) {
+    *stderr_out = stderr_pipe;
+  } else {
+    close(stderr_pipe);
+  }
+  return p;
+}
+
+
+// Drain fd into a buffer pointed to by 'buffer'. Assumes that the data is a
+// C string, and null-terminates it. Returns the number of bytes read.
+static intptr_t drain_fd(int fd, char** buffer) {
+  const intptr_t kDrainInitSize = 64;
+  char* buf = reinterpret_cast<char*>(malloc(kDrainInitSize));
+  intptr_t free_space = kDrainInitSize;
+  intptr_t total_read = 0;
+  intptr_t read_size = 0;
+  while ((read_size = read(fd, buf + total_read, free_space)) != 0) {
+    if (read_size == -1) {
+      break;
+    }
+    total_read += read_size;
+    free_space -= read_size;
+    if (free_space <= 1) {
+      // size = size * 1.5.

[siva, 2016/08/18 00:38:46]

maybe // new_size = size * 1.5.

[zra, 2016/08/18 15:49:31]

Done.

+      intptr_t new_size = (total_read << 1) - (total_read >> 1);
+      buf = reinterpret_cast<char*>(realloc(buf, new_size));
+      free_space = new_size - total_read;
+    }
+  }
+  buf[total_read] = '\0';
+  close(fd);
+  *buffer = buf;
+  return total_read;
+}
+
+
+// Runs test 'test_name' and gives stdout and stderr for the test in
+// 'test_stdout' and 'test_stderr'. Returns the exit code from the test.
+static int run_test(mx_handle_t binary_vmo, const char* test_name,
+                    char** test_stdout, char** test_stderr) {
+  int stdout_pipe = -1;
+  int stderr_pipe = -1;
+  mx_handle_t p = start_test(binary_vmo, test_name, &stdout_pipe, &stderr_pipe);
+  if (p < 0) {
+    return p;
+  }
+
+  drain_fd(stdout_pipe, test_stdout);
+  drain_fd(stderr_pipe, test_stderr);
+
   mx_signals_state_t state;
   mx_status_t r = mx_handle_wait_one(
       p, MX_SIGNAL_SIGNALED, MX_TIME_INFINITE, &state);
   if (r != NO_ERROR) {
     fprintf(stderr, "[process(%x): wait failed? %d]\n", p, r);
+    fflush(0);
+    free(test_stdout);
+    free(test_stderr);
     return -1;
   }
 
@@ -175,6 +286,9 @@ static int run_test(const char* test_name) {
       p, MX_INFO_PROCESS, &proc_info, sizeof(proc_info));
   if (ret != sizeof(proc_info)) {
     fprintf(stderr, "[process(%x): handle_get_info failed? %ld]\n", p, ret);
+    fflush(0);
+    free(test_stdout);
+    free(test_stderr);
     return -1;
   }
 
@@ -183,17 +297,22 @@ static int run_test(const char* test_name) {
 }
 
 
-static void handle_result(intptr_t result, const char* test) {
+static void handle_result(
+    intptr_t result, char* test_stdout, char* test_stderr, const char* test) {
   if (result != 0) {
     if (!isExpectFail(test) && !isBug(test)) {
-      printf("******** Test %s FAILED\n", test);
+      printf("**** Test %s FAILED\n\nstdout:\n%s\nstderr:\n%s\n",
+          test, test_stdout, test_stderr);
     }
   } else {
     if (isExpectFail(test)) {
-      printf("******** Test %s is expected to fail, but PASSED\n", test);
-    }
-    if (isBug(test)) {
-      printf("******** Test %s is marked as a bug, but PASSED\n", test);
+      printf("**** Test %s is expected to fail, but PASSED\n\n"
+             "stdout:\n%s\nstderr:\n%s\n",
+             test, test_stdout, test_stderr);
+    } else if (isBug(test)) {
+      printf("**** Test %s is marked as a bug, but PASSED\n", test);
+    } else {
+      printf("**** Test %s PASSED\n", test);
     }
   }
 }
@@ -204,6 +323,7 @@ typedef struct {
   char** test_list;
   intptr_t test_list_length;
   intptr_t* test_list_index;
+  mx_handle_t binary_vmo;
 } runner_args_t;
 
 
@@ -211,12 +331,20 @@ static void* test_runner_thread(void* arg) {
   runner_args_t* args = reinterpret_cast<runner_args_t*>(arg);
 
   pthread_mutex_lock(args->test_list_lock);
+  mx_handle_t binary_vmo = args->binary_vmo;
   while (*args->test_list_index < args->test_list_length) {
     const intptr_t index = *args->test_list_index;
     *args->test_list_index = index + 1;
     pthread_mutex_unlock(args->test_list_lock);
+
     const char* test = args->test_list[index];
-    handle_result(run_test(test), test);
+    char* test_stdout = NULL;
+    char* test_stderr = NULL;
+    mx_handle_t vmo_dup = mx_handle_duplicate(binary_vmo, MX_RIGHT_SAME_RIGHTS);
+    int test_status = run_test(vmo_dup, test, &test_stdout, &test_stderr);

[siva, 2016/08/18 00:38:46]

should this be :
if (test_status != -1) {
  handle_result(...);
  free(test_stdout);
  free(test_stderr);
}

[zra, 2016/08/18 15:49:31]

-1 is a return code from run_test that we need to report/handle. free() is NULL
safe.

[siva, 2016/08/18 16:06:12]

but test_stdout and test_stderr are not being set to NULL after the free when
return code is -1.

+    handle_result(test_status, test_stdout, test_stderr, test);
+    free(test_stdout);
+    free(test_stderr);
     pthread_mutex_lock(args->test_list_lock);
   }
   pthread_mutex_unlock(args->test_list_lock);
@@ -225,11 +353,17 @@ static void* test_runner_thread(void* arg) {
 }
 
 
-static void trim(char* line) {
-  const intptr_t line_len = strlen(line);
-  if (line[line_len - 1] == '\n') {
-    line[line_len - 1] = '\0';
+static void run_all_tests(runner_args_t* args) {
+  const intptr_t num_cpus = mxr_get_nprocs_conf();
+  pthread_t* threads =
+    reinterpret_cast<pthread_t*>(malloc(num_cpus * sizeof(pthread_t)));
+  for (int i = 0; i < num_cpus; i++) {
+    pthread_create(&threads[i], NULL, test_runner_thread, args);
   }
+  for (int i = 0; i < num_cpus; i++) {
+    pthread_join(threads[i], NULL);
+  }
+  free(threads);
 }
 
 
@@ -238,64 +372,59 @@ static bool should_run(const char* test) {
 }
 
 
-static intptr_t count_lines(FILE* fp) {
-  intptr_t lines = 0;
-
-  // Make sure we're at the beginning of the file.
-  rewind(fp);
-
-  intptr_t ch;
-  while ((ch = fgetc(fp)) != EOF) {
-    if (ch == '\n') {
-      lines++;
+static char** parse_test_list(char* list_output, intptr_t* length) {
+  const intptr_t list_output_length = strlen(list_output);
+  intptr_t test_count = 0;
+  for (int i = 0; i < list_output_length; i++) {
+    if (list_output[i] == '\n') {
+      test_count++;
     }
   }
-
-  rewind(fp);
-  return lines;
-}
-
-
-static intptr_t read_lines(FILE* fp, char** lines, intptr_t lines_length) {
-  char* test = NULL;
-  size_t len = 0;
-  ssize_t read;
-  intptr_t i = 0;
-  while (((read = getline(&test, &len, fp)) != -1) && (i < lines_length)) {
-    trim(test);
-    if (!should_run(test)) {
-      continue;
+  char** test_list;
+  test_list = reinterpret_cast<char**>(malloc(test_count * sizeof(*test_list)));
+  char* test = list_output;
+  char* strtok_context;
+  intptr_t idx = 0;
+  while ((test = strtok_r(test, "\n", &strtok_context)) != NULL) {
+    if (should_run(test)) {
+      test_list[idx] = strdup(test);
+      idx++;
     }
-    lines[i] = strdup(test);
-    i++;
+    test = NULL;
   }
 
-  if (test != NULL) {
-    free(test);
-  }
-  return i;
+  *length = idx;
+  return test_list;
 }
 
 
 int main(int argc, char** argv) {
-  if (argc <= 1) {
-    fprintf(stderr, "Pass the path to a file containing the list of tests\n");
+  // TODO(zra): Read test binary path from the command line.
+
+  // Load in the binary.
+  mx_handle_t binary_vmo = launchpad_vmo_from_file(kRunVmTestsPath);
+
+  // Run with --list to grab the list of tests.
+  char* list_stdout = NULL;
+  char* list_stderr = NULL;
+  mx_handle_t list_vmo = mx_handle_duplicate(binary_vmo, MX_RIGHT_SAME_RIGHTS);
+  int list_result = run_test(list_vmo, "--list", &list_stdout, &list_stderr);
+  if (list_result != 0) {
+    fprintf(stderr, "Failed to list tests: %s\n%s\n", list_stdout, list_stderr);
+    fflush(0);
     return -1;
   }
-  const char* tests_path = argv[1];
 
-  FILE* fp = fopen(tests_path, "r");
-  if (fp == NULL) {
-    fprintf(stderr, "Failed to read the file: %s\n", tests_path);
-    return -1;
-  }
+  // Parse the test list into an array of C strings.
+  intptr_t lines_count;
+  char** test_list = parse_test_list(list_stdout, &lines_count);
+  free(list_stdout);
+  free(list_stderr);
 
-  intptr_t lines_count = count_lines(fp);
-  char** test_list =
-      reinterpret_cast<char**>(malloc(sizeof(*test_list) * lines_count));
-  lines_count = read_lines(fp, test_list, lines_count);
-  fclose(fp);
+  fprintf(stdout, "Found %ld tests\n", lines_count);
+  fflush(0);
 
+  // Run the tests across a number of threads equal to the number of cores.
   pthread_mutex_t args_mutex;
   pthread_mutex_init(&args_mutex, NULL);
   intptr_t test_list_index = 0;
@@ -304,29 +433,22 @@ int main(int argc, char** argv) {
   args.test_list = test_list;
   args.test_list_length = lines_count;
   args.test_list_index = &test_list_index;
+  args.binary_vmo = binary_vmo;
+  run_all_tests(&args);
 
-  const intptr_t num_cpus = mxr_get_nprocs_conf();
-  pthread_t* threads =
-    reinterpret_cast<pthread_t*>(malloc(num_cpus * sizeof(pthread_t)));
-  for (int i = 0; i < num_cpus; i++) {
-    pthread_create(&threads[i], NULL, test_runner_thread, &args);
-  }
-
-  for (int i = 0; i < num_cpus; i++) {
-    pthread_join(threads[i], NULL);
-  }
-
-  free(threads);
+  // Cleanup.
   for (int i = 0; i < lines_count; i++) {
     free(test_list[i]);
   }
   free(test_list);
   pthread_mutex_destroy(&args_mutex);
+  mx_handle_close(binary_vmo);
 
+  // Complain if we didn't try to run all of the tests.
   if (test_list_index != lines_count) {
     fprintf(stderr, "Failed to attempt all the tests!\n");
+    fflush(0);
     return -1;
   }
-
   return 0;
 }