Creating a pexe content handler to translate and run pexes.

mojo/nacl/nonsfi/irt_pnacl_translator_compile.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <fcntl.h>
+
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "mojo/nacl/nonsfi/irt_mojo_nonsfi.h"
+#include "mojo/public/cpp/bindings/string.h"
+#include "native_client/src/untrusted/irt/irt_dev.h"
+
+void serve_translate_request(const struct nacl_irt_pnacl_compile_funcs* funcs) {
+  // Acquire the handle -- this is our mechanism to contact the
+  // content handler which called us.
+  MojoHandle handle;
+  nacl::_MojoGetInitialHandle(&handle);
+
+  // Read / Wait until we can get the pexe file name.
+  MojoResult result;
+  char pexe_file_name[PATH_MAX];
+  uint32_t path_size = sizeof(pexe_file_name);
+  do {
+    result = MojoReadMessage(handle,

[Mark Seaborn, 2015/10/20 21:32:40]

I don't think you should be using raw Mojo IPC.  Can you use Mojo's message
marshalling layer?

Also, there should be a big TODO somewhere to pass FDs across IPC instead of
passing filenames. :-)

[Sean Klein, 2015/10/22 21:50:00]

I think this is somewhere that vtl should get involved in the discussion.

1) I'm not entirely sure how to use the mojom layer for sending marshalled
strings when my only access to the world is through a raw "MojoHandle".
2) We discussed the concept of sending FDs through IPC -- It seems something
that Mojo devs are strongly opposed to.

+                             pexe_file_name,
+                             &path_size,
+                             nullptr,
+                             nullptr,
+                             MOJO_READ_MESSAGE_FLAG_NONE);
+    if (result == MOJO_RESULT_SHOULD_WAIT) {
+      result = MojoWait(handle,
+                        MOJO_HANDLE_SIGNAL_READABLE,
+                        MOJO_DEADLINE_INDEFINITE,
+                        nullptr);
+      if (result != MOJO_RESULT_OK)
+        LOG(FATAL) << "Pexe translator could not wait for pexe name";
+    } else {
+      break;
+    }
+  } while (true);
+  if (result != MOJO_RESULT_OK)
+    LOG(FATAL) << "Pexe translator could not read pexe name";
+  pexe_file_name[path_size] = '\0';

[Mark Seaborn, 2015/10/20 21:32:40]

Couldn't path_size be sizeof(pexe_file_name)?  In that case, this would overrun
the buffer.

[Sean Klein, 2015/10/22 21:50:00]

Using "PATH_MAX + 1" to allow room for null terminator

+
+  // Now that we have the pexe file name, make an object file. This is necessary
+  // for the callback into LLVM through 'funcs'. Open the object file, and
+  // prepare to pass the nexe through the compiler callback.
+  base::FilePath obj_file_name;
+  if (!CreateTemporaryFile(&obj_file_name))
+    LOG(FATAL) << "Could not make temporary object file";
+  int obj_file_fds[] = { open(obj_file_name.value().c_str(), O_RDWR, O_TRUNC) };

[Mark Seaborn, 2015/10/20 21:32:40]

This looks a bit quirky. :-)

You could just do:
int obj_file_fd = open(...);

and use &obj_file_fd as the array pointer below.

However, why are you using just one object file?  This will basically turn off
multi-threaded translation for pnacl-llc, which needs one object file per
thread.

You could add a TODO to use multiple object files, but maybe it wouldn't be hard
to implement that for this first version?

[Sean Klein, 2015/10/22 21:50:00]

The "quirky" version was there so the type of obj_file_fds would be the same
once multiple object files were added. Updated to the single "obj_file_fd"
version.

TODO included for multiple object files -- I will definitely get to that, since
we care about compilation speed, but it will involve changes across the mojom
interfaces, so I am going to defer it to a later change (as it is an
optimization).

+
+  if (obj_file_fds[0] < 0)
+    LOG(FATAL) << "Could not create temp file for compiled pexe";
+
+  // TODO(smklein): Is there a less arbitrary number to choose?

[Mark Seaborn, 2015/10/20 21:32:40]

What is Chromium using for the thread count?

This shouldn't be more than the number of CPUs.

[Sean Klein, 2015/10/22 21:50:00]

It's passed in as an argument, so I'm not entirely sure:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/nacl_irt/irt...

But I know it is in the range of 1 to 16 threads.

+  uint32_t num_threads = 8;
+  size_t obj_file_fd_count = 1;
+  char relocation_model[] = "-relocation-model=pic";
+  char force_tls[] = "-force-tls-non-pic";
+  char bitcode_format[] = "-bitcode-format=pnacl";
+  char *args[] = { relocation_model, force_tls, bitcode_format };
+  size_t argc = 3;
+  funcs->init_callback(num_threads, obj_file_fds, obj_file_fd_count,
+                       args, argc);
+
+  // Read the pexe using fread, and write the pexe into the callback function.
+  char buf[0x100000];

[Mark Seaborn, 2015/10/20 21:32:40]

That's a lot to stack-allocate.  Can you heap-allocate it instead?

[Sean Klein, 2015/10/22 21:50:00]

Done with new / delete[].

+  FILE* pexe_file_stream = fopen(pexe_file_name, "r");
+  // Once the pexe has been opened, it is no longer needed, so we unlink it.
+  if (unlink(pexe_file_name))
+    LOG(FATAL) << "Could not unlink temporary pexe file";
+  if (pexe_file_stream == nullptr)
+    LOG(FATAL) << "Could not open pexe for reading";
+  for(;;) {
+    size_t num_bytes_from_pexe = fread(buf, 1, sizeof(buf), pexe_file_stream);
+    if (ferror(pexe_file_stream)) {
+      LOG(FATAL) << "Error reading from pexe file stream";
+    }
+    if (num_bytes_from_pexe == 0) {
+      break;
+    }
+    funcs->data_callback(buf, num_bytes_from_pexe);
+  }
+
+  if (fclose(pexe_file_stream))
+    LOG(FATAL) << "Failed to close pexe file stream from compiler nexe";
+  funcs->end_callback();
+
+  // Write the name of the object file back to the content handler
+  // to signify that we are done compiling (and to provide information
+  // for the linking stage).
+  result = MojoWriteMessage(handle,
+                            obj_file_name.value().c_str(),
+                            obj_file_name.value().size(),
+                            nullptr,
+                            0,
+                            MOJO_WRITE_MESSAGE_FLAG_NONE);
+  if (result != MOJO_RESULT_OK)
+    LOG(FATAL) << "Could not write message to content handler: " << result;
+}
+
+const struct nacl_irt_private_pnacl_translator_compile
+    nacl_irt_private_pnacl_translator_compile = {
+  serve_translate_request
+};