Fix error reporting in the PNaCl bitcode reader.

lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h

 //===- NaClBitcodeReader.h ------------------------------------*- C++ -*-===//
 //     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 110 matching lines @@
   // Assigns Idx to the given value, overwriting the existing entry
   // and possibly modifying the type of the entry.
   void OverwriteValue(Value *V, unsigned Idx);
 };
 
 
 class NaClBitcodeReader : public GVMaterializer {
   NaClBitcodeHeader Header;  // Header fields of the PNaCl bitcode file.
   LLVMContext &Context;
   Module *TheModule;
+  bool Verbose;  // True if error messages should be printed to errs()
   PNaClAllowedIntrinsics AllowedIntrinsics;
   std::unique_ptr<MemoryBuffer> Buffer;
   std::unique_ptr<NaClBitstreamReader> StreamFile;
   NaClBitstreamCursor Stream;
   StreamingMemoryObject *LazyStreamer;
   uint64_t NextUnreadBit;
   bool SeenValueSymbolTable;
-
-  std::string ErrorString;
-
   std::vector<Type*> TypeList;
   NaClBitcodeReaderValueList ValueList;
 
   // Holds information about each BasicBlock in the function being read.
   struct BasicBlockInfo {
     // A basic block within the function being modeled.
     BasicBlock *BB;
     // The set of generated conversions.
     DenseMap<NaClBitcodeReaderCast, CastInst*> CastMap;
     // The set of generated conversions that were added for phi nodes,
@@ skipping 23 matching lines @@
   /// map contains info about where to find deferred function body in the
   /// stream.
   DenseMap<Function*, uint64_t> DeferredFunctionInfo;
 
   /// \brief True if we should only accept supported bitcode format.
   bool AcceptSupportedBitcodeOnly;
 
   /// \brief Integer type use for PNaCl conversion of pointers.
   Type *IntPtrType;
 
+  static const std::error_category &BitcodeErrorCategory();
+
 public:
+
+  /// Types of errors reported.
+  enum ErrorType {
+    InvalidBBForInstruction,  // No basic block for instruction.

[jvoung (off chromium), 2014/12/01 23:23:50]

Is this the same as InvalidInstructionWithNoBB in the vanilla LLVM reader?

[Karl, 2014/12/03 18:32:09]

Yes. I didn't like the name. However, it probably is better to follow the
same convention. Changing to InvalidInstructionWithNoBB.

+    InvalidBitstream,         // Error in bitstream format.
+    InvalidConstantReference, // Bad constant reference.
+    InvalidMultipleBlocks, // Multiple blocks for a kind of block that should
+                           // have only one.
+    InvalidRecord, // Record doesn't have expected size or structure.
+    InvalidType,   // Invalid type in record.
+    InvalidTypeForValue, // Type of value incorrect.
+    InvalidValue,  // Invalid value in record.
+    MalformedBlock, // Unable to advance over block.
+    NoFunctionInStream, // Unable to find function in bitcode stream.

[jvoung (off chromium), 2014/12/01 23:23:50]

Is this the same as "CouldNotFindFunctionInStream" in the vanilla LLVM reader?
It might be good to keep the same name where possible.

[Karl, 2014/12/03 18:32:09]

Like above, changing to CouldNotFindFunctionInStream to be more consistent
with LLVM reader.

+  };
+
   explicit NaClBitcodeReader(MemoryBuffer *buffer, LLVMContext &C,
-                             bool AcceptSupportedOnly = true)
-      : Context(C), TheModule(0), AllowedIntrinsics(&C),
+                             bool Verbose,
+                             bool AcceptSupportedOnly)
+      : Context(C), TheModule(0), Verbose(Verbose), AllowedIntrinsics(&C),

[jvoung (off chromium), 2014/12/01 23:23:49]

nullptr instead of 0

[Karl, 2014/12/03 18:32:09]

Done.

         Buffer(buffer),
         LazyStreamer(0), NextUnreadBit(0), SeenValueSymbolTable(false),

[jvoung (off chromium), 2014/12/01 23:23:49]

LazyStreamer(nullptr)

[Karl, 2014/12/03 18:32:09]

Done.

         ValueList(),
         SeenFirstFunctionBody(false),
         AcceptSupportedBitcodeOnly(AcceptSupportedOnly),
         IntPtrType(IntegerType::get(C, PNaClIntPtrTypeBitSize)) {
   }
   explicit NaClBitcodeReader(StreamingMemoryObject *streamer,
                              LLVMContext &C,
-                             bool AcceptSupportedOnly = true)
-      : Context(C), TheModule(0), AllowedIntrinsics(&C),
+                             bool Verbose,
+                             bool AcceptSupportedOnly)
+      : Context(C), TheModule(0), Verbose(Verbose), AllowedIntrinsics(&C),

[jvoung (off chromium), 2014/12/01 23:23:50]

TheModule(nullptr), etc.

[Karl, 2014/12/03 18:32:09]

Done.

         Buffer(nullptr),
         LazyStreamer(streamer), NextUnreadBit(0), SeenValueSymbolTable(false),
         ValueList(),
         SeenFirstFunctionBody(false),
         AcceptSupportedBitcodeOnly(AcceptSupportedOnly),
         IntPtrType(IntegerType::get(C, PNaClIntPtrTypeBitSize)) {
   }
   ~NaClBitcodeReader() override {
     FreeState();
   }
 
   void FreeState();
 
   bool isMaterializable(const GlobalValue *GV) const override;
   bool isDematerializable(const GlobalValue *GV) const override;
   std::error_code Materialize(GlobalValue *GV) override;
   std::error_code MaterializeModule(Module *M) override;
   void Dematerialize(GlobalValue *GV) override;
   void releaseBuffer() override;
 
-  bool Error(const std::string &Str) {
-    ErrorString = Str;
-    return true;
+  std::error_code Error(ErrorType E) const {
+    return std::error_code(E, BitcodeErrorCategory());
   }
-  const std::string &getErrorString() const { return ErrorString; }
+
+  /// Generates the corresponding verbose Message, then generates error.
+  std::error_code Error(ErrorType E, const std::string &Message) const;
 
   /// @brief Main interface to parsing a bitcode buffer.
   /// @returns true if an error occurred.
-  bool ParseBitcodeInto(Module *M);
+  std::error_code ParseBitcodeInto(Module *M);
 
 private:
   // Returns false if Header is acceptable.
   bool AcceptHeader() const {
     return !(Header.IsSupported() ||
              (!AcceptSupportedBitcodeOnly && Header.IsReadable()));
   }
   uint32_t GetPNaClVersion() const {
     return Header.GetPNaClVersion();
   }
@@ skipping 59 matching lines @@
   Value *ConvertOpToType(Value *Op, Type *T, unsigned BBIndex);
 
   /// \brief If Op is a scalar value, this is a nop.  If Op is a
   /// pointer value, a PtrToInt instruction is inserted (in BBIndex)
   /// to convert Op to an integer.  For defaults on DeferInsertion,
   /// see comments for method CreateCast.
   Value *ConvertOpToScalar(Value *Op, unsigned BBIndex,
                            bool DeferInsertion = false);
 
   /// \brief Install instruction I into basic block BB.
-  bool InstallInstruction(BasicBlock *BB, Instruction *I);
+  std::error_code InstallInstruction(BasicBlock *BB, Instruction *I);
 
   FunctionType *AddPointerTypesToIntrinsicType(StringRef Name,
                                                FunctionType *FTy);
   void AddPointerTypesToIntrinsicParams();
-  bool ParseModule(bool Resume);
-  bool ParseTypeTable();
-  bool ParseTypeTableBody();
-  bool ParseGlobalVars();
-  bool ParseValueSymbolTable();
-  bool ParseConstants();
-  bool RememberAndSkipFunctionBody();
-  bool ParseFunctionBody(Function *F);
-  bool GlobalCleanup();
-  bool InitStream();
-  bool InitStreamFromBuffer();
-  bool InitLazyStream();
-  bool FindFunctionInStream(Function *F,
-         DenseMap<Function*, uint64_t>::iterator DeferredFunctionInfoIterator);
+  std::error_code ParseModule(bool Resume);
+  std::error_code ParseTypeTable();
+  std::error_code ParseTypeTableBody();
+  std::error_code ParseGlobalVars();
+  std::error_code ParseValueSymbolTable();
+  std::error_code ParseConstants();
+  std::error_code RememberAndSkipFunctionBody();
+  std::error_code ParseFunctionBody(Function *F);
+  std::error_code GlobalCleanup();
+  std::error_code InitStream();
+  std::error_code InitStreamFromBuffer();
+  std::error_code InitLazyStream();
+  std::error_code FindFunctionInStream(
+      Function *F,
+      DenseMap<Function*, uint64_t>::iterator DeferredFunctionInfoIterator);
 };
 
 } // End llvm namespace
 

[JF, 2014/12/02 00:49:04]

It's kind of ugly, but ReaderWriter.h also implements the following, which you
should probably do too:

namespace std {
template <> struct is_error_code_enum<llvm::BitcodeError> : std::true_type {};
}

[Karl, 2014/12/03 18:32:09]

Not in our local code base yet.

 #endif