Fix the NaCl bitstream reader to report fatal errors.

include/llvm/Bitcode/NaCl/NaClBitcodeParser.h

 //===- NaClBitcodeParser.h -----------------------------------*- C++ -*-===//
 //     Low-level bitcode driver to parse PNaCl bitcode files.
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
@@ skipping 395 matching lines @@
   /// The block ID to use if a global abbreviation. Note: This field is
   /// updated by calls to method SetBID.
   unsigned GlobalBlockID;
 };
 
 /// Parses a block in the PNaCl bitcode stream.
 class NaClBitcodeParser {
   // Allow listener privledges, so that it can update/call the parser
   // using a clean API.
   friend class NaClBitcodeParserListener;
+
+  // Implements an error handler for errors in the bitstream reader.
+  // Redirects bitstream reader errors to corresponding parrser error
+  // reporting function.
+  class ErrorHandler : public NaClBitstreamCursor::ErrorHandler {
+    NaClBitcodeParser *Parser;
+  public:
+    ErrorHandler(NaClBitcodeParser *Parser,
+                 NaClBitstreamCursor &Cursor):
+        NaClBitstreamCursor::ErrorHandler(Cursor), Parser(Parser) {}
+    LLVM_ATTRIBUTE_NORETURN
+    void Fatal(const std::string &ErrorMessage) const final {
+      Parser->FatalAt(getCurrentBitNo(), ErrorMessage);
+    }
+    ~ErrorHandler() override {}
+  };
+
 public:
-
   // Creates a parser to parse the the block at the given cursor in
   // the PNaCl bitcode stream. This instance is a "dummy" instance
   // that starts the parser.
   explicit NaClBitcodeParser(NaClBitstreamCursor &Cursor)
       : EnclosingParser(0),
         Block(ILLEGAL_BLOCK_ID, Cursor),
         Record(Block),
         Listener(0),
-        ErrStream(&errs())
-  {}
+        ErrStream(&errs()) {
+    std::unique_ptr<NaClBitstreamCursor::ErrorHandler>
+        ErrHandler(new ErrorHandler(this, Cursor));
+    Cursor.setErrorHandler(ErrHandler);

[jvoung (off chromium), 2015/02/23 18:58:56]

Does "Cursor.setErrorHandler(make_unique<ErrorHandler>(this, Cursor));" or
something like that work (from STLExtras.h)?

[Karl, 2015/02/23 20:46:22]

The problem is that there is two different types, and no conversion for
unique_ptr:
   NaClBitstreamCursor::ErrorHandler
   NaClBitcodeParser::ErrorHandler

+  }
 
   virtual ~NaClBitcodeParser();
 
   /// Reads the (top-level) block associated with the given block
   /// record at the stream cursor. Returns true if unable to parse.
   /// Can be called multiple times to parse multiple blocks.
   bool Parse();
 
   // Called once the bitstream reader has entered the corresponding
   // subblock.  Argument NumWords is set to the number of words in the
@@ skipping 38 matching lines @@
   // 2) Temporarily modify it for a single error message. In this context,
   //    the method Error() is overridden in the derived class, and
   //    calls this method twice. Once before calling base method Error(),
   //    and followed by a second call to restore the default error stream.
   raw_ostream &setErrStream(raw_ostream &Stream) {
     raw_ostream &OldErrStream = *ErrStream;
     ErrStream = &Stream;
     return OldErrStream;
   }
 
-  // Called when error occurs. Message is the error to report. Always
+  // Called when an error occurs. BitPosition is the bit position the
+  // error was found, and Message is the error to report. Always
   // returns true (the error return value of Parse).
-  virtual bool Error(const std::string &Message) {
-    *ErrStream << "Error: " << Message << "\n";
-    return true;
+  virtual bool ErrorAt(uint64_t BitPosition, const std::string &Message);
+
+  // Called when an error occurs. Message is the error to
+  // report. Always returns true (the error return value of Parse).
+  bool Error(const std::string &Message) {
+    // Overrides NaClBitcodeParser::Error and defines the reporting

[jvoung (off chromium), 2015/02/23 18:58:55]

Similar question to below Fatal() about "Overrides"

[Karl, 2015/02/23 20:46:22]

Not sure why this comment is here anymore. Removing.

+    // point to be the beginning of the current record being parsed,
+    // instead of at the end of the record.
+    return ErrorAt(Record.GetStartBit(), Message);
+  }
+
+  // Called when a fatal error occurs. BitPosition is the bit position
+  // the error was found, and Message is the error to report. Does not
+  // return.
+  LLVM_ATTRIBUTE_NORETURN
+  virtual void FatalAt(uint64_t BitPosition, const std::string &Message);
+
+  // Called when a fatal error occurs. Message is the error to
+  // report. Does not return.
+  LLVM_ATTRIBUTE_NORETURN
+  void Fatal(const std::string &Message) {
+    // Overrides NaClBitcodeParser::Fatal and defines the reporting

[jvoung (off chromium), 2015/02/23 18:58:55]

I don't quite understand the "Overrides NaClBitcodeParser::Fatal" comment.

Isn't this the definition of NaClBitcodeParser::Fatal method?

[Karl, 2015/02/23 20:46:22]

Also removed comment here.

+    // point to be the beginning of the current record being parsed,
+    // instead of at the end of the record.
+    FatalAt(Record.GetStartBit(), Message);
+  }
+
+  // Generates fatal generic error message.
+  LLVM_ATTRIBUTE_NORETURN
+  void Fatal() {
+    Fatal("Fatal error occurred!");
   }
 
   // Returns the number of bits in this block, including nested blocks.
   unsigned GetBlockNumBits() const {
     return Block.GetNumBits();
   }
 
   // Returns the number of bits in this block, excluding nested blocks.
   unsigned GetBlockLocalNumBits() const {
     return Block.GetLocalNumBits();
@@ skipping 92 matching lines @@
   }
 
   // Parses a BlockInfo block, where processing is handled through
   // a listener in the bitstream reader.
   bool ParseBlockInfoInternal();
 
   // Parses the non-BlockInfo block. Returns true if unable to parse the
   // block.
   bool ParseBlockInternal();
 
-
   void operator=(const NaClBitcodeParser &Parser) LLVM_DELETED_FUNCTION;
   NaClBitcodeParser(const NaClBitcodeParser &Parser) LLVM_DELETED_FUNCTION;
 
 };
 
 }  // namespace llvm
 
 #endif