Fix error reporting in the PNaCl bitcode reader.

include/llvm/Bitcode/NaCl/NaClReaderWriter.h

 //===-- llvm/Bitcode/NaCl/NaClReaderWriter.h - ------------------*- C++ -*-===//
 //      NaCl Bitcode reader/writer.
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This header defines interfaces to read and write NaCl bitcode wire format
 // files.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_BITCODE_NACL_NACLREADERWRITER_H
 #define LLVM_BITCODE_NACL_NACLREADERWRITER_H
 
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ErrorOr.h"
 
 #include <string>
 
 namespace llvm {
   class MemoryBuffer;
   class LLVMContext;
   class Module;
   class raw_ostream;
   class NaClBitcodeHeader;
   class NaClBitstreamWriter;
   class StreamingMemoryObject;
 
   /// Defines the data layout used for PNaCl bitcode files. We set the
   /// data layout of the module in the bitcode readers rather than in
   /// pnacl-llc so that 'opt' will also use the correct data layout if
   /// it is run on a pexe.
   extern const char *PNaClDataLayout;
 
   /// Allows (function) local symbol tables (unsupported) in PNaCl bitcode
   /// files.
   extern cl::opt<bool> PNaClAllowLocalSymbolTables;
 
   /// \brief Defines the integer bit size used to model pointers in PNaCl.
   static const unsigned PNaClIntPtrTypeBitSize = 32;
 
   /// getNaClLazyBitcodeModule - Read the header of the specified bitcode buffer
   /// and prepare for lazy deserialization of function bodies.  If successful,
   /// this takes ownership of 'buffer' and returns a non-null pointer.  On
-  /// error, this returns null, *does not* take ownership of Buffer, and fills
-  /// in *ErrMsg with an error description if ErrMsg is non-null.
+  /// error, this returns an error code and *does not* take ownership of Buffer.
+  ///
+  /// When VerboseErrors is true, more descriptive error messages are also
+  /// written to errs().
   ///
   /// The AcceptSupportedOnly argument is used to decide which PNaCl versions
   /// of the PNaCl bitcode to accept. There are three forms:
   ///    1) Readable and supported.
   ///    2) Readable and unsupported. Allows testing of code before becoming
   ///       supported, as well as running experiments on the bitcode format.
   ///    3) Unreadable.
   /// When AcceptSupportedOnly is true, only form 1 is allowed. When
   /// AcceptSupportedOnly is false, forms 1 and 2 are allowed.
-  Module *getNaClLazyBitcodeModule(MemoryBuffer *Buffer,
-                                   LLVMContext &Context,
-                                   std::string *ErrMsg = 0,
-                                   bool AcceptSupportedOnly = true);
+  ErrorOr<Module *> getNaClLazyBitcodeModule(MemoryBuffer *Buffer,
+                                             LLVMContext &Context,
+                                             bool VerboseErrors = false,
+                                             bool AcceptSupportedOnly = true);
 
   /// getNaClStreamedBitcodeModule - Read the header of the specified stream
   /// and prepare for lazy deserialization and streaming of function bodies.
   /// On error, this returns null, and fills in *ErrMsg with an error
   /// description if ErrMsg is non-null.
   ///
-  /// See getNaClLazyBitcodeModule for an explanation of argument
-  /// AcceptSupportedOnly.
+  /// See getNaClLazyBitcodeModule for an explanation of arguments
+  /// VerboseErrors, AcceptSupportedOnly.
   Module *getNaClStreamedBitcodeModule(const std::string &name,

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

Interesting... Didn't realize that upstream this isn't also an ErrorOr<Module
*>. I wonder why it is a different interface?

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

I noticed it too. However, since it wasn't changed on the LLVM side, I decided
not to change the NaCl side in this CL. Rather, since this CL is already large,
put that off to a separate CL. Added a TODO to make this clear.

                                        StreamingMemoryObject *streamer,
                                        LLVMContext &Context,
+                                       bool VerboseErrors = false,
                                        std::string *ErrMsg = 0,

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

nullptr

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

Done.

                                        bool AcceptSupportedOnly = true);
 
   /// NaClParseBitcodeFile - Read the specified bitcode file,
-  /// returning the module.  If an error occurs, this returns null and
-  /// fills in *ErrMsg if it is non-null.  This method *never* takes
-  /// ownership of Buffer.
+  /// returning the module.  If an error_code if error occurs.  This

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

This line doesn't make sense.

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

Cleaned this up.

+  /// method *never* takes ownership of Buffer.
   ///
-  /// See getNaClLazyBitcodeModule for an explanation of argument
-  /// AcceptSupportedOnly.
-  Module *NaClParseBitcodeFile(MemoryBuffer *Buffer, LLVMContext &Context,
-                               std::string *ErrMsg = 0,
-                               bool AcceptSupportedOnly = true);
+  /// See getNaClLazyBitcodeModule for an explanation of arguments
+  /// VerboseErrors, AcceptSupportedOnly.
+  ErrorOr<Module *> NaClParseBitcodeFile(MemoryBuffer *Buffer,
+                                         LLVMContext &Context,
+                                         bool VerboseErrors = false,
+                                         bool AcceptSupportedOnly = true);
 
   /// NaClWriteBitcodeToFile - Write the specified module to the
   /// specified raw output stream, using PNaCl wire format.  For
   /// streams where it matters, the given stream should be in "binary"
   /// mode.
   ///
-  /// The AcceptSupportedOnly argument is used to decide which PNaCl versions
-  /// of the PNaCl bitcode to generate. There are two forms:
-  ///    1) Writable and supported.
-  ///    2) Writable and unsupported. Allows testing of code before becoming
-  ///       supported, as well as running experiments on the bitcode format.
-  /// When AcceptSupportedOnly is true, only form 1 is allowed. When
-  /// AcceptSupportedOnly is false, forms 1 and 2 are allowed.
+  /// See getNaClLazyBitcodeModule for an explanation of arguments
+  /// VerboseErrors, AcceptSupportedOnly.

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

This method doesn't have a VerboseErrors argument.

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

Good point. Incorrectly changed comment to write routine. Reverting to previous
comment.

   void NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out,
                               bool AcceptSupportedOnly = true);
 
   /// isNaClBitcode - Return true if the given bytes are the magic bytes for
   /// PNaCl bitcode wire format.
   ///
   inline bool isNaClBitcode(const unsigned char *BufPtr,
                         const unsigned char *BufEnd) {
     return BufPtr+4 <= BufEnd &&
         BufPtr[0] == 'P' &&
@@ skipping 14 matching lines @@
 
   /// NaClObjDump - Read PNaCl bitcode file from input, and print a
   /// textual representation of its contents. NoRecords and NoAssembly
   /// define what should not be included in the dump. Note: The caller
   /// retains ownership of the Input memory buffer.
   bool NaClObjDump(MemoryBuffer *Input, raw_ostream &output,
                    bool NoRecords, bool NoAssembly);
 
 } // end llvm namespace
 #endif