diff options
| -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 | ||||
| -rw-r--r--[-rwxr-xr-x] | src/client/windows/handler/exception_handler.cc | 60 | ||||
| -rw-r--r-- | src/client/windows/handler/exception_handler.h | 20 | ||||
| -rw-r--r--[-rwxr-xr-x] | src/client/windows/unittests/exception_handler_test.cc | 46 | ||||
| -rw-r--r-- | src/tools/linux/core2md/core2md.cc | 5 |
10 files changed, 29 insertions, 305 deletions
diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index fffc3e64..f505ff33 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -465,8 +465,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, crashing_process, context, context_size, - mapping_list_, - app_memory_list_); + mapping_list_); } // static @@ -516,19 +515,4 @@ 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 fab74d28..4bc8ba72 100644 --- a/src/client/linux/handler/exception_handler.h +++ b/src/client/linux/handler/exception_handler.h @@ -194,13 +194,6 @@ 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); @@ -259,10 +252,6 @@ 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 3ce93adc..ab3e3072 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -788,42 +788,3 @@ 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 9c834791..e0aeb3bd 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -373,7 +373,6 @@ 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,8 +384,7 @@ class MinidumpWriter { #endif dumper_(dumper), memory_blocks_(dumper_->allocator()), - mapping_list_(mappings), - app_memory_list_(appmem) { + mapping_list_(mappings) { } bool Init() { @@ -460,9 +458,6 @@ class MinidumpWriter { return false; dir.CopyIndex(dir_index++, &dirent); - if (!WriteAppMemory()) - return false; - if (!WriteMemoryListStream(&dirent)) return false; dir.CopyIndex(dir_index++, &dirent); @@ -764,30 +759,6 @@ 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. @@ -1363,22 +1334,17 @@ 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; - AppMemoryList a; - return WriteMinidump(filename, crashing_process, blob, blob_size, m, a); + 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, - const AppMemoryList& appmem) { + const MappingList& mappings) { if (blob_size != sizeof(ExceptionHandler::CrashContext)) return false; const ExceptionHandler::CrashContext* context = @@ -1388,7 +1354,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, appmem, &dumper); + MinidumpWriter writer(filename, context, mappings, &dumper); if (!writer.Init()) return false; return writer.Dump(); @@ -1396,9 +1362,8 @@ 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, appmem, dumper); + MinidumpWriter writer(filename, 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 4ff3b469..e79eb79b 100644 --- a/src/client/linux/minidump_writer/minidump_writer.h +++ b/src/client/linux/minidump_writer/minidump_writer.h @@ -51,16 +51,6 @@ 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. @@ -74,16 +64,13 @@ typedef std::list<AppMemory> AppMemoryList; 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 and -// a list of additional memory regions to be included in the minidump. +// This overload also allows passing a list of known mappings. bool WriteMinidump(const char* filename, pid_t crashing_process, const void* blob, size_t blob_size, - const MappingList& mappings, - const AppMemoryList& appdata); + const MappingList& mappings); 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 4f69013a..31e1440d 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -32,7 +32,6 @@ #include <sys/stat.h> #include <sys/syscall.h> #include <sys/types.h> -#include <ucontext.h> #include <unistd.h> #include <string> @@ -154,13 +153,12 @@ 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, memory_list)); + mappings)); // Read the minidump. Load the module list, and ensure that // the mmap'ed |memory| is listed with the given module name @@ -259,14 +257,13 @@ 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, memory_list)); + mappings)); // Read the minidump. Load the module list, and ensure that // the mmap'ed |memory| is listed with the given module name @@ -387,69 +384,3 @@ 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 9ede97f0..6e5b724a 100755..100644 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -46,7 +46,9 @@ static const int kExceptionHandlerThreadInitialStackSize = 64 * 1024; // This is passed as the context to the MinidumpWriteDump callback. typedef struct { - AppMemoryList::const_iterator iter, end; + ULONG64 memory_base; + ULONG memory_size; + bool finished; } MinidumpCallbackContext; vector<ExceptionHandler*>* ExceptionHandler::handler_stack_ = NULL; @@ -218,9 +220,6 @@ 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 @@ -796,6 +795,9 @@ 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. @@ -820,33 +822,23 @@ bool ExceptionHandler::WriteMinidumpWithException( // pointer, but settle for whatever's available up to the // boundaries of the memory region. const ULONG64 kIPMemorySize = 256; - ULONG64 base = + context.memory_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); - ULONG size = static_cast<ULONG>(end_of_range - base); + context.memory_size = + static_cast<ULONG>(end_of_range - context.memory_base); - AppMemory &elt = app_memory_info_.front(); - elt.ptr = base; - elt.length = size; + context.finished = false; + callback.CallbackRoutine = MinidumpWriteDumpCallback; + callback.CallbackParam = reinterpret_cast<void*>(&context); + callback_pointer = &callback; } } - 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(), @@ -854,7 +846,7 @@ bool ExceptionHandler::WriteMinidumpWithException( dump_type_, exinfo ? &except_info : NULL, &user_streams, - &callback) == TRUE); + callback_pointer) == TRUE); CloseHandle(dump_file); } @@ -882,13 +874,13 @@ BOOL CALLBACK ExceptionHandler::MinidumpWriteDumpCallback( case MemoryCallback: { MinidumpCallbackContext* callback_context = reinterpret_cast<MinidumpCallbackContext*>(context); - if (callback_context->iter == callback_context->end) + if (callback_context->finished) return FALSE; // Include the specified memory region. - callback_output->MemoryBase = callback_context->iter->ptr; - callback_output->MemorySize = callback_context->iter->length; - callback_context->iter++; + callback_output->MemoryBase = callback_context->memory_base; + callback_output->MemorySize = callback_context->memory_size; + callback_context->finished = true; return TRUE; } @@ -932,20 +924,4 @@ 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 38c2c3ca..09f5177c 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -67,7 +67,6 @@ #include <string> #include <vector> -#include <list> #include "client/windows/common/ipc_protocol.h" #include "client/windows/crash_generation/crash_generation_client.h" @@ -79,16 +78,6 @@ 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 @@ -230,11 +219,6 @@ 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; @@ -420,10 +404,6 @@ 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 08960015..74d9a9be 100755..100644 --- a/src/client/windows/unittests/exception_handler_test.cc +++ b/src/client/windows/unittests/exception_handler_test.cc @@ -374,50 +374,4 @@ 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 diff --git a/src/tools/linux/core2md/core2md.cc b/src/tools/linux/core2md/core2md.cc index a0e8f8ba..641496ee 100644 --- a/src/tools/linux/core2md/core2md.cc +++ b/src/tools/linux/core2md/core2md.cc @@ -34,7 +34,6 @@ #include "client/linux/minidump_writer/minidump_writer.h" #include "client/linux/minidump_writer/linux_core_dumper.h" -using google_breakpad::AppMemoryList; using google_breakpad::MappingList; using google_breakpad::LinuxCoreDumper; @@ -47,10 +46,8 @@ bool WriteMinidumpFromCore(const char* filename, const char* core_path, const char* procfs_override) { MappingList mappings; - AppMemoryList memory_list; LinuxCoreDumper dumper(0, core_path, procfs_override); - return google_breakpad::WriteMinidump(filename, mappings, memory_list, - &dumper); + return google_breakpad::WriteMinidump(filename, mappings, &dumper); } int main(int argc, char *argv[]) { |