The input object may be a QueueStreamer, which the compile
server will still have a reference to (even though
downstream the memory object API and parser API thinks it
has a unique_ptr). Terminate the thread quickly on error,
instead of free'ing and causing a use-after-free.
Also set up a report_fatal_error handler which has access
to the server's state. This allows the server to record the
error and stop pushing bytes to the QueueStreamer.
Otherwise the QueueStreamer can get full without a consumer
still active to unblock.
Unfortunately the fatal error handler only terminates the
current thread, and not all worker threads. NaCl doesn't
have support for signals or pthread_kill.
E.g., with pthread_kill(std_thread.native_handle(), SIGABRT).
So, other worker/emitter threads will have to hang waiting on
more input or something.
Random clang-format edits from 3.7.
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4163
TEST= tbd:
I manually ran the translator a dummy text file (invalid bitcode
header), and observed that this no longer crashes. Instead the SRPC
calls finish and I see:
3> [17812,
4147750656:14:23:02.025382] Streaming file at 100000 bps
[17812,
4147750656:14:23:12.511574] RPC call failed: Rpc application returned an error.
[17812,
4147750656:14:23:12.511625] StreamChunk failed
[17812,
4147750656:14:23:12.511655] stream_file: SendDataChunk failed, but returning without failing. Expect call to StreamEnd.4> rpc call initiated StreamEnd::isss
[17812,
4147750656:14:23:12.511931] RPC call failed: Rpc application returned an error.
rpc call complete StreamEnd::isss
output 0: i(0)
output 1: s("")
output 2: s("")
output 3: s("Invalid PNaCl bitcode header")
[17812,
4147750656:14:23:12.512102] Command [rpc] failed.
R=kschimpf@google.com, stichnot@chromium.org
Review URL: https://codereview.chromium.org/
1168543002
#include <cstring>
#include <irt.h>
#include <irt_dev.h>
+#include <pthread.h>
#include <thread>
#include "llvm/Support/QueueStreamer.h"
char *onEndCallback() {
gCompileServer->endInputStream();
gCompileServer->waitForCompileThread();
- // TODO(jvoung): Also return an error string, and UMA data.
- // Set up a report_fatal_error handler to grab that string.
+ // TODO(jvoung): Also return UMA data.
if (gCompileServer->getErrorCode().value()) {
const std::string Error = gCompileServer->getErrorStream().getContents();
return strdup(Error.empty() ? "Some error occurred" : Error.c_str());
new llvm::raw_fd_ostream(FD, CloseOnDtor, Unbuffered));
}
+void fatalErrorHandler(void *UserData, const std::string &Reason,
+ bool GenCrashDialog) {
+ BrowserCompileServer *Server =
+ reinterpret_cast<BrowserCompileServer *>(UserData);
+ Server->setFatalError(Reason);
+ // Only kill the current thread instead of the whole process.
+ // We need the server thread to remain alive in order to respond with the
+ // error message.
+ // We could also try to pthread_kill all other worker threads, but
+ // pthread_kill / raising signals is not supported by NaCl.
+ // We'll have to assume that the worker/emitter threads will be well behaved
+ // after a fatal error in other threads, and either get stuck waiting
+ // on input from a previous stage, or also call report_fatal_error.
+ pthread_exit(0);
+}
+
} // end of anonymous namespace
BrowserCompileServer::~BrowserCompileServer() {}
void BrowserCompileServer::run() {
gCompileServer = this;
- // TODO(jvoung): make a useful fatal error handler that can
- // exit all *worker* threads, keep the server thread alive,
- // and be able to handle a server request for the last error string.
getIRTInterfaces();
gIRTFuncs.serve_translate_request(&SubzeroCallbacks);
}
}
bool BrowserCompileServer::pushInputBytes(const void *Data, size_t NumBytes) {
+ // If there was an earlier error, do not attempt to push bytes to
+ // the QueueStreamer. Otherwise the thread could become blocked.
+ if (HadError.load())
+ return true;
return InputStream->PutBytes(
const_cast<unsigned char *>(
reinterpret_cast<const unsigned char *>(Data)),
NumBytes) != NumBytes;
}
+void BrowserCompileServer::setFatalError(const IceString &Reason) {
+ HadError.store(true);
+ Ctx->getStrError() << Reason;
+ // Make sure that the QueueStreamer is not stuck by signaling an early end.
+ InputStream->SetDone();
+}
+
+ErrorCode &BrowserCompileServer::getErrorCode() {
+ if (HadError.load()) {
+ // HadError means report_fatal_error is called. Make sure that the
+ // LastError is not EC_None. We don't know the type of error so
+ // just pick some error category.
+ LastError.assign(EC_Translation);
+ }
+ return LastError;
+}
+
void BrowserCompileServer::endInputStream() { InputStream->SetDone(); }
void BrowserCompileServer::startCompileThread(int ObjFD) {
&ErrorStream->getStream(), ELFStream.get(),
Flags));
CompileThread = std::thread([this]() {
+ llvm::install_fatal_error_handler(fatalErrorHandler, this);
Ctx->initParserThread();
this->getCompiler().run(ExtraFlags, *Ctx.get(),
// Retain original reference, but the compiler
#ifndef SUBZERO_SRC_ICEBROWSERCOMPILESERVER_H
#define SUBZERO_SRC_ICEBROWSERCOMPILESERVER_H
+#include <atomic>
#include <thread>
#include "IceClFlags.h"
public:
explicit BrowserCompileServer(Compiler &Comp)
- : CompileServer(Comp), InputStream(nullptr) {}
+ : CompileServer(Comp), InputStream(nullptr), HadError(false) {}
~BrowserCompileServer() final;
void run() final;
+ ErrorCode &getErrorCode() final;
+
// Parse and set up the flags for compile jobs.
void getParsedFlags(uint32_t NumThreads, int argc, char **argv);
// Wait for the compile thread to complete then reset the state.
void waitForCompileThread() {
CompileThread.join();
- LastError.assign(Ctx->getErrorStatus()->value());
+ if (Ctx->getErrorStatus()->value())
+ LastError.assign(Ctx->getErrorStatus()->value());
// Reset some state. The InputStream is deleted by the compiler
// so only reset this to nullptr. Free and flush the rest
// of the streams.
StringStream &getErrorStream() { return *ErrorStream; }
+ void setFatalError(const IceString &Reason);
+
private:
class StringStream {
public:
ClFlags Flags;
ClFlagsExtra ExtraFlags;
std::thread CompileThread;
+ std::atomic<bool> HadError;
};
} // end of namespace Ice
virtual void run() = 0;
- ErrorCode &getErrorCode() { return LastError; }
+ virtual ErrorCode &getErrorCode() { return LastError; }
void transferErrorCode(ErrorCodes Code) { LastError.assign(Code); }
protected:
void PNaClTranslator::translate(const std::string &IRFilename,
std::unique_ptr<MemoryObject> &&MemObj) {
-
+ // On error, we report_fatal_error to avoid destroying the MemObj.
+ // That may still be in use by IceBrowserCompileServer. Otherwise,
+ // we need to change the MemObj to be ref-counted, or have a wrapper,
+ // or simply leak. We also need a hook to tell the IceBrowserCompileServer
+ // to unblock its QueueStreamer.
+ // https://code.google.com/p/nativeclient/issues/detail?id=4163
+ Ostream &ErrStream = getContext()->getStrError();
// Read header and verify it is good.
NaClBitcodeHeader Header;
if (Header.Read(MemObj.get())) {
- errs() << "Invalid PNaCl bitcode header.\n";
- ErrorStatus.assign(EC_Bitcode);
- return;
+ llvm::report_fatal_error("Invalid PNaCl bitcode header");
}
if (!Header.IsSupported()) {
- errs() << Header.Unsupported();
+ ErrStream << Header.Unsupported();
if (!Header.IsReadable()) {
- errs() << "Invalid PNaCl bitcode header.\n";
- ErrorStatus.assign(EC_Bitcode);
- return;
+ llvm::report_fatal_error("Invalid PNaCl bitcode header");
}
}
}
if (TopLevelBlocks != 1) {
- errs() << IRFilename
- << ": Contains more than one module. Found: " << TopLevelBlocks
- << "\n";
- ErrorStatus.assign(EC_Bitcode);
+ ErrStream << IRFilename
+ << ": Contains more than one module. Found: " << TopLevelBlocks
+ << "\n";
+ llvm::report_fatal_error("Bitcode has more than one module");
}
if (InputStreamFile.getBitcodeBytes().getExtent() % 4 != 0) {
- errs() << IRFilename
- << ": Bitcode stream should be a multiple of 4 bytes in length.\n";
- ErrorStatus.assign(EC_Bitcode);
- return;
+ ErrStream
+ << IRFilename
+ << ": Bitcode stream should be a multiple of 4 bytes in length.\n";
+ llvm::report_fatal_error("Bitcode stream should be a multiple of 4 bytes");
}
}
OSDN Git Service
