Report fatal translator errors to the browser

Install a fatal error handler for the translator, which stores the error
string, signals an error to the RPC thread, and terminates the thread
(instead of terminating the whole program). This will cause the error to
go to the Javascript console in Chrome

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3519
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/23753003

7 files changed, 93 insertions, 35 deletions

diff --git a/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h b/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
index 4f5f83a17a..214971e701 100644
--- a/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
+++ b/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_BITCODE_NACL_NACLBITCODEHEADER_H
 #define LLVM_BITCODE_NACL_NACLBITCODEHEADER_H
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
 #include <string>
@@ -148,7 +149,7 @@ public:
   ~NaClBitcodeHeader();
 
   /// \brief Installs the fields of the header, defining if the header
-  /// is readable and supported.
+  /// is readable and supported. Sets UnsupportedMessage on failure.
   void InstallFields();
 
   /// \brief Adds a field to the list of fields in a header. Takes ownership
@@ -209,15 +210,21 @@ private:
   // Reads and verifies the first 8 bytes of the header, consisting
   // of the magic number 'PEXE', and the value defining the number
   // of fields and number of bytes used to hold fields.
-  // Returns false if successful.
+  // Returns false if successful, sets UnsupportedMessage otherwise.
   bool ReadPrefix(const unsigned char *BufPtr, const unsigned char *BufEnd,
                   unsigned &NumFields, unsigned &NumBytes);
 
   // Reads and verifies the fields in the header.
-  // Returns false if successful.
+  // Returns false if successful, sets UnsupportedMessage otherwise.
   bool ReadFields(const unsigned char *BufPtr, const unsigned char *BufEnd,
                   unsigned NumFields, unsigned NumBytes);
 
+  // Sets the Unsupported error message and returns true.
+  bool UnsupportedError(StringRef Message) {
+    UnsupportedMessage = Message.str();
+    return true;
+  }
+
 };
 
 } // namespace llvm
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
index 06f346e331..dfab1bb10e 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
@@ -10,6 +10,7 @@
 
 #include "llvm/Bitcode/NaCl/NaClBitcodeHeader.h"
 #include "llvm/Bitcode/NaCl/NaClReaderWriter.h"
+#include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
@@ -118,7 +119,7 @@ std::string NaClBitcodeHeaderField::Contents() const {
     ss << "]";
     break;
   default:
-    report_fatal_error("PNaCL bitcode file contains unknown field type");
+    report_fatal_error("PNaCl bitcode file contains unknown field type");
   }
   return ss.str();
 }
@@ -140,13 +141,19 @@ bool NaClBitcodeHeader::ReadPrefix(const unsigned char *BufPtr,
                                    const unsigned char *BufEnd,
                                    unsigned &NumFields, unsigned &NumBytes) {
   // Must contain PEXE.
-  if (!isNaClBitcode(BufPtr, BufEnd))
+  if (!isNaClBitcode(BufPtr, BufEnd)) {
+    UnsupportedMessage = "Invalid PNaCl bitcode header";
+    if (isBitcode(BufPtr, BufEnd)) {
+      UnsupportedMessage += " (to run in Chrome, bitcode files must be "
+          "finalized using pnacl-finalize)";
+    }
     return true;
+  }
   BufPtr += WordSize;
 
   // Read #Fields and number of bytes needed for the header.
   if (BufPtr + WordSize > BufEnd)
-    return true;
+    return UnsupportedError("Bitcode read failure");
   NumFields = static_cast<unsigned>(BufPtr[0]) |
       (static_cast<unsigned>(BufPtr[1]) << 8);
   NumBytes = static_cast<unsigned>(BufPtr[2]) |
@@ -165,7 +172,7 @@ bool NaClBitcodeHeader::ReadFields(const unsigned char *BufPtr,
     NaClBitcodeHeaderField *Field = new NaClBitcodeHeaderField();
     Fields.push_back(Field);
     if (!Field->Read(BufPtr, BufEnd - BufPtr))
-      return true;
+      return UnsupportedError("Bitcode read failure");
     size_t FieldSize = Field->GetTotalSize();
     BufPtr += FieldSize;
   }
@@ -177,11 +184,11 @@ bool NaClBitcodeHeader::Read(const unsigned char *&BufPtr,
   unsigned NumFields;
   unsigned NumBytes;
   if (ReadPrefix(BufPtr, BufEnd, NumFields, NumBytes))
-    return true;
+    return true; // ReadPrefix sets UnsupportedMessage
   BufPtr += 2 * WordSize;
 
   if (ReadFields(BufPtr, BufEnd, NumFields, NumBytes))
-    return true;
+    return true; // ReadFields sets UnsupportedMessage
   BufPtr += NumBytes;
   InstallFields();
   return false;
@@ -192,9 +199,10 @@ bool NaClBitcodeHeader::Read(StreamableMemoryObject *Bytes) {
   unsigned NumBytes;
   {
     unsigned char Buffer[2 * WordSize];
-    if (Bytes->readBytes(0, sizeof(Buffer), Buffer, NULL) ||
-        ReadPrefix(Buffer, Buffer + sizeof(Buffer), NumFields, NumBytes))
-      return true;
+    if (Bytes->readBytes(0, sizeof(Buffer), Buffer, NULL))
+      return UnsupportedError("Bitcode read failure");
+    if (ReadPrefix(Buffer, Buffer + sizeof(Buffer), NumFields, NumBytes))
+      return true; // ReadPrefix sets UnsupportedMessage
   }
   uint8_t *Header = new uint8_t[NumBytes];
   bool failed =
@@ -202,7 +210,7 @@ bool NaClBitcodeHeader::Read(StreamableMemoryObject *Bytes) {
       ReadFields(Header, Header + NumBytes, NumFields, NumBytes);
   delete[] Header;
   if (failed)
-    return true;
+    return UnsupportedError("Bitcode read failure");
   InstallFields();
   return false;
 }
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index 41acb6f999..e8ef0e6d9e 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -997,7 +997,7 @@ bool NaClBitcodeReader::ParseBitcodeInto(Module *M) {
   M->setDataLayout("e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-"
                    "f32:32:32-f64:64:64-p:32:32:32-v128:32:32");
 
-  if (InitStream()) return Error(Header.Unsupported());
+  if (InitStream()) return true; // InitSream will set the error string.
 
   // We expect a number of well-defined blocks, though we don't necessarily
   // need to understand them all.
@@ -1772,22 +1772,26 @@ bool NaClBitcodeReader::InitStreamFromBuffer() {
     return Error("Bitcode stream should be a multiple of 4 bytes in length");
 
   if (Header.Read(BufPtr, BufEnd))
-    return Error("Invalid PNaCl bitcode header");
+    return Error(Header.Unsupported());
 
   StreamFile.reset(new NaClBitstreamReader(BufPtr, BufEnd));
   Stream.init(*StreamFile);
 
-  return AcceptHeader();
+  if (AcceptHeader())
+    return Error(Header.Unsupported());
+  return false;
 }
 
 bool NaClBitcodeReader::InitLazyStream() {
   StreamingMemoryObject *Bytes = new StreamingMemoryObject(LazyStreamer);
   if (Header.Read(Bytes))
-    return Error("Invalid PNaCl bitcode header");
+    return Error(Header.Unsupported());
 
   StreamFile.reset(new NaClBitstreamReader(Bytes, Header.getHeaderSize()));
   Stream.init(*StreamFile);
-  return AcceptHeader();
+  if (AcceptHeader())
+    return Error(Header.Unsupported());
+  return false;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/tools/pnacl-llc/SRPCStreamer.cpp b/tools/pnacl-llc/SRPCStreamer.cpp
index e6546aef5f..ddec846d2c 100644
--- a/tools/pnacl-llc/SRPCStreamer.cpp
+++ b/tools/pnacl-llc/SRPCStreamer.cpp
@@ -119,19 +119,30 @@ llvm::DataStreamer *SRPCStreamer::init(void *(*Callback)(void *), void *arg,
 }
 
 size_t SRPCStreamer::gotChunk(unsigned char *bytes, size_t len) {
-  if (Error) return 0;
+  if (__sync_fetch_and_add(&Error, 0)) return 0; // Atomic read.
   return Q.PutBytes(bytes, len);
 }
 
 int SRPCStreamer::streamEnd(std::string *ErrMsg) {
   Q.SetDone();
   int err = pthread_join(CompileThread, NULL);
-  if (err) {
+  __sync_synchronize();
+  if (Error) {
+    if (ErrMsg)
+      *ErrMsg = std::string("PNaCl Translator Error: " + ErrorMessage);
+    return 1;
+  } else if (err) {
     if (ErrMsg) *ErrMsg = std::string(strerror(errno));
     return err;
   }
-  if (Error && ErrMsg) *ErrMsg = std::string("compile failed.");
-  return Error;
+  return 0;
+}
+
+void SRPCStreamer::setFatalError(const std::string& message) {
+  __sync_fetch_and_add(&Error, 1);
+  ErrorMessage = message;
+  __sync_synchronize();
+  pthread_exit(NULL);
 }
 
 #endif // __native_client__
diff --git a/tools/pnacl-llc/SRPCStreamer.h b/tools/pnacl-llc/SRPCStreamer.h
index 805552be1f..bde86e5d9f 100644
--- a/tools/pnacl-llc/SRPCStreamer.h
+++ b/tools/pnacl-llc/SRPCStreamer.h
@@ -98,11 +98,12 @@ public:
   size_t gotChunk(unsigned char *bytes, size_t len);
   // Called by the RPC thread. Wait for the compilation thread to finish.
   int streamEnd(std::string *ErrMsg);
-  // Called by the compilation thread. Signal that there was a compilation
-  // error so the RPC thread can abort the stream.
-  void setError() { Error = true; }
+  // Called by the compilation thread. Set the error condition and also
+  // terminate the thread.
+  void setFatalError(const std::string& message);
 private:
-  bool Error;
+  int Error;
+  std::string ErrorMessage;
   QueueStreamer Q;
   pthread_t CompileThread;
 };
diff --git a/tools/pnacl-llc/pnacl-llc.cpp b/tools/pnacl-llc/pnacl-llc.cpp
index 31d546ab3e..cac4fa83a4 100644
--- a/tools/pnacl-llc/pnacl-llc.cpp
+++ b/tools/pnacl-llc/pnacl-llc.cpp
@@ -28,6 +28,7 @@
 #include "llvm/PassManager.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,6 +53,7 @@ using namespace llvm;
 int srpc_main(int argc, char **argv);
 int getObjectFileFD();
 DataStreamer *getNaClBitcodeStreamer();
+fatal_error_handler_t getSRPCErrorHandler();
 #endif
 
 cl::opt<NaClFileFormat>
@@ -215,6 +217,10 @@ int llc_main(int argc, char **argv) {
   LLVMContext &Context = getGlobalContext();
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
+#if defined(__native_client__)
+  install_fatal_error_handler(getSRPCErrorHandler(), NULL);
+#endif
+
   // Initialize targets first, so that --version shows registered targets.
   InitializeAllTargets();
   InitializeAllTargetMCs();
@@ -252,11 +258,15 @@ int llc_main(int argc, char **argv) {
 static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter,
                                  const Twine &Name) {
   if (PNaClABIVerify && Reporter.getErrorCount() > 0) {
-    errs() << (PNaClABIVerifyFatalErrors ? "ERROR: " : "WARNING: ");
-    errs() << Name << " is not valid PNaCl bitcode:\n";
-    Reporter.printErrors(errs());
-    if (PNaClABIVerifyFatalErrors)
-      exit(1);
+    std::string errors;
+    raw_string_ostream os(errors);
+    os << (PNaClABIVerifyFatalErrors ? "ERROR: " : "WARNING: ");
+    os << Name << " is not valid PNaCl bitcode:\n";
+    Reporter.printErrors(os);
+    if (PNaClABIVerifyFatalErrors) {
+      report_fatal_error(os.str());
+    }
+    errs() << os.str();
   }
   Reporter.reset();
 }
@@ -273,11 +283,12 @@ static int compileModule(StringRef ProgramName, LLVMContext &Context) {
 #if defined(__native_client__)
   if (LazyBitcode) {
     std::string StrError;
+    std::string DisplayFilename("<PNaCl-translated pexe>");
     M.reset(getNaClStreamedBitcodeModule(
-        std::string("<SRPC stream>"),
+        DisplayFilename,
         getNaClBitcodeStreamer(), Context, &StrError));
     if (!StrError.empty())
-      Err = SMDiagnostic(InputFilename, SourceMgr::DK_Error, StrError);
+      Err = SMDiagnostic(DisplayFilename, SourceMgr::DK_Error, StrError);
   } else {
     llvm_unreachable("native client SRPC only supports streaming");
   }
@@ -287,8 +298,13 @@ static int compileModule(StringRef ProgramName, LLVMContext &Context) {
 
   mod = M.get();
   if (mod == 0) {
+#if defined(__native_client__)
+    report_fatal_error(Err.getMessage());
+#else
+    // Err.print is prettier, so use it for the non-sandboxed translator.
     Err.print(ProgramName.data(), errs());
     return 1;
+#endif
   }
 
   if (PNaClABIVerify) {
diff --git a/tools/pnacl-llc/srpc_main.cpp b/tools/pnacl-llc/srpc_main.cpp
index 87178c2038..46d79ce441 100644
--- a/tools/pnacl-llc/srpc_main.cpp
+++ b/tools/pnacl-llc/srpc_main.cpp
@@ -32,6 +32,7 @@
 #include "llvm/ADT/OwningPtr.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/Option.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/system_error.h"
 
 using namespace llvm;
@@ -202,8 +203,10 @@ void *run_streamed(void *arg) {
   StreamingThreadData *data = reinterpret_cast<StreamingThreadData *>(arg);
   data->CmdLineVec()->push_back("-streaming-bitcode");
   if (DoTranslate(data->CmdLineVec(), data->ObjectFD()) != 0) {
-    fprintf(stderr, "DoTranslate failed.\n");
-    srpc_streamer->setError();
+    // llc_main only returns 1 (as opposed to calling report_fatal_error)
+    // in conditions we never expect to see in the browser (e.g. bad
+    // command-line flags).
+    srpc_streamer->setFatalError("llc_main unspecified failure");
     return NULL;
   }
   delete data;
@@ -351,6 +354,14 @@ int getObjectFileFD() { return object_file_fd; }
 
 DataStreamer *getNaClBitcodeStreamer() { return NaClBitcodeStreamer; }
 
+// Called from the compilation thread
+void FatalErrorHandler(void *user_data, const std::string& reason,
+                       bool gen_crash_diag) {
+  srpc_streamer->setFatalError(reason);
+}
+
+fatal_error_handler_t getSRPCErrorHandler() { return FatalErrorHandler; }
+
 int srpc_main(int argc, char **argv) {
   if (!NaClSrpcModuleInit()) {
     return 1;