Fix error reporting in the PNaCl bitcode reader.

tools/pnacl-llc/pnacl-llc.cpp

 //===-- pnacl-llc.cpp - PNaCl-specific llc: pexe ---> nexe  ---------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // pnacl-llc: the core of the PNaCl translator, compiling a pexe into a nexe.
@@ skipping 72 matching lines @@
 // General options for llc.  Other pass-specific options are specified
 // within the corresponding llc passes, and target-specific options
 // and back-end code generation options are specified with the target machine.
 //
 static cl::opt<std::string>
 InputFilename(cl::Positional, cl::desc("<input bitcode>"), cl::init("-"));
 
 static cl::opt<std::string>
 OutputFilename("o", cl::desc("Output filename"), cl::value_desc("filename"));
 
+static cl::opt<bool>
+VerboseErrors(
+    "verbose-parse-errors",
+    cl::desc("Print out more descriptive PNaCl bitcode parse errors"),
+    cl::init(false));
+
 // Using bitcode streaming allows compilation of one function at a time. This
 // allows earlier functions to be compiled before later functions are read from
 // the bitcode but of course means no whole-module optimizations. This means
 // that Module passes that run should only touch globals/function declarations
 // and not function bodies, otherwise the streaming and non-streaming code
 // pathes wouldn't emit the same code for each function. For now, streaming is
 // only supported for files and stdin.
 static cl::opt<bool>
 LazyBitcode("streaming-bitcode",
   cl::desc("Use lazy bitcode streaming for file inputs"),
@@ skipping 216 matching lines @@
   Reporter.reset();
 }
 
 static Module* getModule(StringRef ProgramName, LLVMContext &Context,
                          StreamingMemoryObject *StreamingObject) {
   Module *M = nullptr;
   SMDiagnostic Err;
   if (LazyBitcode) {
     std::string StrError;
     if (InputFileFormat == PNaClFormat) {
+      raw_ostream *Verbose = VerboseErrors ? &errs() : nullptr;

[jvoung (off chromium), 2014/12/03 19:43:46]

Now that this is a stream, __native_client__/pnacl-llc in the browser could
potentially have a raw_string_ostream, and concat/prepend that to the
Err.getMessage below. On the other hand, maybe the errors are sufficient and
nullptr is fine.

[Karl, 2014/12/03 20:53:44]

Went ahead and added verbose errors if command line argument says so.

[jvoung (off chromium), 2014/12/03 22:11:13]

What I meant was why not pass in the VerboseStrm always? There's no way to set
this particular commandline flag when loading a pexe in the browser. Only a
subset of flags are set in the browser e.g., O0 vs O2 via the NMF, or when the
browser detects that the machine has SSE4 it'll set --mattr=...).  Then we get a
tad more info when people copy-paste whatever shows up in the JS console. 
Otherwise, this is "dead code" but not in the sense that the compiler can
optimize it out (vs what you had in patch set 2, where you hardcoded it to
false... well, potentially optimized out).

That said, probably the more common type of problems we'll run into are when we
extend the bitcode format and the developer is using an old browser w/ a new
pexe. In that case, we could be more careful bumping the bitcode version or
feature vector, so that the error message is indicated by the header check
instead of deep down somewhere having the reader says some error about an
invalid record.

[Karl, 2014/12/03 22:28:51]

Ok. I'll make it be always when in __native_client__. I double checked the code,
and it will (correctly) add verbose errors corresponding to bad header (and then
return) if there are any problem. Hence, the common type of problem will be
reported in this case.

       M = getNaClStreamedBitcodeModule(
           InputFilename,
-          new ThreadedStreamingCache(StreamingObject), Context, &StrError);
+          new ThreadedStreamingCache(StreamingObject), Context, Verbose,
+          &StrError);
     } else if (InputFileFormat == LLVMFormat) {
       M = getStreamedBitcodeModule(
           InputFilename,
           new ThreadedStreamingCache(StreamingObject), Context, &StrError);
     } else {
       llvm_unreachable("Unknown bitcode format");
     }
     if (!StrError.empty())
       Err = SMDiagnostic(InputFilename, SourceMgr::DK_Error, StrError);
   } else {
 #if defined(__native_client__)
     llvm_unreachable("native client SRPC only supports streaming");
 #else
     // Parses binary bitcode as well as textual assembly
     // (so pulls in more code into pnacl-llc).
-    M = NaClParseIRFile(InputFilename, InputFileFormat, Err, Context);
+    raw_ostream *Verbose = VerboseErrors ? &errs() : nullptr;
+    M = NaClParseIRFile(InputFilename, InputFileFormat, Err, Verbose,
+                        Context);
 #endif
   }
   if (!M) {
 #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 nullptr;
 #endif
@@ skipping 409 matching lines @@
   return 0;
 }
 
 int main(int argc, char **argv) {
 #if defined(__native_client__)
   return srpc_main(argc, argv);
 #else
   return llc_main(argc, argv);
 #endif // __native_client__
 }