diff --git a/Makefile.am b/Makefile.am index 30e6b2f6..084d9813 100644 --- a/Makefile.am +++ b/Makefile.am @@ -220,8 +220,13 @@ src_client_linux_linux_client_unittest_LDADD = \ src/common/md5.lo \ src/common/linux/file_id.lo \ src/common/linux/guid_creator.lo \ - src/common/string_conversion.lo -src_client_linux_linux_client_unittest_DEPENDENCIES = src/client/linux/linux_dumper_unittest_helper src/client/linux/libbreakpad_client.la + src/common/string_conversion.lo \ + src/processor/basic_code_modules.lo \ + src/processor/logging.lo \ + src/processor/minidump.lo \ + src/processor/pathname_stripper.lo + +src_client_linux_linux_client_unittest_DEPENDENCIES = src/client/linux/linux_dumper_unittest_helper src/client/linux/libbreakpad_client.la src/libbreakpad.la endif src_processor_address_map_unittest_SOURCES = \ diff --git a/Makefile.in b/Makefile.in index 67cdd27f..d6506d4b 100644 --- a/Makefile.in +++ b/Makefile.in @@ -801,9 +801,13 @@ TESTS_ENVIRONMENT = @LINUX_HOST_TRUE@ src/common/md5.lo \ @LINUX_HOST_TRUE@ src/common/linux/file_id.lo \ @LINUX_HOST_TRUE@ src/common/linux/guid_creator.lo \ -@LINUX_HOST_TRUE@ src/common/string_conversion.lo +@LINUX_HOST_TRUE@ src/common/string_conversion.lo \ +@LINUX_HOST_TRUE@ src/processor/basic_code_modules.lo \ +@LINUX_HOST_TRUE@ src/processor/logging.lo \ +@LINUX_HOST_TRUE@ src/processor/minidump.lo \ +@LINUX_HOST_TRUE@ src/processor/pathname_stripper.lo -@LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_DEPENDENCIES = src/client/linux/linux_dumper_unittest_helper src/client/linux/libbreakpad_client.la +@LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_DEPENDENCIES = src/client/linux/linux_dumper_unittest_helper src/client/linux/libbreakpad_client.la src/libbreakpad.la src_processor_address_map_unittest_SOURCES = \ src/processor/address_map_unittest.cc diff --git a/configure b/configure index 1e95fe90..bfa46fcc 100755 --- a/configure +++ b/configure @@ -14955,8 +14955,8 @@ fi if test "${enable_m32+set}" = set; then : enableval=$enable_m32; case "${enableval}" in yes) - CFLAGS="${CFLAGS} -m32" - CXXFLAGS="${CXXFLAGS} -m32" + CFLAGS="${CFLAGS} -m32" + CXXFLAGS="${CXXFLAGS} -m32" usem32=true ;; no) diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index 731207a8..3fea1159 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,7 @@ #include "common/linux/eintr_wrapper.h" #include "common/linux/linux_libc_support.h" #include "third_party/lss/linux_syscall_support.h" +#include "google_breakpad/processor/minidump.h" using namespace google_breakpad; @@ -126,6 +128,127 @@ TEST(ExceptionHandlerTest, ChildCrash) { ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); ASSERT_GT(st.st_size, 0u); unlink(minidump_filename.c_str()); +} + +TEST(ExceptionHandlerTest, InstructionPointerMemory) { + int fds[2]; + ASSERT_NE(pipe(fds), -1); + + // These are defined here so the parent can use them to check the + // data from the minidump afterwards. + const u_int32_t kMemorySize = 256; // bytes + const int kOffset = kMemorySize / 2; + // This crashes with SIGILL on x86/x86-64/arm. + const unsigned char instructions[] = { 0xff, 0xff, 0xff, 0xff }; + + const pid_t child = fork(); + if (child == 0) { + close(fds[0]); + ExceptionHandler handler("/tmp", NULL, DoneCallback, (void*) fds[1], + true); + // Get some executable memory. + char* memory = + reinterpret_cast(mmap(NULL, + kMemorySize, + PROT_READ | PROT_WRITE | PROT_EXEC, + MAP_PRIVATE | MAP_ANON, + -1, + 0)); + if (!memory) + exit(0); + + // Write some instructions that will crash. Put them in the middle + // of the block of memory, because the minidump should contain 128 + // bytes on either side of the instruction pointer. + memcpy(memory + kOffset, instructions, sizeof(instructions)); + + // Now execute the instructions, which should crash. + typedef void (*void_function)(void); + void_function memory_function = + 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); + + 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 std::string minidump_filename = std::string("/tmp/") + filename + + ".dmp"; + + struct stat st; + ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0); + 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); + ASSERT_TRUE(minidump.Read()); + + MinidumpException* exception = minidump.GetException(); + MinidumpMemoryList* memory_list = minidump.GetMemoryList(); + ASSERT_TRUE(exception); + ASSERT_TRUE(memory_list); + ASSERT_LT(0, memory_list->region_count()); + + MinidumpContext* context = exception->GetContext(); + ASSERT_TRUE(context); + + u_int64_t instruction_pointer; + switch (context->GetContextCPU()) { + case MD_CONTEXT_X86: + instruction_pointer = context->GetContextX86()->eip; + break; + case MD_CONTEXT_AMD64: + instruction_pointer = context->GetContextAMD64()->rip; + break; + case MD_CONTEXT_ARM: + instruction_pointer = context->GetContextARM()->iregs[15]; + break; + default: + FAIL() << "Unknown context CPU: " << context->GetContextCPU(); + break; + } + + MinidumpMemoryRegion* region = + memory_list->GetMemoryRegionForAddress(instruction_pointer); + ASSERT_TRUE(region); + + EXPECT_EQ(kMemorySize, region->GetSize()); + const u_int8_t* bytes = region->GetMemory(); + ASSERT_TRUE(bytes); + + u_int8_t prefix_bytes[kOffset]; + u_int8_t suffix_bytes[kMemorySize - kOffset - sizeof(instructions)]; + memset(prefix_bytes, 0, sizeof(prefix_bytes)); + memset(suffix_bytes, 0, sizeof(suffix_bytes)); + EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0); + 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); } diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 260ce934..6b55ade0 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -46,6 +46,8 @@ #include "client/linux/minidump_writer/minidump_writer.h" #include "client/minidump_file_writer-inl.h" +#include + #include #include #include @@ -367,7 +369,8 @@ class MinidumpWriter { float_state_(NULL), #endif crashing_tid_(context->tid), - dumper_(crashing_pid) { + dumper_(crashing_pid), + memory_blocks_(dumper_.allocator()) { } bool Init() { @@ -400,7 +403,9 @@ class MinidumpWriter { // A minidump file contains a number of tagged streams. This is the number // of stream which we write. - const unsigned kNumWriters = 11 + !!r_debug; + unsigned kNumWriters = 12; + if (r_debug) + ++kNumWriters; TypedMDRVA header(&minidump_writer_); TypedMDRVA dir(&minidump_writer_); @@ -427,6 +432,10 @@ class MinidumpWriter { return false; dir.CopyIndex(dir_index++, &dirent); + if (!WriteMemoryListStream(&dirent)) + return false; + dir.CopyIndex(dir_index++, &dirent); + if (!WriteExceptionStream(&dirent)) return false; dir.CopyIndex(dir_index++, &dirent); @@ -635,6 +644,50 @@ class MinidumpWriter { memory.Copy(stack_copy, stack_len); thread.stack.start_of_memory_range = (uintptr_t) (stack); thread.stack.memory = memory.location(); + memory_blocks_.push_back(thread.stack); + + // Copy 256 bytes around crashing instruction pointer to minidump. + const size_t kIPMemorySize = 256; + u_int64_t ip = GetInstructionPointer(); + // Bound it to the upper and lower bounds of the memory map + // it's contained within. If it's not in mapped memory, + // don't bother trying to write it. + bool ip_is_mapped = false; + MDMemoryDescriptor ip_memory_d; + for (unsigned i = 0; i < dumper_.mappings().size(); ++i) { + const MappingInfo& mapping = *dumper_.mappings()[i]; + if (ip >= mapping.start_addr && + ip < mapping.start_addr + mapping.size) { + ip_is_mapped = true; + // Try to get 128 bytes before and after the IP, but + // settle for whatever's available. + ip_memory_d.start_of_memory_range = + std::min(mapping.start_addr, + uintptr_t(ip - (kIPMemorySize / 2))); + ip_memory_d.memory.data_size = + std::min(ptrdiff_t(kIPMemorySize), + ptrdiff_t(mapping.start_addr + mapping.size + - ip_memory_d.start_of_memory_range)); + break; + } + } + + if (ip_is_mapped) { + UntypedMDRVA ip_memory(&minidump_writer_); + if (!ip_memory.Allocate(ip_memory_d.memory.data_size)) + return false; + uint8_t* memory_copy = + (uint8_t*) dumper_.allocator()->Alloc(ip_memory_d.memory.data_size); + dumper_.CopyFromProcess( + memory_copy, + thread.thread_id, + reinterpret_cast(ip_memory_d.start_of_memory_range), + ip_memory_d.memory.data_size); + ip_memory.Copy(memory_copy, ip_memory_d.memory.data_size); + ip_memory_d.memory = ip_memory.location(); + memory_blocks_.push_back(ip_memory_d); + } + TypedMDRVA cpu(&minidump_writer_); if (!cpu.Allocate()) return false; @@ -657,6 +710,8 @@ class MinidumpWriter { memory.Copy(stack_copy, info.stack_len); thread.stack.start_of_memory_range = (uintptr_t)(info.stack); thread.stack.memory = memory.location(); + memory_blocks_.push_back(thread.stack); + TypedMDRVA cpu(&minidump_writer_); if (!cpu.Allocate()) return false; @@ -757,6 +812,24 @@ class MinidumpWriter { return true; } + bool WriteMemoryListStream(MDRawDirectory* dirent) { + TypedMDRVA list(&minidump_writer_); + if (!list.AllocateObjectAndArray(memory_blocks_.size(), + sizeof(MDMemoryDescriptor))) + return false; + + dirent->stream_type = MD_MEMORY_LIST_STREAM; + dirent->location = list.location(); + + *list.get() = memory_blocks_.size(); + + for (size_t i = 0; i < memory_blocks_.size(); ++i) { + list.CopyIndexAfterObject(i, &memory_blocks_[i], + sizeof(MDMemoryDescriptor)); + } + return true; + } + bool WriteExceptionStream(MDRawDirectory* dirent) { TypedMDRVA exc(&minidump_writer_); if (!exc.Allocate()) @@ -872,14 +945,26 @@ class MinidumpWriter { uintptr_t GetStackPointer() { return ucontext_->uc_mcontext.gregs[REG_ESP]; } + + uintptr_t GetInstructionPointer() { + return ucontext_->uc_mcontext.gregs[REG_EIP]; + } #elif defined(__x86_64) uintptr_t GetStackPointer() { return ucontext_->uc_mcontext.gregs[REG_RSP]; } + + uintptr_t GetInstructionPointer() { + return ucontext_->uc_mcontext.gregs[REG_RIP]; + } #elif defined(__ARM_EABI__) uintptr_t GetStackPointer() { return ucontext_->uc_mcontext.arm_sp; } + + uintptr_t GetInstructionPointer() { + return ucontext_->uc_mcontext.arm_ip; + } #else #error "This code has not been ported to your platform yet." #endif @@ -1129,6 +1214,10 @@ popline: LinuxDumper dumper_; MinidumpFileWriter minidump_writer_; MDLocationDescriptor crashing_thread_context_; + // Blocks of memory written to the dump. These are all currently + // written while writing the thread list stream, but saved here + // so a memory list stream can be written afterwards. + wasteful_vector memory_blocks_; }; bool WriteMinidump(const char* filename, pid_t crashing_process,