From 43c933d7f8e490d9dbf3f939f3b2f095a170dc84 Mon Sep 17 00:00:00 2001 From: "jcivelli@chromium.org" Date: Thu, 9 Aug 2012 22:59:58 +0000 Subject: [PATCH] Adding a way to create an ExceptionHandler that takes in a file descriptor where the minidump should be created, without the need of opening any other file. BUG=None TEST=Run unit-tests. git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1007 4c0a9323-5329-0410-9bdc-e9ce6186880e --- Makefile.am | 4 + Makefile.in | 11 + src/client/linux/handler/exception_handler.cc | 118 ++--- src/client/linux/handler/exception_handler.h | 92 ++-- .../handler/exception_handler_unittest.cc | 484 +++++++++--------- .../linux/handler/minidump_descriptor.cc | 61 +++ .../linux/handler/minidump_descriptor.h | 84 +++ .../linux/minidump_writer/minidump_writer.cc | 124 +++-- .../linux/minidump_writer/minidump_writer.h | 18 +- .../minidump_writer_unittest.cc | 39 +- src/client/minidump_file_writer.cc | 15 +- src/client/minidump_file_writer.h | 29 +- src/common/tests/auto_tempdir.h | 2 +- 13 files changed, 669 insertions(+), 412 deletions(-) create mode 100644 src/client/linux/handler/minidump_descriptor.cc create mode 100644 src/client/linux/handler/minidump_descriptor.h diff --git a/Makefile.am b/Makefile.am index b87bb351..408a2a6e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -75,6 +75,7 @@ lib_LIBRARIES += src/client/linux/libbreakpad_client.a src_client_linux_libbreakpad_client_a_SOURCES = \ src/client/linux/crash_generation/crash_generation_client.cc \ src/client/linux/handler/exception_handler.cc \ + src/client/linux/handler/minidump_descriptor.cc \ src/client/linux/log/log.cc \ src/client/linux/minidump_writer/linux_dumper.cc \ src/client/linux/minidump_writer/linux_ptrace_dumper.cc \ @@ -327,6 +328,7 @@ src_client_linux_linux_client_unittest_CPPFLAGS = \ -I$(top_srcdir)/src/testing src_client_linux_linux_client_unittest_LDADD = \ src/client/linux/handler/exception_handler.o \ + src/client/linux/handler/minidump_descriptor.o \ src/client/linux/log/log.o \ src/client/linux/crash_generation/crash_generation_client.o \ src/client/linux/minidump_writer/linux_dumper.o \ @@ -881,6 +883,8 @@ EXTRA_DIST = \ src/client/linux/handler/Makefile \ src/client/linux/handler/exception_handler.cc \ src/client/linux/handler/exception_handler.h \ + src/client/linux/handler/minidump_descriptor.cc \ + src/client/linux/handler/minidump_descriptor.h \ src/client/linux/handler/exception_handler_test.cc \ src/client/linux/handler/linux_thread.cc \ src/client/linux/handler/linux_thread.h \ diff --git a/Makefile.in b/Makefile.in index a0bd1c39..f4cfb2ed 100644 --- a/Makefile.in +++ b/Makefile.in @@ -171,6 +171,7 @@ src_client_linux_libbreakpad_client_a_LIBADD = am__src_client_linux_libbreakpad_client_a_SOURCES_DIST = \ src/client/linux/crash_generation/crash_generation_client.cc \ src/client/linux/handler/exception_handler.cc \ + src/client/linux/handler/minidump_descriptor.cc \ src/client/linux/log/log.cc \ src/client/linux/minidump_writer/linux_dumper.cc \ src/client/linux/minidump_writer/linux_ptrace_dumper.cc \ @@ -185,6 +186,7 @@ am__src_client_linux_libbreakpad_client_a_SOURCES_DIST = \ am__dirstamp = $(am__leading_dot)dirstamp @LINUX_HOST_TRUE@am_src_client_linux_libbreakpad_client_a_OBJECTS = src/client/linux/crash_generation/crash_generation_client.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/handler/exception_handler.$(OBJEXT) \ +@LINUX_HOST_TRUE@ src/client/linux/handler/minidump_descriptor.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/log/log.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_dumper.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_ptrace_dumper.$(OBJEXT) \ @@ -1239,6 +1241,7 @@ lib_LIBRARIES = $(am__append_1) $(am__append_3) @LINUX_HOST_TRUE@src_client_linux_libbreakpad_client_a_SOURCES = \ @LINUX_HOST_TRUE@ src/client/linux/crash_generation/crash_generation_client.cc \ @LINUX_HOST_TRUE@ src/client/linux/handler/exception_handler.cc \ +@LINUX_HOST_TRUE@ src/client/linux/handler/minidump_descriptor.cc \ @LINUX_HOST_TRUE@ src/client/linux/log/log.cc \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_dumper.cc \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_ptrace_dumper.cc \ @@ -1418,6 +1421,7 @@ TESTS_ENVIRONMENT = @LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_LDADD = \ @LINUX_HOST_TRUE@ src/client/linux/handler/exception_handler.o \ +@LINUX_HOST_TRUE@ src/client/linux/handler/minidump_descriptor.o \ @LINUX_HOST_TRUE@ src/client/linux/log/log.o \ @LINUX_HOST_TRUE@ src/client/linux/crash_generation/crash_generation_client.o \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_dumper.o \ @@ -2001,6 +2005,8 @@ EXTRA_DIST = \ src/client/linux/handler/Makefile \ src/client/linux/handler/exception_handler.cc \ src/client/linux/handler/exception_handler.h \ + src/client/linux/handler/minidump_descriptor.cc \ + src/client/linux/handler/minidump_descriptor.h \ src/client/linux/handler/exception_handler_test.cc \ src/client/linux/handler/linux_thread.cc \ src/client/linux/handler/linux_thread.h \ @@ -2238,6 +2244,9 @@ src/client/linux/handler/$(DEPDIR)/$(am__dirstamp): src/client/linux/handler/exception_handler.$(OBJEXT): \ src/client/linux/handler/$(am__dirstamp) \ src/client/linux/handler/$(DEPDIR)/$(am__dirstamp) +src/client/linux/handler/minidump_descriptor.$(OBJEXT): \ + src/client/linux/handler/$(am__dirstamp) \ + src/client/linux/handler/$(DEPDIR)/$(am__dirstamp) src/client/linux/log/$(am__dirstamp): @$(MKDIR_P) src/client/linux/log @: > src/client/linux/log/$(am__dirstamp) @@ -3142,6 +3151,7 @@ mostlyclean-compile: -rm -f *.$(OBJEXT) -rm -f src/client/linux/crash_generation/crash_generation_client.$(OBJEXT) -rm -f src/client/linux/handler/exception_handler.$(OBJEXT) + -rm -f src/client/linux/handler/minidump_descriptor.$(OBJEXT) -rm -f src/client/linux/handler/src_client_linux_linux_client_unittest-exception_handler_unittest.$(OBJEXT) -rm -f src/client/linux/log/log.$(OBJEXT) -rm -f src/client/linux/minidump_writer/linux_core_dumper.$(OBJEXT) @@ -3369,6 +3379,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@src/client/$(DEPDIR)/minidump_file_writer.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/crash_generation/$(DEPDIR)/crash_generation_client.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/handler/$(DEPDIR)/exception_handler.Po@am__quote@ +@AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/handler/$(DEPDIR)/minidump_descriptor.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/handler/$(DEPDIR)/src_client_linux_linux_client_unittest-exception_handler_unittest.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/log/$(DEPDIR)/log.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/linux_core_dumper.Po@am__quote@ diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index f505ff33..73c3bdfb 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -93,7 +93,6 @@ #include "client/linux/log/log.h" #include "client/linux/minidump_writer/linux_dumper.h" #include "client/linux/minidump_writer/minidump_writer.h" -#include "common/linux/guid_creator.h" #include "common/linux/eintr_wrapper.h" #include "third_party/lss/linux_syscall_support.h" @@ -126,31 +125,31 @@ pthread_mutex_t ExceptionHandler::handler_stack_mutex_ = PTHREAD_MUTEX_INITIALIZER; // Runs before crashing: normal context. -ExceptionHandler::ExceptionHandler(const string &dump_path, - FilterCallback filter, - MinidumpCallback callback, - void *callback_context, - bool install_handler) - : filter_(filter), - callback_(callback), - callback_context_(callback_context), - handler_installed_(install_handler) -{ - Init(dump_path, -1); -} - -ExceptionHandler::ExceptionHandler(const string &dump_path, +ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor, FilterCallback filter, MinidumpCallback callback, void* callback_context, bool install_handler, const int server_fd) - : filter_(filter), - callback_(callback), - callback_context_(callback_context), - handler_installed_(install_handler) -{ - Init(dump_path, server_fd); + : filter_(filter), + callback_(callback), + callback_context_(callback_context), + minidump_descriptor_(descriptor), + crash_handler_(NULL) { + if (server_fd >= 0) + crash_generation_client_.reset(CrashGenerationClient::TryCreate(server_fd)); + + if (install_handler) + InstallHandlers(); + + if (!IsOutOfProcess() && !minidump_descriptor_.IsFD()) + minidump_descriptor_.UpdatePath(); + + pthread_mutex_lock(&handler_stack_mutex_); + if (!handler_stack_) + handler_stack_ = new std::vector; + handler_stack_->push_back(this); + pthread_mutex_unlock(&handler_stack_mutex_); } // Runs before crashing: normal context. @@ -158,27 +157,6 @@ ExceptionHandler::~ExceptionHandler() { UninstallHandlers(); } -void ExceptionHandler::Init(const string &dump_path, - const int server_fd) -{ - crash_handler_ = NULL; - if (0 <= server_fd) - crash_generation_client_ - .reset(CrashGenerationClient::TryCreate(server_fd)); - - if (handler_installed_) - InstallHandlers(); - - if (!IsOutOfProcess()) - set_dump_path(dump_path); - - pthread_mutex_lock(&handler_stack_mutex_); - if (handler_stack_ == NULL) - handler_stack_ = new std::vector; - handler_stack_->push_back(this); - pthread_mutex_unlock(&handler_stack_mutex_); -} - // Runs before crashing: normal context. bool ExceptionHandler::InstallHandlers() { // We run the signal handlers on an alternative stack because we might have @@ -237,24 +215,6 @@ void ExceptionHandler::UninstallHandlers() { old_handlers_.clear(); } -// Runs before crashing: normal context. -void ExceptionHandler::UpdateNextID() { - GUID guid; - char guid_str[kGUIDStringLength + 1]; - if (CreateGUID(&guid) && GUIDToString(&guid, guid_str, sizeof(guid_str))) { - next_minidump_id_ = guid_str; - next_minidump_id_c_ = next_minidump_id_.c_str(); - - char minidump_path[PATH_MAX]; - snprintf(minidump_path, sizeof(minidump_path), "%s/%s.dmp", - dump_path_c_, - guid_str); - - next_minidump_path_ = minidump_path; - next_minidump_path_c_ = next_minidump_path_.c_str(); - } -} - // void ExceptionHandler::set_crash_handler(HandlerCallback callback) { // crash_handler_ = callback; // } @@ -308,6 +268,7 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) { struct ThreadArgument { pid_t pid; // the crashing process + const MinidumpDescriptor* minidump_descriptor; ExceptionHandler* handler; const void* context; // a CrashContext structure size_t context_size; @@ -378,6 +339,7 @@ bool ExceptionHandler::GenerateDump(CrashContext *context) { ThreadArgument thread_arg; thread_arg.handler = this; + thread_arg.minidump_descriptor = &minidump_descriptor_; thread_arg.pid = getpid(); thread_arg.context = context; thread_arg.context_size = sizeof(*context); @@ -420,11 +382,8 @@ bool ExceptionHandler::GenerateDump(CrashContext *context) { } bool success = r != -1 && WIFEXITED(status) && WEXITSTATUS(status) == 0; - if (callback_) - success = callback_(dump_path_c_, next_minidump_id_c_, - callback_context_, success); - + success = callback_(minidump_descriptor_, callback_context_, success); return success; } @@ -461,7 +420,14 @@ void ExceptionHandler::WaitForContinueSignal() { // Runs on the cloned process. bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, size_t context_size) { - return google_breakpad::WriteMinidump(next_minidump_path_c_, + if (minidump_descriptor_.IsFD()) { + return google_breakpad::WriteMinidump(minidump_descriptor_.fd(), + crashing_process, + context, + context_size, + mapping_list_); + } + return google_breakpad::WriteMinidump(minidump_descriptor_.path(), crashing_process, context, context_size, @@ -469,15 +435,29 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, } // static -bool ExceptionHandler::WriteMinidump(const string &dump_path, +bool ExceptionHandler::WriteMinidump(const string& dump_path, MinidumpCallback callback, void* callback_context) { - ExceptionHandler eh(dump_path, NULL, callback, callback_context, false); + MinidumpDescriptor descriptor(dump_path); + ExceptionHandler eh(descriptor, NULL, callback, callback_context, false, -1); return eh.WriteMinidump(); } bool ExceptionHandler::WriteMinidump() { #if !defined(__ARM_EABI__) + if (!IsOutOfProcess() && !minidump_descriptor_.IsFD()) { + // Update the path of the minidump so that this can be called multiple times + // and new files are created for each minidump. This is done before the + // generation happens, as clients may want to access the MinidumpDescriptor + // after this call to find the exact path to the minidump file. + minidump_descriptor_.UpdatePath(); + } else if (minidump_descriptor_.IsFD()) { + // Reposition the FD to its beginning and resize it to get rid of the + // previous minidump info. + lseek(minidump_descriptor_.fd(), 0, SEEK_SET); + ftruncate(minidump_descriptor_.fd(), 0); + } + // Allow ourselves to be dumped. sys_prctl(PR_SET_DUMPABLE, 1); @@ -489,9 +469,7 @@ bool ExceptionHandler::WriteMinidump() { sizeof(context.float_state)); context.tid = sys_gettid(); - bool success = GenerateDump(&context); - UpdateNextID(); - return success; + return GenerateDump(&context); #else return false; #endif // !defined(__ARM_EABI__) diff --git a/src/client/linux/handler/exception_handler.h b/src/client/linux/handler/exception_handler.h index 4bc8ba72..c339b7dd 100644 --- a/src/client/linux/handler/exception_handler.h +++ b/src/client/linux/handler/exception_handler.h @@ -42,6 +42,7 @@ #include "client/linux/android_ucontext.h" #endif #include "client/linux/crash_generation/crash_generation_client.h" +#include "client/linux/handler/minidump_descriptor.h" #include "client/linux/minidump_writer/minidump_writer.h" #include "common/using_std_string.h" #include "google_breakpad/common/minidump_format.h" @@ -51,8 +52,6 @@ struct sigaction; namespace google_breakpad { -class ExceptionHandler; - // ExceptionHandler // // ExceptionHandler can write a minidump file when an exception occurs, @@ -71,17 +70,18 @@ class ExceptionHandler; // use different minidump callbacks for different call sites. // // In either case, a callback function is called when a minidump is written, -// which receives the unqiue id of the minidump. The caller can use this -// id to collect and write additional application state, and to launch an -// external crash-reporting application. +// which receives the full path or file descriptor of the minidump. The +// caller can collect and write additional application state to that minidump, +// and launch an external crash-reporting application. // // Caller should try to make the callbacks as crash-friendly as possible, // it should avoid use heap memory allocation as much as possible. + class ExceptionHandler { public: // A callback function to run before Breakpad performs any substantial // processing of an exception. A FilterCallback is called before writing - // a minidump. context is the parameter supplied by the user as + // a minidump. |context| is the parameter supplied by the user as // callback_context when the handler was created. // // If a FilterCallback returns true, Breakpad will continue processing, @@ -91,10 +91,10 @@ class ExceptionHandler { typedef bool (*FilterCallback)(void *context); // A callback function to run after the minidump has been written. - // minidump_id is a unique id for the dump, so the minidump - // file is \.dmp. context is the parameter supplied - // by the user as callback_context when the handler was created. succeeded - // indicates whether a minidump file was successfully written. + // |descriptor| contains the file descriptor or file path containing the + // minidump. |context| is the parameter supplied by the user as + // callback_context when the handler was created. |succeeded| indicates + // whether a minidump file was successfully written. // // If an exception occurred and the callback returns true, Breakpad will // treat the exception as fully-handled, suppressing any other handlers from @@ -106,9 +106,8 @@ class ExceptionHandler { // should normally return the value of |succeeded|, or when they wish to // not report an exception of handled, false. Callbacks will rarely want to // return true directly (unless |succeeded| is true). - typedef bool (*MinidumpCallback)(const char *dump_path, - const char *minidump_id, - void *context, + typedef bool (*MinidumpCallback)(const MinidumpDescriptor& descriptor, + void* context, bool succeeded); // In certain cases, a user may wish to handle the generation of the minidump @@ -121,52 +120,52 @@ class ExceptionHandler { void* context); // Creates a new ExceptionHandler instance to handle writing minidumps. - // Before writing a minidump, the optional filter callback will be called. + // Before writing a minidump, the optional |filter| callback will be called. // Its return value determines whether or not Breakpad should write a - // minidump. Minidump files will be written to dump_path, and the optional - // callback is called after writing the dump file, as described above. + // minidump. The minidump content will be written to the file path or file + // descriptor from |descriptor|, and the optional |callback| is called after + // writing the dump file, as described above. // If install_handler is true, then a minidump will be written whenever // an unhandled exception occurs. If it is false, minidumps will only // be written when WriteMinidump is called. - ExceptionHandler(const string &dump_path, - FilterCallback filter, MinidumpCallback callback, + // If |server_fd| is valid, the minidump is generated out-of-process. If it + // is -1, in-process generation will always be used. + ExceptionHandler(const MinidumpDescriptor& descriptor, + FilterCallback filter, + MinidumpCallback callback, void *callback_context, - bool install_handler); - - // Creates a new ExceptionHandler instance that can attempt to - // perform out-of-process dump generation if server_fd is valid. If - // server_fd is invalid, in-process dump generation will be - // used. See the above ctor for a description of the other - // parameters. - ExceptionHandler(const string& dump_path, - FilterCallback filter, MinidumpCallback callback, - void* callback_context, bool install_handler, const int server_fd); - ~ExceptionHandler(); - // Get and set the minidump path. - string dump_path() const { return dump_path_; } - void set_dump_path(const string &dump_path) { - dump_path_ = dump_path; - dump_path_c_ = dump_path_.c_str(); - UpdateNextID(); + const MinidumpDescriptor& minidump_descriptor() const { + return minidump_descriptor_; } void set_crash_handler(HandlerCallback callback) { crash_handler_ = callback; } - // Writes a minidump immediately. This can be used to capture the - // execution state independently of a crash. Returns true on success. + // Writes a minidump immediately. This can be used to capture the execution + // state independently of a crash. + // Returns true on success. + // If the ExceptionHandler has been created with a path, a new file is + // generated for each minidump. The file path can be retrieved in the + // MinidumpDescriptor passed to the MinidumpCallback or by accessing the + // MinidumpDescriptor directly from the ExceptionHandler (with + // minidump_descriptor()). + // If the ExceptionHandler has been created with a file descriptor, the file + // descriptor is repositioned to its beginning and the previous generated + // minidump is overwritten. + // Note that this method is not supposed to be called from a compromised + // context as it uses the heap. bool WriteMinidump(); // Convenience form of WriteMinidump which does not require an // ExceptionHandler instance. - static bool WriteMinidump(const string &dump_path, + static bool WriteMinidump(const string& dump_path, MinidumpCallback callback, - void *callback_context); + void* callback_context); // This structure is passed to minidump_writer.h:WriteMinidump via an opaque // blob. It shouldn't be needed in any user code. @@ -195,8 +194,6 @@ class ExceptionHandler { size_t file_offset); private: - void Init(const string &dump_path, - const int server_fd); bool InstallHandlers(); void UninstallHandlers(); void PreresolveSymbols(); @@ -204,7 +201,6 @@ class ExceptionHandler { void SendContinueSignalToChild(); void WaitForContinueSignal(); - void UpdateNextID(); static void SignalHandler(int sig, siginfo_t* info, void* uc); bool HandleSignal(int sig, siginfo_t* info, void* uc); static int ThreadEntry(void* arg); @@ -217,18 +213,8 @@ class ExceptionHandler { scoped_ptr crash_generation_client_; - string dump_path_; - string next_minidump_path_; - string next_minidump_id_; + MinidumpDescriptor minidump_descriptor_; - // Pointers to C-string representations of the above. These are set - // when the above are set so we can avoid calling c_str during - // an exception. - const char* dump_path_c_; - const char* next_minidump_path_c_; - const char* next_minidump_id_c_; - - const bool handler_installed_; HandlerCallback crash_handler_; // The global exception handler stack. This is need becuase there may exist diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index ea2652d5..6ad3661e 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -52,11 +52,25 @@ using namespace google_breakpad; +namespace { + // Length of a formatted GUID string = // sizeof(MDGUID) * 2 + 4 (for dashes) + 1 (null terminator) const int kGUIDStringSize = 37; -static void sigchld_handler(int signo) { } +void sigchld_handler(int signo) { } + +int CreateTMPFile(const std::string& dir, std::string* path) { + std::string file = dir + "/exception-handler-unittest.XXXXXX"; + const char* c_file = file.c_str(); + // Copy that string, mkstemp needs a C string it can modify. + char* c_path = strdup(c_file); + const int fd = mkstemp(c_path); + if (fd >= 0) + *path = c_path; + free(c_path); + return fd; +} class ExceptionHandlerTest : public ::testing::Test { protected: @@ -75,70 +89,117 @@ class ExceptionHandlerTest : public ::testing::Test { struct sigaction old_action; }; -TEST(ExceptionHandlerTest, Simple) { - AutoTempDir temp_dir; - ExceptionHandler handler(temp_dir.path(), NULL, NULL, NULL, true); -} - -static bool DoneCallback(const char* dump_path, - const char* minidump_id, - void* context, - bool succeeded) { - if (!succeeded) - return succeeded; - - int fd = (intptr_t) context; - uint32_t len = my_strlen(minidump_id); - IGNORE_RET(HANDLE_EINTR(sys_write(fd, &len, sizeof(len)))); - IGNORE_RET(HANDLE_EINTR(sys_write(fd, minidump_id, len))); - sys_close(fd); - - return true; -} - -TEST(ExceptionHandlerTest, ChildCrash) { - AutoTempDir temp_dir; - int fds[2]; - ASSERT_NE(pipe(fds), -1); - - const pid_t child = fork(); - if (child == 0) { - close(fds[0]); - ExceptionHandler handler(temp_dir.path(), NULL, DoneCallback, (void*) fds[1], - true); - *reinterpret_cast(NULL) = 0; - } - close(fds[1]); +void WaitForProcessToTerminate(pid_t process_id, int expected_status) { int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); + ASSERT_NE(HANDLE_EINTR(waitpid(process_id, &status, 0)), -1); ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGSEGV); + ASSERT_EQ(expected_status, WTERMSIG(status)); +} +// Reads the minidump path sent over the pipe |fd| and sets it in |path|. +void ReadMinidumpPathFromPipe(int fd, std::string* path) { struct pollfd pfd; memset(&pfd, 0, sizeof(pfd)); - pfd.fd = fds[0]; + pfd.fd = fd; pfd.events = POLLIN | POLLERR; const int r = HANDLE_EINTR(poll(&pfd, 1, 0)); - ASSERT_EQ(r, 1); + ASSERT_EQ(1, r); ASSERT_TRUE(pfd.revents & POLLIN); uint32_t len; - ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len)); - ASSERT_LT(len, (uint32_t)2048); - char* filename = reinterpret_cast(malloc(len + 1)); - ASSERT_EQ(read(fds[0], filename, len), len); + ASSERT_EQ(static_cast(sizeof(len)), read(fd, &len, sizeof(len))); + ASSERT_LT(len, static_cast(2048)); + char* filename = static_cast(malloc(len + 1)); + ASSERT_EQ(len, read(fd, filename, len)); filename[len] = 0; - close(fds[0]); + close(fd); + *path = filename; + free(filename); +} - const string minidump_filename = temp_dir.path() + "/" + filename + - ".dmp"; +} // namespace + +TEST(ExceptionHandlerTest, SimpleWithPath) { + AutoTempDir temp_dir; + ExceptionHandler handler( + MinidumpDescriptor(temp_dir.path()), NULL, NULL, NULL, true, -1); +} + +TEST(ExceptionHandlerTest, SimpleWithFD) { + AutoTempDir temp_dir; + std::string path; + const int fd = CreateTMPFile(temp_dir.path(), &path); + ExceptionHandler handler(MinidumpDescriptor(fd), NULL, NULL, NULL, true, -1); + close(fd); +} + +static bool DoneCallback(const MinidumpDescriptor& descriptor, + void* context, + bool succeeded) { + if (!succeeded) + return false; + + if (!descriptor.IsFD()) { + int fd = reinterpret_cast(context); + uint32_t len = 0; + len = my_strlen(descriptor.path()); + IGNORE_RET(HANDLE_EINTR(sys_write(fd, &len, sizeof(len)))); + IGNORE_RET(HANDLE_EINTR(sys_write(fd, descriptor.path(), len))); + } + return true; +} + +void ChildCrash(bool use_fd) { + AutoTempDir temp_dir; + int fds[2] = {0}; + int minidump_fd = -1; + std::string minidump_path; + if (use_fd) { + minidump_fd = CreateTMPFile(temp_dir.path(), &minidump_path); + } else { + ASSERT_NE(pipe(fds), -1); + } + + const pid_t child = fork(); + if (child == 0) { + { + scoped_ptr handler; + if (use_fd) { + handler.reset(new ExceptionHandler(MinidumpDescriptor(minidump_fd), + NULL, NULL, NULL, true, -1)); + } else { + close(fds[0]); // Close the reading end. + void* fd_param = reinterpret_cast(fds[1]); + handler.reset(new ExceptionHandler(MinidumpDescriptor(temp_dir.path()), + NULL, DoneCallback, fd_param, + true, -1)); + } + // Crash with the exception handler in scope. + *reinterpret_cast(NULL) = 0; + } + } + if (!use_fd) + close(fds[1]); // Close the writting end. + + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGSEGV)); + + if (!use_fd) + ASSERT_NO_FATAL_FAILURE(ReadMinidumpPathFromPipe(fds[0], &minidump_path)); struct stat st; - ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + ASSERT_EQ(0, stat(minidump_path.c_str(), &st)); ASSERT_GT(st.st_size, 0u); - unlink(minidump_filename.c_str()); + unlink(minidump_path.c_str()); +} + +TEST(ExceptionHandlerTest, ChildCrashWithPath) { + ASSERT_NO_FATAL_FAILURE(ChildCrash(false)); +} + +TEST(ExceptionHandlerTest, ChildCrashWithFD) { + ASSERT_NO_FATAL_FAILURE(ChildCrash(true)); } // Test that memory around the instruction pointer is written @@ -158,8 +219,9 @@ TEST(ExceptionHandlerTest, InstructionPointerMemory) { const pid_t child = fork(); if (child == 0) { close(fds[0]); - ExceptionHandler handler(temp_dir.path(), NULL, DoneCallback, - (void*) fds[1], true); + ExceptionHandler handler(MinidumpDescriptor(temp_dir.path()), NULL, + DoneCallback, reinterpret_cast(fds[1]), + true, -1); // Get some executable memory. char* memory = reinterpret_cast(mmap(NULL, @@ -179,45 +241,25 @@ TEST(ExceptionHandlerTest, InstructionPointerMemory) { // Now execute the instructions, which should crash. typedef void (*void_function)(void); void_function memory_function = - reinterpret_cast(memory + kOffset); + reinterpret_cast(memory + kOffset); memory_function(); } close(fds[1]); - int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); - ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGILL); + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGILL)); - struct pollfd pfd; - memset(&pfd, 0, sizeof(pfd)); - pfd.fd = fds[0]; - pfd.events = POLLIN | POLLERR; - - const int r = HANDLE_EINTR(poll(&pfd, 1, 0)); - ASSERT_EQ(r, 1); - ASSERT_TRUE(pfd.revents & POLLIN); - - uint32_t len; - ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len)); - ASSERT_LT(len, (uint32_t)2048); - char* filename = reinterpret_cast(malloc(len + 1)); - ASSERT_EQ(read(fds[0], filename, len), len); - filename[len] = 0; - close(fds[0]); - - const string minidump_filename = temp_dir.path() + "/" + filename + - ".dmp"; + string minidump_path; + ASSERT_NO_FATAL_FAILURE(ReadMinidumpPathFromPipe(fds[0], &minidump_path)); struct stat st; - ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + ASSERT_EQ(0, stat(minidump_path.c_str(), &st)); ASSERT_GT(st.st_size, 0u); // Read the minidump. Locate the exception record and the // memory list, and then ensure that there is a memory region // in the memory list that covers the instruction pointer from // the exception record. - Minidump minidump(minidump_filename); + Minidump minidump(minidump_path); ASSERT_TRUE(minidump.Read()); MinidumpException* exception = minidump.GetException(); @@ -246,7 +288,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemory) { } MinidumpMemoryRegion* region = - memory_list->GetMemoryRegionForAddress(instruction_pointer); + memory_list->GetMemoryRegionForAddress(instruction_pointer); ASSERT_TRUE(region); EXPECT_EQ(kMemorySize, region->GetSize()); @@ -262,8 +304,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemory) { EXPECT_TRUE(memcmp(bytes + kOffset + sizeof(instructions), suffix_bytes, sizeof(suffix_bytes)) == 0); - unlink(minidump_filename.c_str()); - free(filename); + unlink(minidump_path.c_str()); } // Test that the memory region around the instruction pointer is @@ -283,16 +324,17 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMinBound) { const pid_t child = fork(); if (child == 0) { close(fds[0]); - ExceptionHandler handler(temp_dir.path(), NULL, DoneCallback, - (void*) fds[1], true); + ExceptionHandler handler(MinidumpDescriptor(temp_dir.path()), NULL, + DoneCallback, reinterpret_cast(fds[1]), + true, -1); // Get some executable memory. char* memory = - reinterpret_cast(mmap(NULL, - kMemorySize, - PROT_READ | PROT_WRITE | PROT_EXEC, - MAP_PRIVATE | MAP_ANON, - -1, - 0)); + reinterpret_cast(mmap(NULL, + kMemorySize, + PROT_READ | PROT_WRITE | PROT_EXEC, + MAP_PRIVATE | MAP_ANON, + -1, + 0)); if (!memory) exit(0); @@ -304,45 +346,25 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMinBound) { // Now execute the instructions, which should crash. typedef void (*void_function)(void); void_function memory_function = - reinterpret_cast(memory + kOffset); + reinterpret_cast(memory + kOffset); memory_function(); } close(fds[1]); - int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); - ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGILL); + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGILL)); - struct pollfd pfd; - memset(&pfd, 0, sizeof(pfd)); - pfd.fd = fds[0]; - pfd.events = POLLIN | POLLERR; - - const int r = HANDLE_EINTR(poll(&pfd, 1, 0)); - ASSERT_EQ(r, 1); - ASSERT_TRUE(pfd.revents & POLLIN); - - uint32_t len; - ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len)); - ASSERT_LT(len, (uint32_t)2048); - char* filename = reinterpret_cast(malloc(len + 1)); - ASSERT_EQ(read(fds[0], filename, len), len); - filename[len] = 0; - close(fds[0]); - - const string minidump_filename = temp_dir.path() + "/" + filename + - ".dmp"; + string minidump_path; + ASSERT_NO_FATAL_FAILURE(ReadMinidumpPathFromPipe(fds[0], &minidump_path)); struct stat st; - ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + ASSERT_EQ(0, stat(minidump_path.c_str(), &st)); ASSERT_GT(st.st_size, 0u); // Read the minidump. Locate the exception record and the // memory list, and then ensure that there is a memory region // in the memory list that covers the instruction pointer from // the exception record. - Minidump minidump(minidump_filename); + Minidump minidump(minidump_path); ASSERT_TRUE(minidump.Read()); MinidumpException* exception = minidump.GetException(); @@ -371,7 +393,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMinBound) { } MinidumpMemoryRegion* region = - memory_list->GetMemoryRegionForAddress(instruction_pointer); + memory_list->GetMemoryRegionForAddress(instruction_pointer); ASSERT_TRUE(region); EXPECT_EQ(kMemorySize / 2, region->GetSize()); @@ -383,9 +405,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMinBound) { EXPECT_TRUE(memcmp(bytes + kOffset, instructions, sizeof(instructions)) == 0); EXPECT_TRUE(memcmp(bytes + kOffset + sizeof(instructions), suffix_bytes, sizeof(suffix_bytes)) == 0); - - unlink(minidump_filename.c_str()); - free(filename); + unlink(minidump_path.c_str()); } // Test that the memory region around the instruction pointer is @@ -408,16 +428,17 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMaxBound) { const pid_t child = fork(); if (child == 0) { close(fds[0]); - ExceptionHandler handler(temp_dir.path(), NULL, DoneCallback, - (void*) fds[1], true); + ExceptionHandler handler(MinidumpDescriptor(temp_dir.path()), NULL, + DoneCallback, reinterpret_cast(fds[1]), + true, -1); // Get some executable memory. char* memory = - reinterpret_cast(mmap(NULL, - kMemorySize, - PROT_READ | PROT_WRITE | PROT_EXEC, - MAP_PRIVATE | MAP_ANON, - -1, - 0)); + reinterpret_cast(mmap(NULL, + kMemorySize, + PROT_READ | PROT_WRITE | PROT_EXEC, + MAP_PRIVATE | MAP_ANON, + -1, + 0)); if (!memory) exit(0); @@ -429,45 +450,24 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMaxBound) { // Now execute the instructions, which should crash. typedef void (*void_function)(void); void_function memory_function = - reinterpret_cast(memory + kOffset); + reinterpret_cast(memory + kOffset); memory_function(); } close(fds[1]); - int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); - ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGILL); + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGILL)); - struct pollfd pfd; - memset(&pfd, 0, sizeof(pfd)); - pfd.fd = fds[0]; - pfd.events = POLLIN | POLLERR; - - const int r = HANDLE_EINTR(poll(&pfd, 1, 0)); - ASSERT_EQ(r, 1); - ASSERT_TRUE(pfd.revents & POLLIN); - - uint32_t len; - ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len)); - ASSERT_LT(len, (uint32_t)2048); - char* filename = reinterpret_cast(malloc(len + 1)); - ASSERT_EQ(read(fds[0], filename, len), len); - filename[len] = 0; - close(fds[0]); - - const string minidump_filename = temp_dir.path() + "/" + filename + - ".dmp"; + string minidump_path; + ASSERT_NO_FATAL_FAILURE(ReadMinidumpPathFromPipe(fds[0], &minidump_path)); struct stat st; - ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + ASSERT_EQ(0, stat(minidump_path.c_str(), &st)); ASSERT_GT(st.st_size, 0u); - // Read the minidump. Locate the exception record and the - // memory list, and then ensure that there is a memory region - // in the memory list that covers the instruction pointer from - // the exception record. - Minidump minidump(minidump_filename); + // Read the minidump. Locate the exception record and the memory list, and + // then ensure that there is a memory region in the memory list that covers + // the instruction pointer from the exception record. + Minidump minidump(minidump_path); ASSERT_TRUE(minidump.Read()); MinidumpException* exception = minidump.GetException(); @@ -496,7 +496,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMaxBound) { } MinidumpMemoryRegion* region = - memory_list->GetMemoryRegionForAddress(instruction_pointer); + memory_list->GetMemoryRegionForAddress(instruction_pointer); ASSERT_TRUE(region); const size_t kPrefixSize = 128; // bytes @@ -510,8 +510,7 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMaxBound) { EXPECT_TRUE(memcmp(bytes + kPrefixSize, instructions, sizeof(instructions)) == 0); - unlink(minidump_filename.c_str()); - free(filename); + unlink(minidump_path.c_str()); } // If AddressSanitizer is used, NULL pointer dereferences generate SIGILL @@ -520,89 +519,52 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryMaxBound) { // this test if AddressSanitizer is used. #ifndef ADDRESS_SANITIZER -// Ensure that an extra memory block doesn't get added when the -// instruction pointer is not in mapped memory. +// Ensure that an extra memory block doesn't get added when the instruction +// pointer is not in mapped memory. TEST(ExceptionHandlerTest, InstructionPointerMemoryNullPointer) { AutoTempDir temp_dir; int fds[2]; ASSERT_NE(pipe(fds), -1); - const pid_t child = fork(); if (child == 0) { close(fds[0]); - ExceptionHandler handler(temp_dir.path(), NULL, DoneCallback, - (void*) fds[1], true); + ExceptionHandler handler(MinidumpDescriptor(temp_dir.path()), NULL, + DoneCallback, reinterpret_cast(fds[1]), + true, -1); // Try calling a NULL pointer. typedef void (*void_function)(void); - void_function memory_function = - reinterpret_cast(NULL); + void_function memory_function = reinterpret_cast(NULL); memory_function(); } close(fds[1]); - int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); - ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGSEGV); + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGSEGV)); - struct pollfd pfd; - memset(&pfd, 0, sizeof(pfd)); - pfd.fd = fds[0]; - pfd.events = POLLIN | POLLERR; - - const int r = HANDLE_EINTR(poll(&pfd, 1, 0)); - ASSERT_EQ(r, 1); - ASSERT_TRUE(pfd.revents & POLLIN); - - uint32_t len; - ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len)); - ASSERT_LT(len, (uint32_t)2048); - char* filename = reinterpret_cast(malloc(len + 1)); - ASSERT_EQ(read(fds[0], filename, len), len); - filename[len] = 0; - close(fds[0]); - - const string minidump_filename = temp_dir.path() + "/" + filename + - ".dmp"; + string minidump_path; + ASSERT_NO_FATAL_FAILURE(ReadMinidumpPathFromPipe(fds[0], &minidump_path)); struct stat st; - ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + ASSERT_EQ(0, stat(minidump_path.c_str(), &st)); ASSERT_GT(st.st_size, 0u); // Read the minidump. Locate the exception record and the // memory list, and then ensure that there is a memory region // in the memory list that covers the instruction pointer from // the exception record. - Minidump minidump(minidump_filename); + Minidump minidump(minidump_path); ASSERT_TRUE(minidump.Read()); MinidumpException* exception = minidump.GetException(); MinidumpMemoryList* memory_list = minidump.GetMemoryList(); ASSERT_TRUE(exception); ASSERT_TRUE(memory_list); - ASSERT_EQ((unsigned int)1, memory_list->region_count()); + ASSERT_EQ(static_cast(1), memory_list->region_count()); - unlink(minidump_filename.c_str()); - free(filename); + unlink(minidump_path.c_str()); } #endif // !ADDRESS_SANITIZER -static bool SimpleCallback(const char* dump_path, - const char* minidump_id, - void* context, - bool succeeded) { - if (!succeeded) - return succeeded; - - string* minidump_file = reinterpret_cast(context); - minidump_file->append(dump_path); - minidump_file->append("/"); - minidump_file->append(minidump_id); - minidump_file->append(".dmp"); - return true; -} - // Test that anonymous memory maps can be annotated with names and IDs. TEST(ExceptionHandlerTest, ModuleInfo) { // These are defined here so the parent can use them to check the @@ -629,37 +591,37 @@ TEST(ExceptionHandlerTest, ModuleInfo) { // Get some memory. char* memory = - reinterpret_cast(mmap(NULL, - kMemorySize, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANON, - -1, - 0)); + reinterpret_cast(mmap(NULL, + kMemorySize, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON, + -1, + 0)); const uintptr_t kMemoryAddress = reinterpret_cast(memory); ASSERT_TRUE(memory); - string minidump_filename; AutoTempDir temp_dir; - ExceptionHandler handler(temp_dir.path(), NULL, SimpleCallback, - (void*)&minidump_filename, true); + ExceptionHandler handler( + MinidumpDescriptor(temp_dir.path()), NULL, NULL, NULL, true, -1); + // Add info about the anonymous memory mapping. handler.AddMappingInfo(kMemoryName, kModuleGUID, kMemoryAddress, kMemorySize, 0); - handler.WriteMinidump(); + ASSERT_TRUE(handler.WriteMinidump()); - // Read the minidump. Load the module list, and ensure that - // the mmap'ed |memory| is listed with the given module name - // and debug ID. - Minidump minidump(minidump_filename); + const MinidumpDescriptor& minidump_desc = handler.minidump_descriptor(); + // Read the minidump. Load the module list, and ensure that the mmap'ed + // |memory| is listed with the given module name and debug ID. + Minidump minidump(minidump_desc.path()); ASSERT_TRUE(minidump.Read()); MinidumpModuleList* module_list = minidump.GetModuleList(); ASSERT_TRUE(module_list); const MinidumpModule* module = - module_list->GetModuleForAddress(kMemoryAddress); + module_list->GetModuleForAddress(kMemoryAddress); ASSERT_TRUE(module); EXPECT_EQ(kMemoryAddress, module->base_address()); @@ -667,7 +629,7 @@ TEST(ExceptionHandlerTest, ModuleInfo) { EXPECT_EQ(kMemoryName, module->code_file()); EXPECT_EQ(module_identifier, module->debug_identifier()); - unlink(minidump_filename.c_str()); + unlink(minidump_desc.path()); } static const unsigned kControlMsgSize = @@ -732,7 +694,8 @@ TEST(ExceptionHandlerTest, ExternalDumper) { const pid_t child = fork(); if (child == 0) { close(fds[0]); - ExceptionHandler handler("/tmp1", NULL, NULL, (void*) fds[1], true); + ExceptionHandler handler(MinidumpDescriptor("/tmp1"), NULL, NULL, + reinterpret_cast(fds[1]), true, -1); handler.set_crash_handler(CrashHandler); *reinterpret_cast(NULL) = 0; } @@ -751,10 +714,10 @@ TEST(ExceptionHandlerTest, ExternalDumper) { msg.msg_controllen = kControlMsgSize; const ssize_t n = HANDLE_EINTR(recvmsg(fds[0], &msg, 0)); - ASSERT_EQ(n, kCrashContextSize); - ASSERT_EQ(msg.msg_controllen, kControlMsgSize); - ASSERT_EQ(msg.msg_flags, 0); - ASSERT_EQ(close(fds[0]), 0); + ASSERT_EQ(kCrashContextSize, n); + ASSERT_EQ(kControlMsgSize, msg.msg_controllen); + ASSERT_EQ(0, msg.msg_flags); + ASSERT_EQ(0, close(fds[0])); pid_t crashing_pid = -1; int signal_fd = -1; @@ -765,8 +728,8 @@ TEST(ExceptionHandlerTest, ExternalDumper) { if (hdr->cmsg_type == SCM_RIGHTS) { const unsigned len = hdr->cmsg_len - (((uint8_t*)CMSG_DATA(hdr)) - (uint8_t*)hdr); - ASSERT_EQ(len, sizeof(int)); - signal_fd = *((int *) CMSG_DATA(hdr)); + ASSERT_EQ(sizeof(int), len); + signal_fd = *(reinterpret_cast(CMSG_DATA(hdr))); } else if (hdr->cmsg_type == SCM_CREDENTIALS) { const struct ucred *cred = reinterpret_cast(CMSG_DATA(hdr)); @@ -783,15 +746,60 @@ TEST(ExceptionHandlerTest, ExternalDumper) { kCrashContextSize)); static const char b = 0; ASSERT_EQ(1U, (HANDLE_EINTR(write(signal_fd, &b, 1)))); - ASSERT_EQ(close(signal_fd), 0); + ASSERT_EQ(0, close(signal_fd)); - int status; - ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1); - ASSERT_TRUE(WIFSIGNALED(status)); - ASSERT_EQ(WTERMSIG(status), SIGSEGV); + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGSEGV)); struct stat st; - ASSERT_EQ(stat(templ.c_str(), &st), 0); - ASSERT_GT(st.st_size, 0u); + ASSERT_EQ(0, stat(templ.c_str(), &st)); + ASSERT_GT(st.st_size, 0U); unlink(templ.c_str()); } + +TEST(ExceptionHandlerTest, GenerateMultipleDumpsWithFD) { + AutoTempDir temp_dir; + std::string path; + const int fd = CreateTMPFile(temp_dir.path(), &path); + ExceptionHandler handler(MinidumpDescriptor(fd), NULL, NULL, NULL, false, -1); + ASSERT_TRUE(handler.WriteMinidump()); + // Check by the size of the data written to the FD that a minidump was + // generated. + off_t size = lseek(fd, 0, SEEK_CUR); + ASSERT_GT(size, 0); + + // Generate another minidump. + ASSERT_TRUE(handler.WriteMinidump()); + size = lseek(fd, 0, SEEK_CUR); + ASSERT_GT(size, 0); +} + +TEST(ExceptionHandlerTest, GenerateMultipleDumpsWithPath) { + AutoTempDir temp_dir; + ExceptionHandler handler(MinidumpDescriptor(temp_dir.path()), NULL, NULL, + NULL, false, -1); + ASSERT_TRUE(handler.WriteMinidump()); + + const MinidumpDescriptor& minidump_1 = handler.minidump_descriptor(); + struct stat st; + ASSERT_EQ(0, stat(minidump_1.path(), &st)); + ASSERT_GT(st.st_size, 0U); + std::string minidump_1_path(minidump_1.path()); + // Check it is a valid minidump. + Minidump minidump1(minidump_1_path); + ASSERT_TRUE(minidump1.Read()); + unlink(minidump_1.path()); + + // Generate another minidump, it should go to a different file. + ASSERT_TRUE(handler.WriteMinidump()); + const MinidumpDescriptor& minidump_2 = handler.minidump_descriptor(); + ASSERT_EQ(0, stat(minidump_2.path(), &st)); + ASSERT_GT(st.st_size, 0U); + std::string minidump_2_path(minidump_2.path()); + // Check it is a valid minidump. + Minidump minidump2(minidump_2_path); + ASSERT_TRUE(minidump2.Read()); + unlink(minidump_2.path()); + + // We expect 2 distinct files. + ASSERT_STRNE(minidump_1_path.c_str(), minidump_2_path.c_str()); +} diff --git a/src/client/linux/handler/minidump_descriptor.cc b/src/client/linux/handler/minidump_descriptor.cc new file mode 100644 index 00000000..130764af --- /dev/null +++ b/src/client/linux/handler/minidump_descriptor.cc @@ -0,0 +1,61 @@ +// Copyright (c) 2012 Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include + +#include "client/linux/handler/minidump_descriptor.h" + +#include "common/linux/guid_creator.h" + +namespace google_breakpad { + +MinidumpDescriptor::MinidumpDescriptor(const MinidumpDescriptor& descriptor) + : fd_(descriptor.fd_), + directory_(descriptor.directory_), + c_path_(NULL) { + // The copy constructor is not allowed to be called on a MinidumpDescriptor + // with a valid path_, as getting its c_path_ would require the heap which + // can cause problems in compromised environments. + assert(descriptor.path_.empty()); +} + +void MinidumpDescriptor::UpdatePath() { + assert(fd_ == -1 && !directory_.empty()); + + GUID guid; + char guid_str[kGUIDStringLength + 1]; + bool r = CreateGUID(&guid) && GUIDToString(&guid, guid_str, sizeof(guid_str)); + assert(r); + + path_.clear(); + path_ = directory_ + "/" + guid_str + ".dmp"; + c_path_ = path_.c_str(); +} + +} // namespace google_breakpad diff --git a/src/client/linux/handler/minidump_descriptor.h b/src/client/linux/handler/minidump_descriptor.h new file mode 100644 index 00000000..18e2cb41 --- /dev/null +++ b/src/client/linux/handler/minidump_descriptor.h @@ -0,0 +1,84 @@ +// Copyright (c) 2012 Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#ifndef CLIENT_LINUX_HANDLER_MINIDUMP_DESCRIPTOR_H_ +#define CLIENT_LINUX_HANDLER_MINIDUMP_DESCRIPTOR_H_ + +#include +#include + +// The MinidumpDescriptor describes how to access a minidump: it can contain +// either a file descriptor or a path. +// Note that when using files, it is created with the path to a directory. +// The actual path where the minidump is generated is created by this class. +namespace google_breakpad { + +class MinidumpDescriptor { + public: + MinidumpDescriptor() : fd_(-1) {} + + explicit MinidumpDescriptor(const std::string& directory) + : fd_(-1), + directory_(directory), + c_path_(NULL) { + assert(!directory.empty()); + } + + explicit MinidumpDescriptor(int fd) : fd_(fd), c_path_(NULL) { + assert(fd != -1); + } + + explicit MinidumpDescriptor(const MinidumpDescriptor& descriptor); + + bool IsFD() const { return fd_ != -1; } + + int fd() const { return fd_; } + + const char* path() const { return c_path_; } + + // Updates the path so it is unique. + // Should be called from a normal context: this methods uses the heap. + void UpdatePath(); + + private: + // The file descriptor where the minidump is generated. + const int fd_; + + // The directory where the minidump should be generated. + const std::string directory_; + // The full path to the generated minidump. + std::string path_; + // The C string of |path_|. Precomputed so it can be access from a compromised + // context. + const char* c_path_; +}; + +} // namespace google_breakpad + +#endif // CLIENT_LINUX_HANDLER_MINIDUMP_DESCRIPTOR_H_ diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 89fc96ff..fefef1d2 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -43,6 +43,7 @@ // a canonical instance in the LinuxDumper object. We use the placement // new form to allocate objects and we don't delete them. +#include "client/linux/handler/minidump_descriptor.h" #include "client/linux/minidump_writer/minidump_writer.h" #include "client/minidump_file_writer-inl.h" @@ -81,6 +82,22 @@ #include "google_breakpad/common/minidump_format.h" #include "third_party/lss/linux_syscall_support.h" +namespace { + +using google_breakpad::ExceptionHandler; +using google_breakpad::LineReader; +using google_breakpad::LinuxDumper; +using google_breakpad::LinuxPtraceDumper; +using google_breakpad::MappingEntry; +using google_breakpad::MappingInfo; +using google_breakpad::MappingList; +using google_breakpad::MinidumpFileWriter; +using google_breakpad::PageAllocator; +using google_breakpad::ThreadInfo; +using google_breakpad::TypedMDRVA; +using google_breakpad::UntypedMDRVA; +using google_breakpad::wasteful_vector; + // Minidump defines register structures which are different from the raw // structures which we get from the kernel. These are platform specific // functions to juggle the ucontext and user structures into minidump format. @@ -90,22 +107,22 @@ typedef MDRawContextX86 RawContextCPU; // Write a uint16_t to memory // out: memory location to write to // v: value to write. -static void U16(void* out, uint16_t v) { +void U16(void* out, uint16_t v) { my_memcpy(out, &v, sizeof(v)); } // Write a uint32_t to memory // out: memory location to write to // v: value to write. -static void U32(void* out, uint32_t v) { +void U32(void* out, uint32_t v) { my_memcpy(out, &v, sizeof(v)); } // Juggle an x86 user_(fp|fpx|)regs_struct into minidump format // out: the minidump structure // info: the collection of register structures. -static void CPUFillFromThreadInfo(MDRawContextX86 *out, - const google_breakpad::ThreadInfo &info) { +void CPUFillFromThreadInfo(MDRawContextX86 *out, + const google_breakpad::ThreadInfo &info) { out->context_flags = MD_CONTEXT_X86_ALL; out->dr0 = info.dregs[0]; @@ -165,8 +182,8 @@ static void CPUFillFromThreadInfo(MDRawContextX86 *out, // Juggle an x86 ucontext into minidump format // out: the minidump structure // info: the collection of register structures. -static void CPUFillFromUContext(MDRawContextX86 *out, const ucontext *uc, - const struct _libc_fpstate* fp) { +void CPUFillFromUContext(MDRawContextX86 *out, const ucontext *uc, + const struct _libc_fpstate* fp) { const greg_t* regs = uc->uc_mcontext.gregs; out->context_flags = MD_CONTEXT_X86_FULL | @@ -206,8 +223,8 @@ static void CPUFillFromUContext(MDRawContextX86 *out, const ucontext *uc, #elif defined(__x86_64) typedef MDRawContextAMD64 RawContextCPU; -static void CPUFillFromThreadInfo(MDRawContextAMD64 *out, - const google_breakpad::ThreadInfo &info) { +void CPUFillFromThreadInfo(MDRawContextAMD64 *out, + const google_breakpad::ThreadInfo &info) { out->context_flags = MD_CONTEXT_AMD64_FULL | MD_CONTEXT_AMD64_SEGMENTS; @@ -265,8 +282,8 @@ static void CPUFillFromThreadInfo(MDRawContextAMD64 *out, my_memcpy(&out->flt_save.xmm_registers, &info.fpregs.xmm_space, 16 * 16); } -static void CPUFillFromUContext(MDRawContextAMD64 *out, const ucontext *uc, - const struct _libc_fpstate* fpregs) { +void CPUFillFromUContext(MDRawContextAMD64 *out, const ucontext *uc, + const struct _libc_fpstate* fpregs) { const greg_t* regs = uc->uc_mcontext.gregs; out->context_flags = MD_CONTEXT_AMD64_FULL; @@ -315,8 +332,8 @@ static void CPUFillFromUContext(MDRawContextAMD64 *out, const ucontext *uc, #elif defined(__ARMEL__) typedef MDRawContextARM RawContextCPU; -static void CPUFillFromThreadInfo(MDRawContextARM *out, - const google_breakpad::ThreadInfo &info) { +void CPUFillFromThreadInfo(MDRawContextARM* out, + const google_breakpad::ThreadInfo& info) { out->context_flags = MD_CONTEXT_ARM_FULL; for (int i = 0; i < MD_CONTEXT_ARM_GPR_COUNT; ++i) @@ -332,8 +349,8 @@ static void CPUFillFromThreadInfo(MDRawContextARM *out, #endif } -static void CPUFillFromUContext(MDRawContextARM *out, const ucontext *uc, - const struct _libc_fpstate* fpregs) { +void CPUFillFromUContext(MDRawContextARM* out, const ucontext* uc, + const struct _libc_fpstate* fpregs) { out->context_flags = MD_CONTEXT_ARM_FULL; out->iregs[0] = uc->uc_mcontext.arm_r0; @@ -366,15 +383,15 @@ static void CPUFillFromUContext(MDRawContextARM *out, const ucontext *uc, #error "This code has not been ported to your platform yet." #endif -namespace google_breakpad { - class MinidumpWriter { public: - MinidumpWriter(const char* filename, + MinidumpWriter(const char* minidump_path, + int minidump_fd, const ExceptionHandler::CrashContext* context, const MappingList& mappings, LinuxDumper* dumper) - : filename_(filename), + : fd_(minidump_fd), + path_(minidump_path), ucontext_(context ? &context->context : NULL), #if !defined(__ARM_EABI__) float_state_(context ? &context->float_state : NULL), @@ -385,15 +402,28 @@ class MinidumpWriter { dumper_(dumper), memory_blocks_(dumper_->allocator()), mapping_list_(mappings) { + // Assert there should be either a valid fd or a valid path, not both. + assert(fd_ != -1 || minidump_path); + assert(fd_ == -1 || !minidump_path); } bool Init() { - return dumper_->Init() && minidump_writer_.Open(filename_) && - dumper_->ThreadsSuspend(); + if (!dumper_->Init()) + return false; + + if (fd_ != -1) + minidump_writer_.SetFile(fd_); + else if (!minidump_writer_.Open(path_)) + return false; + + return dumper_->ThreadsSuspend(); } ~MinidumpWriter() { - minidump_writer_.Close(); + // Don't close the file descriptor when it's been provided explicitly. + // Callers might still need to use it. + if (fd_ == -1) + minidump_writer_.Close(); dumper_->ThreadsResume(); } @@ -1324,7 +1354,10 @@ class MinidumpWriter { return WriteFile(result, buf); } - const char* const filename_; // output filename + // Only one of the 2 member variables below should be set to a valid value. + const int fd_; // File descriptor where the minidum should be written. + const char* path_; // Path to the file where the minidum should be written. + const struct ucontext* const ucontext_; // also from the signal handler const struct _libc_fpstate* const float_state_; // ditto LinuxDumper* dumper_; @@ -1338,15 +1371,12 @@ class MinidumpWriter { const MappingList& mapping_list_; }; -bool WriteMinidump(const char* filename, pid_t crashing_process, - const void* blob, size_t blob_size) { - MappingList m; - return WriteMinidump(filename, crashing_process, blob, blob_size, m); -} -bool WriteMinidump(const char* filename, pid_t crashing_process, - const void* blob, size_t blob_size, - const MappingList& mappings) { +bool WriteMinidumpImpl(const char* minidump_path, + int minidump_fd, + pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings) { if (blob_size != sizeof(ExceptionHandler::CrashContext)) return false; const ExceptionHandler::CrashContext* context = @@ -1356,16 +1386,46 @@ bool WriteMinidump(const char* filename, pid_t crashing_process, reinterpret_cast(context->siginfo.si_addr)); dumper.set_crash_signal(context->siginfo.si_signo); dumper.set_crash_thread(context->tid); - MinidumpWriter writer(filename, context, mappings, &dumper); + MinidumpWriter writer(minidump_path, minidump_fd, context, mappings, &dumper); if (!writer.Init()) return false; return writer.Dump(); } +} // namespace + +namespace google_breakpad { + +bool WriteMinidump(const char* minidump_path, pid_t crashing_process, + const void* blob, size_t blob_size) { + return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size, + MappingList()); +} + +bool WriteMinidump(int minidump_fd, pid_t crashing_process, + const void* blob, size_t blob_size) { + return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size, + MappingList()); +} + +bool WriteMinidump(const char* minidump_path, pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings) { + return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size, + mappings); +} + +bool WriteMinidump(int minidump_fd, pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings) { + return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size, + mappings); +} + bool WriteMinidump(const char* filename, const MappingList& mappings, LinuxDumper* dumper) { - MinidumpWriter writer(filename, NULL, mappings, dumper); + MinidumpWriter writer(filename, -1, NULL, mappings, dumper); if (!writer.Init()) return false; return writer.Dump(); diff --git a/src/client/linux/minidump_writer/minidump_writer.h b/src/client/linux/minidump_writer/minidump_writer.h index e79eb79b..14da94b6 100644 --- a/src/client/linux/minidump_writer/minidump_writer.h +++ b/src/client/linux/minidump_writer/minidump_writer.h @@ -51,21 +51,27 @@ struct MappingEntry { // A list of typedef std::list MappingList; -// Write a minidump to the filesystem. This function does not malloc nor use +// Writes a minidump to the filesystem. These functions do not malloc nor use // libc functions which may. Thus, it can be used in contexts where the state // of the heap may be corrupt. -// filename: the filename to write to. This is opened O_EXCL and fails if -// open fails. +// minidump_path: the path to the file to write to. This is opened O_EXCL and +// fails open fails. // crashing_process: the pid of the crashing process. This must be trusted. // blob: a blob of data from the crashing process. See exception_handler.h // blob_size: the length of |blob|, in bytes // // Returns true iff successful. -bool WriteMinidump(const char* filename, pid_t crashing_process, +bool WriteMinidump(const char* minidump_path, pid_t crashing_process, + const void* blob, size_t blob_size); +// Same as above but takes an open file descriptor instead of a path. +bool WriteMinidump(int minidump_fd, pid_t crashing_process, const void* blob, size_t blob_size); -// This overload also allows passing a list of known mappings. -bool WriteMinidump(const char* filename, pid_t crashing_process, +// These overloads also allow passing a list of known mappings. +bool WriteMinidump(const char* minidump_path, pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings); +bool WriteMinidump(int minidump_fd, pid_t crashing_process, const void* blob, size_t blob_size, const MappingList& mappings); diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index 31e1440d..1fff015f 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -60,7 +60,7 @@ namespace { typedef testing::Test MinidumpWriterTest; } -TEST(MinidumpWriterTest, Setup) { +TEST(MinidumpWriterTest, SetupWithPath) { int fds[2]; ASSERT_NE(-1, pipe(fds)); @@ -89,6 +89,36 @@ TEST(MinidumpWriterTest, Setup) { close(fds[1]); } +TEST(MinidumpWriterTest, SetupWithFD) { + int fds[2]; + ASSERT_NE(-1, pipe(fds)); + + const pid_t child = fork(); + if (child == 0) { + close(fds[1]); + char b; + HANDLE_EINTR(read(fds[0], &b, sizeof(b))); + close(fds[0]); + syscall(__NR_exit); + } + close(fds[0]); + + ExceptionHandler::CrashContext context; + memset(&context, 0, sizeof(context)); + + AutoTempDir temp_dir; + string templ = temp_dir.path() + "/minidump-writer-unittest"; + int fd = open(templ.c_str(), O_CREAT | O_WRONLY, S_IRWXU); + // Set a non-zero tid to avoid tripping asserts. + context.tid = 1; + ASSERT_TRUE(WriteMinidump(fd, child, &context, sizeof(context))); + struct stat st; + ASSERT_EQ(stat(templ.c_str(), &st), 0); + ASSERT_GT(st.st_size, 0u); + + close(fds[1]); +} + // Test that mapping info can be specified when writing a minidump, // and that it ends up in the module list of the minidump. TEST(MinidumpWriterTest, MappingInfo) { @@ -261,9 +291,8 @@ TEST(MinidumpWriterTest, MappingInfoContained) { mapping.first = info; memcpy(mapping.second, kModuleGUID, sizeof(MDGUID)); mappings.push_back(mapping); - ASSERT_TRUE( - WriteMinidump(dumpfile.c_str(), child, &context, sizeof(context), - mappings)); + ASSERT_TRUE(WriteMinidump(dumpfile.c_str(), child, &context, sizeof(context), + mappings)); // Read the minidump. Load the module list, and ensure that // the mmap'ed |memory| is listed with the given module name @@ -355,8 +384,6 @@ TEST(MinidumpWriterTest, DeletedBinary) { ASSERT_EQ(stat(templ.c_str(), &st), 0); ASSERT_GT(st.st_size, 0u); - - Minidump minidump(templ.c_str()); ASSERT_TRUE(minidump.Read()); diff --git a/src/client/minidump_file_writer.cc b/src/client/minidump_file_writer.cc index db304662..c2674103 100644 --- a/src/client/minidump_file_writer.cc +++ b/src/client/minidump_file_writer.cc @@ -48,11 +48,16 @@ namespace google_breakpad { const MDRVA MinidumpFileWriter::kInvalidMDRVA = static_cast(-1); -MinidumpFileWriter::MinidumpFileWriter() : file_(-1), position_(0), size_(0) { +MinidumpFileWriter::MinidumpFileWriter() + : file_(-1), + close_file_when_destroyed_(true), + position_(0), + size_(0) { } MinidumpFileWriter::~MinidumpFileWriter() { - Close(); + if (close_file_when_destroyed_) + Close(); } bool MinidumpFileWriter::Open(const char *path) { @@ -66,6 +71,12 @@ bool MinidumpFileWriter::Open(const char *path) { return file_ != -1; } +void MinidumpFileWriter::SetFile(const int file) { + assert(file_ == -1); + file_ = file; + close_file_when_destroyed_ = false; +} + bool MinidumpFileWriter::Close() { bool result = true; diff --git a/src/client/minidump_file_writer.h b/src/client/minidump_file_writer.h index 21d4ce68..313b250b 100644 --- a/src/client/minidump_file_writer.h +++ b/src/client/minidump_file_writer.h @@ -55,6 +55,16 @@ template class TypedMDRVA; // header->get()->signature = MD_HEADER_SIGNATURE; // : // writer.Close(); +// +// An alternative is to use SetFile and provide a file descriptor: +// MinidumpFileWriter writer; +// writer.SetFile(minidump_fd); +// TypedMDRVA header(&writer_); +// header.Allocate(); +// header->get()->signature = MD_HEADER_SIGNATURE; +// : +// writer.Close(); + class MinidumpFileWriter { public: // Invalid MDRVA (Minidump Relative Virtual Address) @@ -66,11 +76,19 @@ public: // Open |path| as the destination of the minidump data. Any existing file // will be overwritten. - // Return true on success, or false on failure + // Return true on success, or false on failure. bool Open(const char *path); - // Close the current file - // Return true on success, or false on failure + // Sets the file descriptor |file| as the destination of the minidump data. + // Can be used as an alternative to Open() when a file descriptor is + // available. + // Note that |fd| is not closed when the instance of MinidumpFileWriter is + // destroyed. + void SetFile(const int file); + + // Close the current file (that was either created when Open was called, or + // specified with SetFile). + // Return true on success, or false on failure. bool Close(); // Copy the contents of |str| to a MDString and write it to the file. @@ -106,9 +124,12 @@ public: // unable to allocate the bytes. MDRVA Allocate(size_t size); - // The file descriptor for the output file + // The file descriptor for the output file. int file_; + // Whether |file_| should be closed when the instance is destroyed. + bool close_file_when_destroyed_; + // Current position in buffer MDRVA position_; diff --git a/src/common/tests/auto_tempdir.h b/src/common/tests/auto_tempdir.h index 9c84177c..a54bc053 100644 --- a/src/common/tests/auto_tempdir.h +++ b/src/common/tests/auto_tempdir.h @@ -51,7 +51,7 @@ namespace google_breakpad { class AutoTempDir { public: AutoTempDir() { - char temp_dir[] = TEMPDIR "/breakpad.XXXXXXXXXX"; + char temp_dir[] = TEMPDIR "/breakpad.XXXXXX"; EXPECT_TRUE(mkdtemp(temp_dir) != NULL); path_.assign(temp_dir); }