Report fatal translator errors to the browser

lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp

 //===- NaClBitcodeReader.cpp ----------------------------------------------===//
 //     Internal NaClBitcodeReader implementation
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
@@ skipping 968 matching lines @@
   // PNaCl does not support different DataLayouts in pexes, so we
   // implicitly set the DataLayout to the following default.
   //
   // This is not usually needed by the backend, but it might be used
   // by IR passes that the PNaCl translator runs.  We set this in the
   // reader rather than in pnacl-llc so that 'opt' will also use the
   // correct DataLayout if it is run on a pexe.
   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.
   while (1) {
     if (Stream.AtEndOfStream())
       return false;
 
     NaClBitstreamEntry Entry =
       Stream.advance(NaClBitstreamCursor::AF_DontAutoprocessAbbrevs);
 
@@ skipping 746 matching lines @@
 }
 
 bool NaClBitcodeReader::InitStreamFromBuffer() {
   const unsigned char *BufPtr = (const unsigned char*)Buffer->getBufferStart();
   const unsigned char *BufEnd = BufPtr+Buffer->getBufferSize();
 
   if (Buffer->getBufferSize() & 3)
     return Error("Bitcode stream should be a multiple of 4 bytes in length");
 
   if (Header.Read(BufPtr, BufEnd))
     return Error("Invalid PNaCl bitcode header");

[jvoung (off chromium), 2013/09/10 20:39:56]

A bit difficult to figure out which error message gets through.

Doesn't this early return of Error("...") mean that we never get to the
Header.Unsupported() call on line 1762, so the UnsupportedMessage that was set
from NaClBitcodeHeader::ReadPrefix/Read won't be read?

[Derek Schuff, 2013/09/13 19:53:36]

Agreed.
I added some comments and made the error handling a bit more explicit in the
bitcode reader and header.

 
   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");
 
   StreamFile.reset(new NaClBitstreamReader(Bytes, Header.getHeaderSize()));
   Stream.init(*StreamFile);
-  return AcceptHeader();
+  if (AcceptHeader())
+    return Error(Header.Unsupported());
+  return false;
 }
 
 //===----------------------------------------------------------------------===//
 // External interface
 //===----------------------------------------------------------------------===//
 
 /// getNaClLazyBitcodeModule - lazy function-at-a-time loading from a file.
 ///
 Module *llvm::getNaClLazyBitcodeModule(MemoryBuffer *Buffer,
                                        LLVMContext& Context,
@@ skipping 54 matching lines @@
   if (M->MaterializeAllPermanently(ErrMsg)) {
     delete M;
     return 0;
   }
 
   // TODO: Restore the use-lists to the in-memory state when the bitcode was
   // written.  We must defer until the Module has been fully materialized.
 
   return M;
 }