Merge r1065: Don't bail if a thread's stack pointer is invalid

Currently, if a thread's stack pointer is not within a valid memory page,
the minidump writing will fail with an error.  This change allows an invalid
stack pointer by simply setting the memory size to zero in the minidump.
The processing code already checks for the size being zero, although it
currently just gives an error (see https://breakpad.appspot.com/413002/).

BUG=google-breakpad:499, chromium-os:34880
TEST=make check, manually ran minidump-2-core and core2md
Original Review URL: https://breakpad.appspot.com/478002

* To make this build, there is an additional #include <ucontext.h>.
Review URL: https://breakpad.appspot.com/485002

Cr-Mirrored-From: https://chromium.googlesource.com/breakpad/breakpad
Cr-Mirrored-Commit: dea6ce6eb165ae7911ad15ce733c76fd08d0dad0
diff --git a/client/linux/minidump_writer/linux_core_dumper.cc b/client/linux/minidump_writer/linux_core_dumper.cc
index 3e8c92f..47cc26c 100644
--- a/client/linux/minidump_writer/linux_core_dumper.cc
+++ b/client/linux/minidump_writer/linux_core_dumper.cc
@@ -102,9 +102,8 @@
 #else
 #error "This code hasn't been ported to your platform yet."
 #endif
-
-  return GetStackInfo(&info->stack, &info->stack_len,
-                      reinterpret_cast<uintptr_t>(stack_pointer));
+  info->stack_pointer = reinterpret_cast<uintptr_t>(stack_pointer);
+  return true;
 }
 
 bool LinuxCoreDumper::IsPostMortem() const {
diff --git a/client/linux/minidump_writer/linux_core_dumper_unittest.cc b/client/linux/minidump_writer/linux_core_dumper_unittest.cc
index d04ef3e..50b3193 100644
--- a/client/linux/minidump_writer/linux_core_dumper_unittest.cc
+++ b/client/linux/minidump_writer/linux_core_dumper_unittest.cc
@@ -105,6 +105,9 @@
   for (unsigned i = 0; i < kNumOfThreads; ++i) {
     ThreadInfo info;
     EXPECT_TRUE(dumper.GetThreadInfoByIndex(i, &info));
+    const void* stack;
+    size_t stack_len;
+    EXPECT_TRUE(dumper.GetStackInfo(&stack, &stack_len, info.stack_pointer));
     EXPECT_EQ(getpid(), info.ppid);
   }
 }
diff --git a/client/linux/minidump_writer/linux_dumper.h b/client/linux/minidump_writer/linux_dumper.h
index 0fca993..839ce90 100644
--- a/client/linux/minidump_writer/linux_dumper.h
+++ b/client/linux/minidump_writer/linux_dumper.h
@@ -69,10 +69,7 @@
   pid_t tgid;   // thread group id
   pid_t ppid;   // parent process
 
-  // Even on platforms where the stack grows down, the following will point to
-  // the smallest address in the stack.
-  const void* stack;  // pointer to the stack area
-  size_t stack_len;  // length of the stack to copy
+  uintptr_t stack_pointer;  // thread stack pointer
 
 
 #if defined(__i386) || defined(__x86_64)
diff --git a/client/linux/minidump_writer/linux_ptrace_dumper.cc b/client/linux/minidump_writer/linux_ptrace_dumper.cc
index 5591067..2955c6b 100644
--- a/client/linux/minidump_writer/linux_ptrace_dumper.cc
+++ b/client/linux/minidump_writer/linux_ptrace_dumper.cc
@@ -218,9 +218,9 @@
 #else
 #error "This code hasn't been ported to your platform yet."
 #endif
+  info->stack_pointer = reinterpret_cast<uintptr_t>(stack_pointer);
 
-  return GetStackInfo(&info->stack, &info->stack_len,
-                      (uintptr_t) stack_pointer);
+  return true;
 }
 
 bool LinuxPtraceDumper::IsPostMortem() const {
diff --git a/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
index 61f21b8..5ccfbd2 100644
--- a/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
+++ b/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
@@ -230,6 +230,10 @@
   ThreadInfo one_thread;
   for (size_t i = 0; i < dumper.threads().size(); ++i) {
     EXPECT_TRUE(dumper.GetThreadInfoByIndex(i, &one_thread));
+    const void* stack;
+    size_t stack_len;
+    EXPECT_TRUE(dumper.GetStackInfo(&stack, &stack_len,
+        one_thread.stack_pointer));
     // In the helper program, we stored a pointer to the thread id in a
     // specific register. Check that we can recover its value.
 #if defined(__ARM_EABI__)
diff --git a/client/linux/minidump_writer/minidump_writer.cc b/client/linux/minidump_writer/minidump_writer.cc
index 88d59f2..7b41778 100644
--- a/client/linux/minidump_writer/minidump_writer.cc
+++ b/client/linux/minidump_writer/minidump_writer.cc
@@ -655,6 +655,31 @@
 #endif
   }
 
+  bool FillThreadStack(MDRawThread* thread, uintptr_t stack_pointer,
+                       uint8_t** stack_copy) {
+    *stack_copy = NULL;
+    const void* stack;
+    size_t stack_len;
+    if (dumper_->GetStackInfo(&stack, &stack_len, stack_pointer)) {
+      UntypedMDRVA memory(&minidump_writer_);
+      if (!memory.Allocate(stack_len))
+        return false;
+      *stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len));
+      dumper_->CopyFromProcess(*stack_copy, thread->thread_id, stack,
+                               stack_len);
+      memory.Copy(*stack_copy, stack_len);
+      thread->stack.start_of_memory_range =
+          reinterpret_cast<uintptr_t>(stack);
+      thread->stack.memory = memory.location();
+      memory_blocks_.push_back(thread->stack);
+    } else {
+      thread->stack.start_of_memory_range = stack_pointer;
+      thread->stack.memory.data_size = 0;
+      thread->stack.memory.rva = minidump_writer_.position();
+    }
+    return true;
+  }
+
   // Write information about the threads.
   bool WriteThreadListStream(MDRawDirectory* dirent) {
     const unsigned num_threads = dumper_->threads().size();
@@ -672,26 +697,16 @@
       MDRawThread thread;
       my_memset(&thread, 0, sizeof(thread));
       thread.thread_id = dumper_->threads()[i];
+
       // We have a different source of information for the crashing thread. If
       // we used the actual state of the thread we would find it running in the
       // signal handler with the alternative stack, which would be deeply
       // unhelpful.
       if (static_cast<pid_t>(thread.thread_id) == GetCrashThread() &&
           !dumper_->IsPostMortem()) {
-        const void* stack;
-        size_t stack_len;
-        if (!dumper_->GetStackInfo(&stack, &stack_len, GetStackPointer()))
+        uint8_t* stack_copy;
+        if (!FillThreadStack(&thread, GetStackPointer(), &stack_copy))
           return false;
-        UntypedMDRVA memory(&minidump_writer_);
-        if (!memory.Allocate(stack_len))
-          return false;
-        uint8_t* stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len));
-        dumper_->CopyFromProcess(stack_copy, thread.thread_id, stack,
-                                 stack_len);
-        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;
@@ -741,30 +756,26 @@
           return false;
         my_memset(cpu.get(), 0, sizeof(RawContextCPU));
         CPUFillFromUContext(cpu.get(), ucontext_, float_state_);
-        PopSeccompStackFrame(cpu.get(), thread, stack_copy);
+        if (stack_copy)
+          PopSeccompStackFrame(cpu.get(), thread, stack_copy);
         thread.thread_context = cpu.location();
         crashing_thread_context_ = cpu.location();
       } else {
         ThreadInfo info;
         if (!dumper_->GetThreadInfoByIndex(i, &info))
           return false;
-        UntypedMDRVA memory(&minidump_writer_);
-        if (!memory.Allocate(info.stack_len))
+
+        uint8_t* stack_copy;
+        if (!FillThreadStack(&thread, info.stack_pointer, &stack_copy))
           return false;
-        uint8_t* stack_copy = reinterpret_cast<uint8_t*>(Alloc(info.stack_len));
-        dumper_->CopyFromProcess(stack_copy, thread.thread_id, info.stack,
-                                 info.stack_len);
-        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<RawContextCPU> cpu(&minidump_writer_);
         if (!cpu.Allocate())
           return false;
         my_memset(cpu.get(), 0, sizeof(RawContextCPU));
         CPUFillFromThreadInfo(cpu.get(), info);
-        PopSeccompStackFrame(cpu.get(), thread, stack_copy);
+        if (stack_copy)
+          PopSeccompStackFrame(cpu.get(), thread, stack_copy);
         thread.thread_context = cpu.location();
         if (dumper_->threads()[i] == GetCrashThread()) {
           assert(dumper_->IsPostMortem());
@@ -913,9 +924,16 @@
 
   bool WriteMemoryListStream(MDRawDirectory* dirent) {
     TypedMDRVA<uint32_t> list(&minidump_writer_);
-    if (!list.AllocateObjectAndArray(memory_blocks_.size(),
-                                     sizeof(MDMemoryDescriptor)))
-      return false;
+    if (memory_blocks_.size()) {
+      if (!list.AllocateObjectAndArray(memory_blocks_.size(),
+                                       sizeof(MDMemoryDescriptor)))
+        return false;
+    } else {
+      // Still create the memory list stream, although it will have zero
+      // memory blocks.
+      if (!list.Allocate())
+        return false;
+    }
 
     dirent->stream_type = MD_MEMORY_LIST_STREAM;
     dirent->location = list.location();
diff --git a/client/linux/minidump_writer/minidump_writer_unittest.cc b/client/linux/minidump_writer/minidump_writer_unittest.cc
index 1fff015..43e9f30 100644
--- a/client/linux/minidump_writer/minidump_writer_unittest.cc
+++ b/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>
@@ -411,3 +412,83 @@
   module_identifier += "0";
   EXPECT_EQ(module_identifier, module->debug_identifier());
 }
+
+// Test that an invalid thread stack pointer still results in a minidump.
+TEST(MinidumpWriterTest, InvalidStackPointer) {
+  int fds[2];
+  ASSERT_NE(-1, pipe(fds));
+
+  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.
+  ASSERT_EQ(0, getcontext(&context.context));
+  context.tid = child;
+
+  // Fake the child's stack pointer for its crashing thread.  NOTE: This must
+  // be an invalid memory address for the child process (stack or otherwise).
+#if defined(__i386)
+  // Try 1MB below the current stack.
+  uintptr_t invalid_stack_pointer =
+      reinterpret_cast<uintptr_t>(&context) - 1024*1024;
+  context.context.uc_mcontext.gregs[REG_ESP] = invalid_stack_pointer;
+#elif defined(__x86_64)
+  // Try 1MB below the current stack.
+  uintptr_t invalid_stack_pointer =
+      reinterpret_cast<uintptr_t>(&context) - 1024*1024;
+  context.context.uc_mcontext.gregs[REG_RSP] = invalid_stack_pointer;
+#elif defined(__ARM_EABI__)
+  // Try 1MB below the current stack.
+  uintptr_t invalid_stack_pointer =
+      reinterpret_cast<uintptr_t>(&context) - 1024*1024;
+  context.context.uc_mcontext.arm_sp = invalid_stack_pointer;
+#else
+# error "This code has not been ported to your platform yet."
+#endif
+
+  AutoTempDir temp_dir;
+  string templ = temp_dir.path() + "/minidump-writer-unittest";
+  // NOTE: In previous versions of Breakpad, WriteMinidump() would fail if
+  // presented with an invalid stack pointer.
+  ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context)));
+
+  // Read the minidump. Ensure that the memory region is present
+  Minidump minidump(templ.c_str());
+  ASSERT_TRUE(minidump.Read());
+
+  // TODO(ted.mielczarek,mkrebs): Enable this part of the test once
+  // https://breakpad.appspot.com/413002/ is committed.
+#if 0
+  // Make sure there's a thread without a stack.  NOTE: It's okay if
+  // GetThreadList() shows the error: "ERROR: MinidumpThread has a memory
+  // region problem".
+  MinidumpThreadList* dump_thread_list = minidump.GetThreadList();
+  ASSERT_TRUE(dump_thread_list);
+  bool found_empty_stack = false;
+  for (int i = 0; i < dump_thread_list->thread_count(); i++) {
+    MinidumpThread* thread = dump_thread_list->GetThreadAtIndex(i);
+    ASSERT_TRUE(thread->thread() != NULL);
+    // When the stack size is zero bytes, GetMemory() returns NULL.
+    if (thread->GetMemory() == NULL) {
+      found_empty_stack = true;
+      break;
+    }
+  }
+  // NOTE: If you fail this, first make sure that "invalid_stack_pointer"
+  // above is indeed set to an invalid address.
+  ASSERT_TRUE(found_empty_stack);
+#endif
+
+  close(fds[1]);
+}