diff --git a/src/client/mac/handler/minidump_generator.cc b/src/client/mac/handler/minidump_generator.cc index 73086743..8e3faf27 100644 --- a/src/client/mac/handler/minidump_generator.cc +++ b/src/client/mac/handler/minidump_generator.cc @@ -641,7 +641,7 @@ bool MinidumpGenerator::WriteMemoryListStream( unsigned int i; for (i = 0; i < memory_count; ++i) { - list.CopyIndexAfterObject(i++, &memory_blocks_[i], + list.CopyIndexAfterObject(i, &memory_blocks_[i], sizeof(MDMemoryDescriptor)); } diff --git a/src/client/mac/tests/exception_handler_test.cc b/src/client/mac/tests/exception_handler_test.cc index 4bdd1856..004bc8e7 100644 --- a/src/client/mac/tests/exception_handler_test.cc +++ b/src/client/mac/tests/exception_handler_test.cc @@ -29,6 +29,7 @@ // exception_handler_test.cc: Unit tests for google_breakpad::ExceptionHandler +#include #include #include #include @@ -241,7 +242,7 @@ TEST_F(ExceptionHandlerTest, InstructionPointerMemory) { // 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 = @@ -594,4 +595,63 @@ TEST_F(ExceptionHandlerTest, InstructionPointerMemoryNullPointer) { ASSERT_EQ((unsigned int)1, memory_list->region_count()); } +static void *Junk(void *) { + sleep(1000000); + return NULL; +} + +// Test that the memory list gets written correctly when multiple +// threads are running. +TEST_F(ExceptionHandlerTest, MemoryListMultipleThreads) { + // Give the child process a pipe to report back on. + int fds[2]; + ASSERT_EQ(0, pipe(fds)); + + pid_t pid = fork(); + if (pid == 0) { + close(fds[0]); + ExceptionHandler eh(tempDir.path, NULL, MDCallback, &fds[1], true, NULL); + + // Run an extra thread so >2 memory regions will be written. + pthread_t junk_thread; + if (pthread_create(&junk_thread, NULL, Junk, NULL) == 0) + pthread_detach(junk_thread); + + // Just crash. + Crasher(); + + // not reached + exit(1); + } + // In the parent process. + ASSERT_NE(-1, pid); + close(fds[1]); + + // Wait for the background process to return the minidump file. + close(fds[1]); + char minidump_file[PATH_MAX]; + ssize_t nbytes = read(fds[0], minidump_file, sizeof(minidump_file)); + ASSERT_NE(0, nbytes); + // Ensure that minidump file exists and is > 0 bytes. + struct stat st; + ASSERT_EQ(0, stat(minidump_file, &st)); + ASSERT_LT(0, st.st_size); + + // Child process should have exited with a zero status. + int ret; + ASSERT_EQ(pid, waitpid(pid, &ret, 0)); + EXPECT_NE(0, WIFEXITED(ret)); + EXPECT_EQ(0, WEXITSTATUS(ret)); + + // Read the minidump, and verify that the memory list can be read. + Minidump minidump(minidump_file); + ASSERT_TRUE(minidump.Read()); + + MinidumpMemoryList* memory_list = minidump.GetMemoryList(); + ASSERT_TRUE(memory_list); + // Verify that there are three memory regions: + // one per thread, and one for the instruction pointer memory. + ASSERT_EQ((unsigned int)3, memory_list->region_count()); +} + }