First attempt to capture parser/translation errors in browser.
Adds a notion of an (optional) error stream to the existing
log and emit streams. If not specified, the log stream is used.
Error messages in parser/translation are sent to this new error
stream.
In the browser compiler server, a separate error (string) stream is
created to capture errors. Method onEndCallBack returns the contents
of the error stream (if non-empty) instead of a generic error message.
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4138
R=jvoung@chromium.org
Review URL: https://codereview.chromium.org/1052833003
diff --git a/src/IceBrowserCompileServer.cpp b/src/IceBrowserCompileServer.cpp
index 85c2efe..92d42e2 100644
--- a/src/IceBrowserCompileServer.cpp
+++ b/src/IceBrowserCompileServer.cpp
@@ -81,7 +81,8 @@
// TODO(jvoung): Also return an error string, and UMA data.
// Set up a report_fatal_error handler to grab that string.
if (gCompileServer->getErrorCode().value()) {
- return strdup("Some error occurred");
+ const std::string Error = gCompileServer->getErrorStream().getContents();
+ return strdup(Error.empty() ? "Some error occurred" : Error.c_str());
}
return nullptr;
}
@@ -142,9 +143,12 @@
LogStream->SetUnbuffered();
EmitStream = getOutputStream(ObjFD);
EmitStream->SetBufferSize(1 << 14);
+ std::unique_ptr<StringStream> ErrStrm(new StringStream());
+ ErrorStream = std::move(ErrStrm);
ELFStream.reset(new ELFStreamer(*EmitStream.get()));
Ctx.reset(new GlobalContext(LogStream.get(), EmitStream.get(),
- ELFStream.get(), Flags));
+ &ErrorStream->getStream(), ELFStream.get(),
+ Flags));
CompileThread = std::thread([this]() {
Ctx->initParserThread();
this->getCompiler().run(ExtraFlags, *Ctx.get(),
diff --git a/src/IceBrowserCompileServer.h b/src/IceBrowserCompileServer.h
index 1ef8121..a34a03d 100644
--- a/src/IceBrowserCompileServer.h
+++ b/src/IceBrowserCompileServer.h
@@ -39,6 +39,7 @@
BrowserCompileServer() = delete;
BrowserCompileServer(const BrowserCompileServer &) = delete;
BrowserCompileServer &operator=(const BrowserCompileServer &) = delete;
+ class StringStream;
public:
explicit BrowserCompileServer(Compiler &Comp)
@@ -74,7 +75,20 @@
ELFStream.reset(nullptr);
}
+ StringStream &getErrorStream() {
+ return *ErrorStream;
+ }
+
private:
+ class StringStream {
+ public:
+ StringStream() : StrBuf(Buffer) {}
+ const IceString &getContents() { return StrBuf.str(); }
+ Ostream &getStream() { return StrBuf; }
+ private:
+ std::string Buffer;
+ llvm::raw_string_ostream StrBuf;
+ };
// This currently only handles a single compile request, hence one copy
// of the state.
std::unique_ptr<GlobalContext> Ctx;
@@ -84,6 +98,7 @@
llvm::QueueStreamer *InputStream;
std::unique_ptr<Ostream> LogStream;
std::unique_ptr<llvm::raw_fd_ostream> EmitStream;
+ std::unique_ptr<StringStream> ErrorStream;
std::unique_ptr<ELFStreamer> ELFStream;
ClFlags Flags;
ClFlagsExtra ExtraFlags;
diff --git a/src/IceCompileServer.cpp b/src/IceCompileServer.cpp
index 86ebb0b..cf398d4 100644
--- a/src/IceCompileServer.cpp
+++ b/src/IceCompileServer.cpp
@@ -96,7 +96,8 @@
return transferErrorCode(getReturnValue(ExtraFlags, Ice::EC_Bitcode));
}
- Ctx.reset(new GlobalContext(Ls.get(), Os.get(), ELFStr.get(), Flags));
+ Ctx.reset(new GlobalContext(Ls.get(), Os.get(), Ls.get(), ELFStr.get(),
+ Flags));
if (Ctx->getFlags().getNumTranslationThreads() != 0) {
std::thread CompileThread([this, &ExtraFlags, &InputStream]() {
Ctx->initParserThread();
diff --git a/src/IceGlobalContext.cpp b/src/IceGlobalContext.cpp
index 1c9e3c4..8de57f3 100644
--- a/src/IceGlobalContext.cpp
+++ b/src/IceGlobalContext.cpp
@@ -213,14 +213,18 @@
Str << "\n";
}
-GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit,
+GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
ELFStreamer *ELFStr, const ClFlags &Flags)
: ConstPool(new ConstantPool()), ErrorStatus(), StrDump(OsDump),
- StrEmit(OsEmit), Flags(Flags), RNG(Flags.getRandomSeed()), ObjectWriter(),
+ StrEmit(OsEmit), StrError(OsError), Flags(Flags),
+ RNG(Flags.getRandomSeed()), ObjectWriter(),
OptQ(/*Sequential=*/Flags.isSequential(),
/*MaxSize=*/Flags.getNumTranslationThreads()),
// EmitQ is allowed unlimited size.
EmitQ(/*Sequential=*/Flags.isSequential()) {
+ assert(OsDump && "OsDump is not defined for GlobalContext");
+ assert(OsEmit && "OsEmit is not defined for GlobalContext");
+ assert(OsError && "OsError is not defined for GlobalContext");
// Make sure thread_local fields are properly initialized before any
// accesses are made. Do this here instead of at the start of
// main() so that all clients (e.g. unit tests) can benefit for
@@ -278,8 +282,8 @@
if (Func->hasError()) {
getErrorStatus()->assign(EC_Translation);
OstreamLocker L(this);
- getStrDump() << "ICE translation error: " << Func->getFunctionName()
- << ": " << Func->getError() << "\n";
+ getStrError() << "ICE translation error: " << Func->getFunctionName()
+ << ": " << Func->getError() << "\n";
Item = new EmitterWorkItem(Func->getSequenceNumber());
} else {
Func->getAssembler<>()->setInternal(Func->getInternal());
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index 9264d43..a1fd133 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -145,14 +145,17 @@
};
public:
- GlobalContext(Ostream *OsDump, Ostream *OsEmit, ELFStreamer *ELFStreamer,
- const ClFlags &Flags);
+ // The dump stream is a log stream while emit is the stream code
+ // is emitted to. The error stream is strictly for logging errors.
+ GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
+ ELFStreamer *ELFStreamer, const ClFlags &Flags);
~GlobalContext();
- // The dump and emit streams need to be used by only one thread at a
- // time. This is done by exclusively reserving the streams via
- // lockStr() and unlockStr(). The OstreamLocker class can be used
- // to conveniently manage this.
+ //
+ // The dump, error, and emit streams need to be used by only one
+ // thread at a time. This is done by exclusively reserving the
+ // streams via lockStr() and unlockStr(). The OstreamLocker class
+ // can be used to conveniently manage this.
//
// The model is that a thread grabs the stream lock, then does an
// arbitrary amount of work during which far-away callees may grab
@@ -163,6 +166,7 @@
void lockStr() { StrLock.lock(); }
void unlockStr() { StrLock.unlock(); }
Ostream &getStrDump() { return *StrDump; }
+ Ostream &getStrError() { return *StrError; }
Ostream &getStrEmit() { return *StrEmit; }
LockedPtr<ErrorCode> getErrorStatus() {
@@ -418,6 +422,7 @@
StrLockType StrLock;
Ostream *StrDump; // Stream for dumping / diagnostics
Ostream *StrEmit; // Stream for code emission
+ Ostream *StrError; // Stream for logging errors.
ICE_CACHELINE_BOUNDARY;
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index ce46239..42a207b 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -488,7 +488,7 @@
Ice::GlobalContext *Context = Translator.getContext();
{ // Lock while printing out error message.
Ice::OstreamLocker L(Context);
- raw_ostream &OldErrStream = setErrStream(Context->getStrDump());
+ raw_ostream &OldErrStream = setErrStream(Context->getStrError());
NaClBitcodeParser::ErrorAt(Level, Bit, Message);
setErrStream(OldErrStream);
}
