diff options
author | ted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2012-07-19 22:03:39 +0000 |
---|---|---|
committer | ted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2012-07-19 22:03:39 +0000 |
commit | 2f56276fbfe519913845565f178cfa385da93657 (patch) | |
tree | f031e25d09fd0ec8bdb7fc8837979ae4f2bf1982 /src/client | |
parent | Add some more DWARF ARM register numbers to the DWARF CFI register maps (diff) | |
download | breakpad-2f56276fbfe519913845565f178cfa385da93657.tar.xz |
Allow adding extra memory regions to minidump on linux/windows
A=Bill McCloskey <wmccloskey@mozilla.com> R=ted at https://bugzilla.mozilla.org/show_bug.cgi?id=662646
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@989 4c0a9323-5329-0410-9bdc-e9ce6186880e
Diffstat (limited to 'src/client')
-rw-r--r-- | src/client/linux/handler/exception_handler.cc | 18 | ||||
-rw-r--r-- | src/client/linux/handler/exception_handler.h | 11 | ||||
-rw-r--r-- | src/client/linux/handler/exception_handler_unittest.cc | 39 | ||||
-rw-r--r-- | src/client/linux/minidump_writer/minidump_writer.cc | 45 | ||||
-rw-r--r-- | src/client/linux/minidump_writer/minidump_writer.h | 17 | ||||
-rw-r--r-- | src/client/linux/minidump_writer/minidump_writer_unittest.cc | 73 | ||||
-rwxr-xr-x[-rw-r--r--] | src/client/windows/handler/exception_handler.cc | 60 | ||||
-rw-r--r-- | src/client/windows/handler/exception_handler.h | 20 | ||||
-rwxr-xr-x[-rw-r--r--] | src/client/windows/unittests/exception_handler_test.cc | 46 |
9 files changed, 301 insertions, 28 deletions
diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index f505ff33..fffc3e64 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -465,7 +465,8 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, crashing_process, context, context_size, - mapping_list_); + mapping_list_, + app_memory_list_); } // static @@ -515,4 +516,19 @@ void ExceptionHandler::AddMappingInfo(const string& name, mapping_list_.push_back(mapping); } +void ExceptionHandler::RegisterAppMemory(void *ptr, size_t length) { + app_memory_list_.push_back(AppMemory(ptr, length)); +} + +void ExceptionHandler::UnregisterAppMemory(void *ptr) { + for (AppMemoryList::iterator iter = app_memory_list_.begin(); + iter != app_memory_list_.end(); + ++iter) { + if (iter->ptr == ptr) { + app_memory_list_.erase(iter); + return; + } + } +} + } // namespace google_breakpad diff --git a/src/client/linux/handler/exception_handler.h b/src/client/linux/handler/exception_handler.h index 4bc8ba72..fab74d28 100644 --- a/src/client/linux/handler/exception_handler.h +++ b/src/client/linux/handler/exception_handler.h @@ -194,6 +194,13 @@ class ExceptionHandler { size_t mapping_size, size_t file_offset); + // Register a block of memory of len bytes starting at address p + // to be copied to the minidump when a crash happens. + void RegisterAppMemory(void *ptr, size_t length); + + // Unregister a block of memory that was registered with RegisterAppMemory. + void UnregisterAppMemory(void *ptr); + private: void Init(const string &dump_path, const int server_fd); @@ -252,6 +259,10 @@ class ExceptionHandler { // Callers can add extra info about mappings for cases where the // dumper code cannot extract enough information from /proc/<pid>/maps. MappingList mapping_list_; + + // Callers can request additional memory regions to be included in + // the dump. + AppMemoryList app_memory_list_; }; } // namespace google_breakpad diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index f9314729..2bad5fb5 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -785,3 +785,42 @@ TEST(ExceptionHandlerTest, ExternalDumper) { ASSERT_GT(st.st_size, 0u); unlink(templ.c_str()); } + +// Test that an additional memory region can be added to the minidump. +TEST(ExceptionHandlerTest, AdditionalMemory) { + const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE); + // Get some heap memory. + u_int8_t* memory = new u_int8_t[kMemorySize]; + const uintptr_t kMemoryAddress = reinterpret_cast<uintptr_t>(memory); + ASSERT_TRUE(memory); + // Stick some data into the memory so the contents can be verified. + for (unsigned int i = 0; i < kMemorySize; ++i) { + memory[i] = i % 255; + } + + string minidump_filename; + AutoTempDir temp_dir; + ExceptionHandler handler(temp_dir.path(), NULL, SimpleCallback, + (void*)&minidump_filename, true); + // Add the memory region to the list of memory to be included. + handler.RegisterAppMemory(memory, kMemorySize); + handler.WriteMinidump(); + + // Read the minidump. Ensure that the memory region is present + Minidump minidump(minidump_filename); + ASSERT_TRUE(minidump.Read()); + + MinidumpMemoryList* dump_memory_list = minidump.GetMemoryList(); + ASSERT_TRUE(dump_memory_list); + const MinidumpMemoryRegion* region = + dump_memory_list->GetMemoryRegionForAddress(kMemoryAddress); + ASSERT_TRUE(region); + + EXPECT_EQ(kMemoryAddress, region->GetBase()); + EXPECT_EQ(kMemorySize, region->GetSize()); + + // Verify memory contents. + EXPECT_EQ(0, memcmp(region->GetMemory(), memory, kMemorySize)); + + delete[] memory; +} diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 209e1720..f6ccff38 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -374,6 +374,7 @@ class MinidumpWriter { MinidumpWriter(const char* filename, const ExceptionHandler::CrashContext* context, const MappingList& mappings, + const AppMemoryList& appmem, LinuxDumper* dumper) : filename_(filename), ucontext_(context ? &context->context : NULL), @@ -385,7 +386,8 @@ class MinidumpWriter { #endif dumper_(dumper), memory_blocks_(dumper_->allocator()), - mapping_list_(mappings) { + mapping_list_(mappings), + app_memory_list_(appmem) { } bool Init() { @@ -459,6 +461,9 @@ class MinidumpWriter { return false; dir.CopyIndex(dir_index++, &dirent); + if (!WriteAppMemory()) + return false; + if (!WriteMemoryListStream(&dirent)) return false; dir.CopyIndex(dir_index++, &dirent); @@ -760,6 +765,30 @@ class MinidumpWriter { return true; } + // Write application-provided memory regions. + bool WriteAppMemory() { + for (AppMemoryList::const_iterator iter = app_memory_list_.begin(); + iter != app_memory_list_.end(); + ++iter) { + uint8_t* data_copy = + (uint8_t*) dumper_->allocator()->Alloc(iter->length); + dumper_->CopyFromProcess(data_copy, GetCrashThread(), iter->ptr, + iter->length); + + UntypedMDRVA memory(&minidump_writer_); + if (!memory.Allocate(iter->length)) { + return false; + } + memory.Copy(data_copy, iter->length); + MDMemoryDescriptor desc; + desc.start_of_memory_range = (uintptr_t)iter->ptr; + desc.memory = memory.location(); + memory_blocks_.push_back(desc); + } + + return true; + } + static bool ShouldIncludeMapping(const MappingInfo& mapping) { if (mapping.name[0] == 0 || // only want modules with filenames. mapping.offset || // only want to include one mapping per shared lib. @@ -1335,17 +1364,22 @@ class MinidumpWriter { wasteful_vector<MDMemoryDescriptor> memory_blocks_; // Additional information about some mappings provided by the caller. const MappingList& mapping_list_; + // Additional memory regions to be included in the dump, + // provided by the caller. + const AppMemoryList& app_memory_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); + AppMemoryList a; + return WriteMinidump(filename, crashing_process, blob, blob_size, m, a); } bool WriteMinidump(const char* filename, pid_t crashing_process, const void* blob, size_t blob_size, - const MappingList& mappings) { + const MappingList& mappings, + const AppMemoryList& appmem) { if (blob_size != sizeof(ExceptionHandler::CrashContext)) return false; const ExceptionHandler::CrashContext* context = @@ -1355,7 +1389,7 @@ bool WriteMinidump(const char* filename, pid_t crashing_process, reinterpret_cast<uintptr_t>(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(filename, context, mappings, appmem, &dumper); if (!writer.Init()) return false; return writer.Dump(); @@ -1363,8 +1397,9 @@ bool WriteMinidump(const char* filename, pid_t crashing_process, bool WriteMinidump(const char* filename, const MappingList& mappings, + const AppMemoryList& appmem, LinuxDumper* dumper) { - MinidumpWriter writer(filename, NULL, mappings, dumper); + MinidumpWriter writer(filename, NULL, mappings, appmem, 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..4ff3b469 100644 --- a/src/client/linux/minidump_writer/minidump_writer.h +++ b/src/client/linux/minidump_writer/minidump_writer.h @@ -51,6 +51,16 @@ struct MappingEntry { // A list of <MappingInfo, GUID> typedef std::list<MappingEntry> MappingList; +// These entries store a list of memory regions that the client wants included +// in the minidump. +struct AppMemory { + AppMemory(void *ptr, size_t length) : ptr(ptr), length(length) {} + + void *ptr; + size_t length; +}; +typedef std::list<AppMemory> AppMemoryList; + // Write a minidump to the filesystem. This function does not malloc nor use // libc functions which may. Thus, it can be used in contexts where the state // of the heap may be corrupt. @@ -64,13 +74,16 @@ typedef std::list<MappingEntry> MappingList; bool WriteMinidump(const char* filename, pid_t crashing_process, const void* blob, size_t blob_size); -// This overload also allows passing a list of known mappings. +// This overload also allows passing a list of known mappings and +// a list of additional memory regions to be included in the minidump. bool WriteMinidump(const char* filename, pid_t crashing_process, const void* blob, size_t blob_size, - const MappingList& mappings); + const MappingList& mappings, + const AppMemoryList& appdata); bool WriteMinidump(const char* filename, const MappingList& mappings, + const AppMemoryList& appdata, LinuxDumper* dumper); } // namespace google_breakpad diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index 4dbbbe86..be6be139 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -32,6 +32,7 @@ #include <sys/stat.h> #include <sys/syscall.h> #include <sys/types.h> +#include <ucontext.h> #include <unistd.h> #include <string> @@ -152,12 +153,13 @@ TEST(MinidumpWriterTest, MappingInfo) { strcpy(info.name, kMemoryName); MappingList mappings; + AppMemoryList memory_list; MappingEntry mapping; mapping.first = info; memcpy(mapping.second, kModuleGUID, sizeof(MDGUID)); mappings.push_back(mapping); ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context), - mappings)); + mappings, memory_list)); // Read the minidump. Load the module list, and ensure that // the mmap'ed |memory| is listed with the given module name @@ -256,13 +258,14 @@ TEST(MinidumpWriterTest, MappingInfoContained) { strcpy(info.name, kMemoryName); MappingList mappings; + AppMemoryList memory_list; MappingEntry mapping; mapping.first = info; memcpy(mapping.second, kModuleGUID, sizeof(MDGUID)); mappings.push_back(mapping); ASSERT_TRUE( WriteMinidump(dumpfile.c_str(), child, &context, sizeof(context), - mappings)); + mappings, memory_list)); // Read the minidump. Load the module list, and ensure that // the mmap'ed |memory| is listed with the given module name @@ -383,3 +386,69 @@ TEST(MinidumpWriterTest, DeletedBinary) { module_identifier += "0"; EXPECT_EQ(module_identifier, module->debug_identifier()); } + +// Test that an additional memory region can be added to the minidump. +TEST(MinidumpWriterTest, AdditionalMemory) { + int fds[2]; + ASSERT_NE(-1, pipe(fds)); + + // These are defined here so the parent can use them to check the + // data from the minidump afterwards. + const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE); + // Get some heap memory. + u_int8_t* memory = new u_int8_t[kMemorySize]; + const uintptr_t kMemoryAddress = reinterpret_cast<uintptr_t>(memory); + ASSERT_TRUE(memory); + // Stick some data into the memory so the contents can be verified. + for (int i = 0; i < kMemorySize; ++i) { + memory[i] = i % 255; + } + + 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; + // This needs a valid context for minidump writing to work, but getting + // a useful one from the child is too much work, so just use one from + // the parent since the child is just a forked copy anyway. + //TODO(ted): this won't work for Android if unit tests ever get run there. + ASSERT_EQ(0, getcontext(&context.context)); + context.tid = child; + + AutoTempDir temp_dir; + string templ = "/tmp/minidump-memory.dmp"; //temp_dir.path() + "/minidump-writer-unittest"; + unlink(templ.c_str()); + + MappingList mappings; + AppMemoryList memory_list; + // Add the memory region to the list of memory to be included. + memory_list.push_back(AppMemory(memory, kMemorySize)); + ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context), + mappings, memory_list)); + + // Read the minidump. Ensure that the memory region is present + Minidump minidump(templ.c_str()); + ASSERT_TRUE(minidump.Read()); + + MinidumpMemoryList* dump_memory_list = minidump.GetMemoryList(); + ASSERT_TRUE(dump_memory_list); + const MinidumpMemoryRegion* region = + dump_memory_list->GetMemoryRegionForAddress(kMemoryAddress); + ASSERT_TRUE(region); + + EXPECT_EQ(kMemoryAddress, region->GetBase()); + EXPECT_EQ(kMemorySize, region->GetSize()); + + // Verify memory contents. + EXPECT_EQ(0, memcmp(region->GetMemory(), memory, kMemorySize)); + + delete[] memory; + close(fds[1]); +} diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc index 6e5b724a..9ede97f0 100644..100755 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -46,9 +46,7 @@ static const int kExceptionHandlerThreadInitialStackSize = 64 * 1024; // This is passed as the context to the MinidumpWriteDump callback. typedef struct { - ULONG64 memory_base; - ULONG memory_size; - bool finished; + AppMemoryList::const_iterator iter, end; } MinidumpCallbackContext; vector<ExceptionHandler*>* ExceptionHandler::handler_stack_ = NULL; @@ -220,6 +218,9 @@ void ExceptionHandler::Initialize(const wstring& dump_path, set_dump_path(dump_path); } + // Reserve one element for the instruction memory + app_memory_info_.push_back(AppMemory(0, 0)); + // There is a race condition here. If the first instance has not yet // initialized the critical section, the second (and later) instances may // try to use uninitialized critical section object. The feature of multiple @@ -795,9 +796,6 @@ bool ExceptionHandler::WriteMinidumpWithException( ++user_streams.UserStreamCount; } - MINIDUMP_CALLBACK_INFORMATION callback; - MinidumpCallbackContext context; - MINIDUMP_CALLBACK_INFORMATION* callback_pointer = NULL; // Older versions of DbgHelp.dll don't correctly put the memory around // the faulting instruction pointer into the minidump. This // callback will ensure that it gets included. @@ -822,23 +820,33 @@ bool ExceptionHandler::WriteMinidumpWithException( // pointer, but settle for whatever's available up to the // boundaries of the memory region. const ULONG64 kIPMemorySize = 256; - context.memory_base = + ULONG64 base = (std::max)(reinterpret_cast<ULONG64>(info.BaseAddress), instruction_pointer - (kIPMemorySize / 2)); ULONG64 end_of_range = (std::min)(instruction_pointer + (kIPMemorySize / 2), reinterpret_cast<ULONG64>(info.BaseAddress) + info.RegionSize); - context.memory_size = - static_cast<ULONG>(end_of_range - context.memory_base); + ULONG size = static_cast<ULONG>(end_of_range - base); - context.finished = false; - callback.CallbackRoutine = MinidumpWriteDumpCallback; - callback.CallbackParam = reinterpret_cast<void*>(&context); - callback_pointer = &callback; + AppMemory &elt = app_memory_info_.front(); + elt.ptr = base; + elt.length = size; } } + MinidumpCallbackContext context; + context.iter = app_memory_info_.begin(); + context.end = app_memory_info_.end(); + + // Skip the reserved element if there was no instruction memory + if (context.iter->ptr == 0) + context.iter++; + + MINIDUMP_CALLBACK_INFORMATION callback; + callback.CallbackRoutine = MinidumpWriteDumpCallback; + callback.CallbackParam = reinterpret_cast<void*>(&context); + // The explicit comparison to TRUE avoids a warning (C4800). success = (minidump_write_dump_(GetCurrentProcess(), GetCurrentProcessId(), @@ -846,7 +854,7 @@ bool ExceptionHandler::WriteMinidumpWithException( dump_type_, exinfo ? &except_info : NULL, &user_streams, - callback_pointer) == TRUE); + &callback) == TRUE); CloseHandle(dump_file); } @@ -874,13 +882,13 @@ BOOL CALLBACK ExceptionHandler::MinidumpWriteDumpCallback( case MemoryCallback: { MinidumpCallbackContext* callback_context = reinterpret_cast<MinidumpCallbackContext*>(context); - if (callback_context->finished) + if (callback_context->iter == callback_context->end) return FALSE; // Include the specified memory region. - callback_output->MemoryBase = callback_context->memory_base; - callback_output->MemorySize = callback_context->memory_size; - callback_context->finished = true; + callback_output->MemoryBase = callback_context->iter->ptr; + callback_output->MemorySize = callback_context->iter->length; + callback_context->iter++; return TRUE; } @@ -924,4 +932,20 @@ void ExceptionHandler::UpdateNextID() { next_minidump_path_c_ = next_minidump_path_.c_str(); } +void ExceptionHandler::RegisterAppMemory(void *ptr, size_t length) { + app_memory_info_.push_back(AppMemory(reinterpret_cast<ULONG64>(ptr), + static_cast<ULONG>(length))); +} + +void ExceptionHandler::UnregisterAppMemory(void *ptr) { + for (AppMemoryList::iterator iter = app_memory_info_.begin(); + iter != app_memory_info_.end(); + ++iter) { + if (iter->ptr == reinterpret_cast<ULONG64>(ptr)) { + app_memory_info_.erase(iter); + return; + } + } +} + } // namespace google_breakpad diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h index 09f5177c..38c2c3ca 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -67,6 +67,7 @@ #include <string> #include <vector> +#include <list> #include "client/windows/common/ipc_protocol.h" #include "client/windows/crash_generation/crash_generation_client.h" @@ -78,6 +79,16 @@ namespace google_breakpad { using std::vector; using std::wstring; +// These entries store a list of memory regions that the client wants included +// in the minidump. +struct AppMemory { + AppMemory(ULONG64 ptr, ULONG length) : ptr(ptr), length(length) {} + + ULONG64 ptr; + ULONG length; +}; +typedef std::list<AppMemory> AppMemoryList; + class ExceptionHandler { public: // A callback function to run before Breakpad performs any substantial @@ -219,6 +230,11 @@ class ExceptionHandler { // Returns whether out-of-process dump generation is used or not. bool IsOutOfProcess() const { return crash_generation_client_.get() != NULL; } + // Calling RegisterAppMemory(p, len) causes len bytes starting + // at address p to be copied to the minidump when a crash happens. + void RegisterAppMemory(void *ptr, size_t length); + void UnregisterAppMemory(void *ptr); + private: friend class AutoExceptionHandler; @@ -404,6 +420,10 @@ class ExceptionHandler { // to not interfere with debuggers. bool handle_debug_exceptions_; + // Callers can request additional memory regions to be included in + // the dump. + AppMemoryList app_memory_info_; + // A stack of ExceptionHandler objects that have installed unhandled // exception filters. This vector is used by HandleException to determine // which ExceptionHandler object to route an exception to. When an diff --git a/src/client/windows/unittests/exception_handler_test.cc b/src/client/windows/unittests/exception_handler_test.cc index 74d9a9be..08960015 100644..100755 --- a/src/client/windows/unittests/exception_handler_test.cc +++ b/src/client/windows/unittests/exception_handler_test.cc @@ -374,4 +374,50 @@ TEST_F(ExceptionHandlerTest, WriteMinidumpTest) { //TODO(ted): more comprehensive tests... } +TEST_F(ExceptionHandlerTest, AdditionalMemory) { + SYSTEM_INFO si; + GetSystemInfo(&si); + const u_int32_t kMemorySize = si.dwPageSize; + // Get some heap memory. + u_int8_t* memory = new u_int8_t[kMemorySize]; + const uintptr_t kMemoryAddress = reinterpret_cast<uintptr_t>(memory); + ASSERT_TRUE(memory); + // Stick some data into the memory so the contents can be verified. + for (unsigned int i = 0; i < kMemorySize; ++i) { + memory[i] = i % 255; + } + + ExceptionHandler handler(temp_path_, + NULL, + DumpCallback, + NULL, + ExceptionHandler::HANDLER_ALL); + // Add the memory region to the list of memory to be included. + handler.RegisterAppMemory(memory, kMemorySize); + ASSERT_TRUE(handler.WriteMinidump()); + ASSERT_FALSE(dump_file.empty()); + + string minidump_filename; + ASSERT_TRUE(WindowsStringUtils::safe_wcstombs(dump_file, + &minidump_filename)); + + // Read the minidump. Ensure that the memory region is present + Minidump minidump(minidump_filename); + ASSERT_TRUE(minidump.Read()); + + MinidumpMemoryList* dump_memory_list = minidump.GetMemoryList(); + ASSERT_TRUE(dump_memory_list); + const MinidumpMemoryRegion* region = + dump_memory_list->GetMemoryRegionForAddress(kMemoryAddress); + ASSERT_TRUE(region); + + EXPECT_EQ(kMemoryAddress, region->GetBase()); + EXPECT_EQ(kMemorySize, region->GetSize()); + + // Verify memory contents. + EXPECT_EQ(0, memcmp(region->GetMemory(), memory, kMemorySize)); + + delete[] memory; +} + } // namespace |