Subzero: Allow overriding command-line args from the browser.

src/IceBrowserCompileServer.cpp

 //===- subzero/src/IceBrowserCompileServer.cpp - Browser compile server ---===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// \brief Defines the browser-based compile server.
 ///
 //===----------------------------------------------------------------------===//
 
 // Can only compile this with the NaCl compiler (needs irt.h, and the
 // unsandboxed LLVM build using the trusted compiler does not have irt.h).
 #include "IceBrowserCompileServer.h"
+#include "IceRangeSpec.h"
 
 #if PNACL_BROWSER_TRANSLATOR
 
 // Headers which are not properly part of the SDK are included by their path in
 // the NaCl tree.
 #ifdef __pnacl__
 #include "native_client/src/untrusted/nacl/pnacl.h"
 #endif // __pnacl__
 
 #include "llvm/Support/QueueStreamer.h"
 
 #include <cstring>
+#include <fstream>
 #include <irt.h>
 #include <irt_dev.h>
 #include <pthread.h>
 #include <thread>
 
 namespace Ice {
 
 // Create C wrappers around callback handlers for the IRT interface.
 namespace {
 
 BrowserCompileServer *gCompileServer;
 struct nacl_irt_private_pnacl_translator_compile gIRTFuncs;
 
 void getIRTInterfaces() {
   size_t QueryResult =
       nacl_interface_query(NACL_IRT_PRIVATE_PNACL_TRANSLATOR_COMPILE_v0_1,
                            &gIRTFuncs, sizeof(gIRTFuncs));
   if (QueryResult != sizeof(gIRTFuncs))
     llvm::report_fatal_error("Failed to get translator compile IRT interface");
 }
 
+// Allow pnacl-sz arguments to be supplied externally, instead of coming from
+// the browser.  This is meant to be used for debugging.
+//
+// If the SZARGFILE environment variable is set to a file name, arguments are
+// read from that file, one argument per line.  This requires setting 3
+// environment variables before starting the browser:
+//
+// NACL_ENV_PASSTHROUGH=NACL_DANGEROUS_ENABLE_FILE_ACCESS,NACLENV_SZARGFILE
+// NACL_DANGEROUS_ENABLE_FILE_ACCESS=1
+// NACLENV_SZARGFILE=/path/to/myargs.txt
+//
+// In addition, Chrome needs to be launched with the "--no-sandbox" argument.
+//
+// If the SZARGLIST environment variable is set, arguments are extracted from
+// that variable's value, separated by the '|' character (being careful to
+// escape/quote special shell characters).  This requires setting 2 environment
+// variables before starting the browser:
+//
+// NACL_ENV_PASSTHROUGH=NACLENV_SZARGLIST
+// NACLENV_SZARGLIST=arg
+//
+// This does not require the "--no-sandbox" argument, and is therefore much
+// safer, but does require restarting the browser to change the arguments.
+//
+// If external arguments are supplied, the browser's NumThreads specification is
+// ignored, to allow -threads to be specified as an external argument.  Note
+// that the browser normally supplies the "-O2" argument, so externally supplied
+// arguments might want to provide an explicit -O argument.
+std::vector<std::string> getExternalArgs() {
+  std::vector<std::string> ExternalArgs;

[John, 2016/04/20 14:17:24]

I still think this should be disallowed in a release browser build. I'd be
scared to release a version of sz that accepts arguments not coming from the
browser

[Jim Stichnoth, 2016/04/20 16:53:54]

I originally felt this way.

Then I changed my mind - wouldn't it be great if we could enable remote
debugging/diagnosis by having the user run with some SZARGLIST?

But now I realize that the MINIMAL build is basically entirely lacking in any
diagnostic output, and we don't really want to change that without good reason,
and besides we can almost always act as the end user for the purpose of
diagnosis, and we'll just drop in a non-MINIMAL translator for that purpose.

TL;DR: Done.

+  char ArgsFileVar[] = "SZARGFILE";

[John, 2016/04/20 14:17:24]

const

[Jim Stichnoth, 2016/04/20 16:53:54]

Sadly, archaic getenv() was apparently invented before "const" was discovered.

I.e., I must pass a non-const char* argument.  :(

+  char ArgsListVar[] = "SZARGLIST";
+  if (const char *ArgsFilename = getenv(ArgsFileVar)) {
+    std::ifstream ArgsStream(ArgsFilename);
+    std::string Arg;
+    while (ArgsStream >> std::ws, std::getline(ArgsStream, Arg)) {
+      if (!Arg.empty() && Arg[0] == '#')
+        continue;
+      ExternalArgs.emplace_back(Arg);
+    }
+    if (ExternalArgs.empty()) {
+      llvm::report_fatal_error("Failed to read arguments from file '" +
+                               std::string(ArgsFilename) + "'");
+    }
+  } else if (const char *ArgsList = getenv(ArgsListVar)) {
+    // Leverage the RangeSpec tokenizer.
+    auto Args = RangeSpec::tokenize(ArgsList, '|');
+    ExternalArgs.insert(ExternalArgs.end(), Args.begin(), Args.end());
+  }
+  return ExternalArgs;
+}
+
 char *onInitCallback(uint32_t NumThreads, int *ObjFileFDs,
                      size_t ObjFileFDCount, char **CLArgs, size_t CLArgsLen) {
   if (ObjFileFDCount < 1) {
     std::string Buffer;
     llvm::raw_string_ostream StrBuf(Buffer);
     StrBuf << "Invalid number of FDs for onInitCallback " << ObjFileFDCount
            << "\n";
     return strdup(StrBuf.str().c_str());
   }
   int ObjFileFD = ObjFileFDs[0];
   if (ObjFileFD < 0) {
     std::string Buffer;
     llvm::raw_string_ostream StrBuf(Buffer);
     StrBuf << "Invalid FD given for onInitCallback " << ObjFileFD << "\n";
     return strdup(StrBuf.str().c_str());
   }
   // CLArgs is almost an "argv", but is missing the argv[0] program name.
-  std::vector<char *> Argv;
-  char ProgramName[] = "pnacl-sz.nexe";
+  std::vector<const char *> Argv;
+  constexpr static char ProgramName[] = "pnacl-sz.nexe";
   Argv.reserve(CLArgsLen + 1);
   Argv.push_back(ProgramName);
-  for (size_t i = 0; i < CLArgsLen; ++i) {
-    Argv.push_back(CLArgs[i]);
+
+  bool UseNumThreadsFromBrowser = true;
+  auto ExternalArgs = getExternalArgs();
+  if (ExternalArgs.empty()) {
+    for (size_t i = 0; i < CLArgsLen; ++i) {
+      Argv.push_back(CLArgs[i]);
+    }
+  } else {
+    for (auto &Arg : ExternalArgs) {
+      Argv.emplace_back(Arg.c_str());
+    }
+    UseNumThreadsFromBrowser = false;
   }
   // NOTE: strings pointed to by argv are owned by the caller, but we parse
   // here before returning and don't store them.
-  gCompileServer->getParsedFlags(NumThreads, Argv.size(), Argv.data());
-  gCompileServer->startCompileThread(ObjFileFD);
+  gCompileServer->getParsedFlags(UseNumThreadsFromBrowser, NumThreads,
+                                 Argv.size(), Argv.data());
+  int LogFD = STDOUT_FILENO;
+  if (getFlags().getLogFilename() == "/dev/stderr") {
+    LogFD = STDERR_FILENO;
+  } else if (getFlags().getLogFilename() != "-") {
+    llvm::report_fatal_error(
+        "Log file name must be either '-' or '/dev/stderr'");

[John, 2016/04/20 14:17:24]

Can the user change this option?? It seems like an invalid argument should lead
to a warning, not crash sz

[Jim Stichnoth, 2016/04/20 16:53:54]

Done.  Pushed the logic down into startCompileThread() so that we can log a
warning to the log stream after the log stream is created.

+  }
+  gCompileServer->startCompileThread(ObjFileFD, LogFD);
   return nullptr;
 }
 
 int onDataCallback(const void *Data, size_t NumBytes) {
   return gCompileServer->pushInputBytes(Data, NumBytes) ? 1 : 0;
 }
 
 char *onEndCallback() {
   gCompileServer->endInputStream();
   gCompileServer->waitForCompileThread();
@@ skipping 66 matching lines @@
 } // end of anonymous namespace
 
 BrowserCompileServer::~BrowserCompileServer() = default;
 
 void BrowserCompileServer::run() {
   gCompileServer = this;
   getIRTInterfaces();
   gIRTFuncs.serve_translate_request(&SubzeroCallbacks);
 }
 
-void BrowserCompileServer::getParsedFlags(uint32_t NumThreads, int argc,
-                                          char **argv) {
+void BrowserCompileServer::getParsedFlags(bool UseNumThreadsFromBrowser,
+                                          uint32_t NumThreads, int argc,
+                                          const char *const *argv) {
   ClFlags::parseFlags(argc, argv);
   ClFlags::getParsedClFlags(ClFlags::Flags);
   // Set some defaults which aren't specified via the argv string.
-  ClFlags::Flags.setNumTranslationThreads(NumThreads);
+  if (UseNumThreadsFromBrowser)
+    ClFlags::Flags.setNumTranslationThreads(NumThreads);
   ClFlags::Flags.setUseSandboxing(true);
   ClFlags::Flags.setOutFileType(FT_Elf);
   ClFlags::Flags.setTargetArch(getTargetArch());
   ClFlags::Flags.setInputFileFormat(llvm::PNaClFormat);
 }
 
 bool BrowserCompileServer::pushInputBytes(const void *Data, size_t NumBytes) {
   // If there was an earlier error, do not attempt to push bytes to the
   // QueueStreamer. Otherwise the thread could become blocked.
   if (HadError.load())
@@ skipping 16 matching lines @@
     // HadError means report_fatal_error is called. Make sure that the
     // LastError is not EC_None. We don't know the type of error so just pick
     // some error category.
     LastError.assign(EC_Translation);
   }
   return LastError;
 }
 
 void BrowserCompileServer::endInputStream() { InputStream->SetDone(); }
 
-void BrowserCompileServer::startCompileThread(int ObjFD) {
+void BrowserCompileServer::startCompileThread(int ObjFD, int LogFD) {
   InputStream = new llvm::QueueStreamer();
-  LogStream = getOutputStream(STDOUT_FILENO);
+  LogStream = getOutputStream(LogFD);
   LogStream->SetUnbuffered();
   EmitStream = getOutputStream(ObjFD);
   EmitStream->SetBufferSize(1 << 14);
   std::unique_ptr<StringStream> ErrStrm(new StringStream());
   ErrorStream = std::move(ErrStrm);
   ELFStream.reset(new ELFStreamer(*EmitStream.get()));
   Ctx.reset(new GlobalContext(LogStream.get(), EmitStream.get(),
                               &ErrorStream->getStream(), ELFStream.get()));
   CompileThread = std::thread([this]() {
     llvm::install_fatal_error_handler(fatalErrorHandler, this);
@@ skipping 19 matching lines @@
   llvm::report_fatal_error("no browser hookups");
 }
 
 ErrorCode &BrowserCompileServer::getErrorCode() {
   llvm::report_fatal_error("no browser hookups");
 }
 
 } // end of namespace Ice
 
 #endif // PNACL_BROWSER_TRANSLATOR