First attempt to capture parser/translation errors in browser.

src/IceBrowserCompileServer.cpp

@@ -81,7 +81,8 @@ char *onEndCallback() {
   // TODO(jvoung): Also return an error string, and UMA data.
   // Set up a report_fatal_error handler to grab that string.
   if (gCompileServer->getErrorCode().value()) {
-    return strdup("Some error occurred");
+    const std::string Error = gCompileServer->getErrorStream().getContents();
+    return strdup(Error.empty() ? "Some error occurred" : Error.c_str());
   }
   return nullptr;
 }
@@ -142,9 +143,12 @@ void BrowserCompileServer::startCompileThread(int ObjFD) {
   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(),
-                              ELFStream.get(), Flags));
+                              &ErrorStream->getStream(), ELFStream.get(),
+                              Flags));
   CompileThread = std::thread([this]() {
     Ctx->initParserThread();
     this->getCompiler().run(ExtraFlags, *Ctx.get(),

src/IceBrowserCompileServer.h

@@ -39,6 +39,7 @@ class BrowserCompileServer : public CompileServer {
   BrowserCompileServer() = delete;
   BrowserCompileServer(const BrowserCompileServer &) = delete;
   BrowserCompileServer &operator=(const BrowserCompileServer &) = delete;
+  class StringStream;
 
 public:
   explicit BrowserCompileServer(Compiler &Comp)
@@ -74,7 +75,20 @@ public:
     ELFStream.reset(nullptr);
   }
 
+  StringStream &getErrorStream() {
+    return *ErrorStream;
+  }
+
 private:
+  class StringStream {
+  public:
+    StringStream() : StrBuf(Buffer) {}
+    const IceString &getContents() { return StrBuf.str(); }
+    Ostream &getStream() { return StrBuf; }
+  private:
+    std::string Buffer;
+    llvm::raw_string_ostream StrBuf;
+  };
   // This currently only handles a single compile request, hence one copy
   // of the state.
   std::unique_ptr<GlobalContext> Ctx;
@@ -84,6 +98,7 @@ private:
   llvm::QueueStreamer *InputStream;
   std::unique_ptr<Ostream> LogStream;
   std::unique_ptr<llvm::raw_fd_ostream> EmitStream;
+  std::unique_ptr<StringStream> ErrorStream;
   std::unique_ptr<ELFStreamer> ELFStream;
   ClFlags Flags;
   ClFlagsExtra ExtraFlags;

src/IceCompileServer.cpp

@@ -96,7 +96,8 @@ void CLCompileServer::run() {
     return transferErrorCode(getReturnValue(ExtraFlags, Ice::EC_Bitcode));
   }
 
-  Ctx.reset(new GlobalContext(Ls.get(), Os.get(), ELFStr.get(), Flags));
+  Ctx.reset(new GlobalContext(Ls.get(), Os.get(), Ls.get(), ELFStr.get(),
+                              Flags));
   if (Ctx->getFlags().getNumTranslationThreads() != 0) {
     std::thread CompileThread([this, &ExtraFlags, &InputStream]() {
       Ctx->initParserThread();

src/IceGlobalContext.cpp

@@ -213,14 +213,18 @@ void GlobalContext::CodeStats::dump(const IceString &Name, Ostream &Str) {
   Str << "\n";
 }
 
-GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit,
+GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
                              ELFStreamer *ELFStr, const ClFlags &Flags)
     : ConstPool(new ConstantPool()), ErrorStatus(), StrDump(OsDump),
-      StrEmit(OsEmit), Flags(Flags), RNG(Flags.getRandomSeed()), ObjectWriter(),
+      StrEmit(OsEmit), StrError(OsError), Flags(Flags),
+      RNG(Flags.getRandomSeed()), ObjectWriter(),
       OptQ(/*Sequential=*/Flags.isSequential(),
            /*MaxSize=*/Flags.getNumTranslationThreads()),
       // EmitQ is allowed unlimited size.
       EmitQ(/*Sequential=*/Flags.isSequential()) {
+  assert(OsDump && "OsDump is not defined for GlobalContext");
+  assert(OsEmit && "OsEmit is not defined for GlobalContext");
+  assert(OsError && "OsError is not defined for GlobalContext");
   // Make sure thread_local fields are properly initialized before any
   // accesses are made.  Do this here instead of at the start of
   // main() so that all clients (e.g. unit tests) can benefit for
@@ -278,7 +282,7 @@ void GlobalContext::translateFunctions() {
     if (Func->hasError()) {
       getErrorStatus()->assign(EC_Translation);
       OstreamLocker L(this);
-      getStrDump() << "ICE translation error: " << Func->getFunctionName()
+      getStrError() << "ICE translation error: " << Func->getFunctionName()
                     << ": " << Func->getError() << "\n";
       Item = new EmitterWorkItem(Func->getSequenceNumber());
     } else {

src/IceGlobalContext.h

@@ -145,14 +145,17 @@ class GlobalContext {
   };
 
 public:
-  GlobalContext(Ostream *OsDump, Ostream *OsEmit, ELFStreamer *ELFStreamer,
-                const ClFlags &Flags);
+  // The dump stream is a log stream while emit is the stream code
+  // is emitted to. The error stream is strictly for logging errors.
+  GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
+                ELFStreamer *ELFStreamer, const ClFlags &Flags);
   ~GlobalContext();
 
-  // The dump and emit streams need to be used by only one thread at a
-  // time.  This is done by exclusively reserving the streams via
-  // lockStr() and unlockStr().  The OstreamLocker class can be used
-  // to conveniently manage this.
+  //
+  // The dump, error, and emit streams need to be used by only one
+  // thread at a time.  This is done by exclusively reserving the
+  // streams via lockStr() and unlockStr().  The OstreamLocker class
+  // can be used to conveniently manage this.
   //
   // The model is that a thread grabs the stream lock, then does an
   // arbitrary amount of work during which far-away callees may grab
@@ -163,6 +166,7 @@ public:
   void lockStr() { StrLock.lock(); }
   void unlockStr() { StrLock.unlock(); }
   Ostream &getStrDump() { return *StrDump; }
+  Ostream &getStrError() { return *StrError; }
   Ostream &getStrEmit() { return *StrEmit; }
 
   LockedPtr<ErrorCode> getErrorStatus() {
@@ -418,6 +422,7 @@ private:
   StrLockType StrLock;
   Ostream *StrDump; // Stream for dumping / diagnostics
   Ostream *StrEmit; // Stream for code emission
+  Ostream *StrError; // Stream for logging errors.
 
   ICE_CACHELINE_BOUNDARY;
 

src/PNaClTranslator.cpp

@@ -488,7 +488,7 @@ bool TopLevelParser::ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
   Ice::GlobalContext *Context = Translator.getContext();
   { // Lock while printing out error message.
     Ice::OstreamLocker L(Context);
-    raw_ostream &OldErrStream = setErrStream(Context->getStrDump());
+    raw_ostream &OldErrStream = setErrStream(Context->getStrError());
     NaClBitcodeParser::ErrorAt(Level, Bit, Message);
     setErrStream(OldErrStream);
   }

unittest/BitcodeMunge.cpp

@@ -38,7 +38,7 @@ bool IceTest::SubzeroBitcodeMunger::runTest(const char *TestName,
                                             size_t MungeSize) {
   const bool AddHeader = true;
   setupTest(TestName, Munges, MungeSize, AddHeader);
-  Ice::GlobalContext Ctx(DumpStream, DumpStream, nullptr, Flags);
+  Ice::GlobalContext Ctx(DumpStream, DumpStream, DumpStream, nullptr, Flags);
   Ice::PNaClTranslator Translator(&Ctx);
   Translator.translateBuffer(TestName, MungedInput.get());