Fix WriteMemoryListStream to remove an extraneous loop index variable increment.

Patch by timeless <timeless@mozdev.org>, R=me, unittest by me.
See https://bugzilla.mozilla.org/show_bug.cgi?id=615534

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@735 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 62 insertions, 2 deletions

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 <pthread.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -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());
+}
+
 }