Add optional file size limit for minidumps

When there are upwards of 200 threads in a crashing process, each having an
8KB stack, this can result in a huge, 1.8MB minidump file.  So I added a
parameter that, if set, can compel the minidump writer to dump less stack.
More specifically, if the writer expects to go over the limit (due to the
number of threads), then it will dump less of a thread's stack after the
first 20 threads.

There are two ways to specify the limit, depending on how you write minidumps:
1) If you call WriteMinidump() directly, there's now a version of the
   function that takes the minidump size limit as an argument.
2) If you use the ExceptionHandler class, the MinidumpDescriptor object you
   pass to it now has a set_size_limit() method you would call before
   passing it to the constructor.

BUG=chromium-os:31447, chromium:154546
TEST=Wrote a size-limit unittest; Ran unittests
Review URL: https://breakpad.appspot.com/487002

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

5 files changed, 266 insertions, 17 deletions

diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc
index 9a56c14b..fd12642c 100644
--- a/src/client/linux/handler/exception_handler.cc
+++ b/src/client/linux/handler/exception_handler.cc
@@ -521,6 +521,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
                               size_t context_size) {
   if (minidump_descriptor_.IsFD()) {
     return google_breakpad::WriteMinidump(minidump_descriptor_.fd(),
+                                          minidump_descriptor_.size_limit(),
                                           crashing_process,
                                           context,
                                           context_size,
@@ -528,6 +529,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
                                           app_memory_list_);
   }
   return google_breakpad::WriteMinidump(minidump_descriptor_.path(),
+                                        minidump_descriptor_.size_limit(),
                                         crashing_process,
                                         context,
                                         context_size,
diff --git a/src/client/linux/handler/minidump_descriptor.h b/src/client/linux/handler/minidump_descriptor.h
index dc2719ac..3036cadb 100644
--- a/src/client/linux/handler/minidump_descriptor.h
+++ b/src/client/linux/handler/minidump_descriptor.h
@@ -31,6 +31,8 @@
 #define CLIENT_LINUX_HANDLER_MINIDUMP_DESCRIPTOR_H_
 
 #include <assert.h>
+#include <sys/types.h>
+
 #include <string>
 
 #include "common/using_std_string.h"
@@ -48,11 +50,15 @@ class MinidumpDescriptor {
   explicit MinidumpDescriptor(const string& directory)
       : fd_(-1),
         directory_(directory),
-        c_path_(NULL) {
+        c_path_(NULL),
+        size_limit_(-1) {
     assert(!directory.empty());
   }
 
-  explicit MinidumpDescriptor(int fd) : fd_(fd), c_path_(NULL) {
+  explicit MinidumpDescriptor(int fd)
+      : fd_(fd),
+        c_path_(NULL),
+        size_limit_(-1) {
     assert(fd != -1);
   }
 
@@ -71,6 +77,9 @@ class MinidumpDescriptor {
   // Should be called from a normal context: this methods uses the heap.
   void UpdatePath();
 
+  off_t size_limit() const { return size_limit_; }
+  void set_size_limit(off_t limit) { size_limit_ = limit; }
+
  private:
   // The file descriptor where the minidump is generated.
   int fd_;
@@ -82,6 +91,8 @@ class MinidumpDescriptor {
   // The C string of |path_|. Precomputed so it can be access from a compromised
   // context.
   const char* c_path_;
+
+  off_t size_limit_;
 };
 
 }  // namespace google_breakpad
diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc
index 9fb40358..8f2f1950 100644
--- a/src/client/linux/minidump_writer/minidump_writer.cc
+++ b/src/client/linux/minidump_writer/minidump_writer.cc
@@ -55,6 +55,7 @@
 #if defined(__ANDROID__)
 #include <sys/system_properties.h>
 #endif
+#include <sys/types.h>
 #include <sys/ucontext.h>
 #include <sys/user.h>
 #include <sys/utsname.h>
@@ -375,6 +376,22 @@ void CPUFillFromUContext(MDRawContextARM* out, const ucontext* uc,
 
 class MinidumpWriter {
  public:
+  // The following kLimit* constants are for when minidump_size_limit_ is set
+  // and the minidump size might exceed it.
+  //
+  // Estimate for how big each thread's stack will be (in bytes).
+  static const unsigned kLimitAverageThreadStackLength = 8 * 1024;
+  // Number of threads whose stack size we don't want to limit.  These base
+  // threads will simply be the first N threads returned by the dumper (although
+  // the crashing thread will never be limited).  Threads beyond this count are
+  // the extra threads.
+  static const unsigned kLimitBaseThreadCount = 20;
+  // Maximum stack size to dump for any extra thread (in bytes).
+  static const unsigned kLimitMaxExtraThreadStackLen = 2 * 1024;
+  // Make sure this number of additional bytes can fit in the minidump
+  // (exclude the stack data).
+  static const unsigned kLimitMinidumpFudgeFactor = 64 * 1024;
+
   MinidumpWriter(const char* minidump_path,
                  int minidump_fd,
                  const ExceptionHandler::CrashContext* context,
@@ -391,6 +408,7 @@ class MinidumpWriter {
         float_state_(NULL),
 #endif
         dumper_(dumper),
+        minidump_size_limit_(-1),
         memory_blocks_(dumper_->allocator()),
         mapping_list_(mappings),
         app_memory_list_(appmem) {
@@ -629,12 +647,14 @@ class MinidumpWriter {
   }
 
   bool FillThreadStack(MDRawThread* thread, uintptr_t stack_pointer,
-                       uint8_t** stack_copy) {
+                       int max_stack_len, 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 (max_stack_len >= 0 && stack_len > max_stack_len)
+        stack_len = max_stack_len;
       if (!memory.Allocate(stack_len))
         return false;
       *stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len));
@@ -666,6 +686,21 @@ class MinidumpWriter {
 
     *list.get() = num_threads;
 
+    // If there's a minidump size limit, check if it might be exceeded.  Since
+    // most of the space is filled with stack data, just check against that.
+    // If this expects to exceed the limit, set extra_thread_stack_len such
+    // that any thread beyond the first kLimitBaseThreadCount threads will
+    // have only kLimitMaxExtraThreadStackLen bytes dumped.
+    int extra_thread_stack_len = -1;  // default to no maximum
+    if (minidump_size_limit_ >= 0) {
+      const unsigned estimated_total_stack_size = num_threads *
+          kLimitAverageThreadStackLength;
+      const off_t estimated_minidump_size = minidump_writer_.position() +
+          estimated_total_stack_size + kLimitMinidumpFudgeFactor;
+      if (estimated_minidump_size > minidump_size_limit_)
+        extra_thread_stack_len = kLimitMaxExtraThreadStackLen;
+    }
+
     for (unsigned i = 0; i < num_threads; ++i) {
       MDRawThread thread;
       my_memset(&thread, 0, sizeof(thread));
@@ -679,7 +714,7 @@ class MinidumpWriter {
           ucontext_ &&
           !dumper_->IsPostMortem()) {
         uint8_t* stack_copy;
-        if (!FillThreadStack(&thread, GetStackPointer(), &stack_copy))
+        if (!FillThreadStack(&thread, GetStackPointer(), -1, &stack_copy))
           return false;
 
         // Copy 256 bytes around crashing instruction pointer to minidump.
@@ -740,7 +775,11 @@ class MinidumpWriter {
           return false;
 
         uint8_t* stack_copy;
-        if (!FillThreadStack(&thread, info.stack_pointer, &stack_copy))
+        int max_stack_len = -1;  // default to no maximum for this thread
+        if (minidump_size_limit_ >= 0 && i >= kLimitBaseThreadCount)
+          max_stack_len = extra_thread_stack_len;
+        if (!FillThreadStack(&thread, info.stack_pointer, max_stack_len,
+            &stack_copy))
           return false;
 
         TypedMDRVA<RawContextCPU> cpu(&minidump_writer_);
@@ -1112,6 +1151,8 @@ class MinidumpWriter {
     return true;
   }
 
+  void set_minidump_size_limit(off_t limit) { minidump_size_limit_ = limit; }
+
  private:
   void* Alloc(unsigned bytes) {
     return dumper_->allocator()->Alloc(bytes);
@@ -1437,6 +1478,7 @@ class MinidumpWriter {
   const struct _libc_fpstate* const float_state_;  // ditto
   LinuxDumper* dumper_;
   MinidumpFileWriter minidump_writer_;
+  off_t minidump_size_limit_;
   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
@@ -1452,21 +1494,26 @@ class MinidumpWriter {
 
 bool WriteMinidumpImpl(const char* minidump_path,
                        int minidump_fd,
+                       off_t minidump_size_limit,
                        pid_t crashing_process,
                        const void* blob, size_t blob_size,
                        const MappingList& mappings,
                        const AppMemoryList& appmem) {
-  if (blob_size != sizeof(ExceptionHandler::CrashContext))
-    return false;
-  const ExceptionHandler::CrashContext* context =
-      reinterpret_cast<const ExceptionHandler::CrashContext*>(blob);
   LinuxPtraceDumper dumper(crashing_process);
-  dumper.set_crash_address(
-      reinterpret_cast<uintptr_t>(context->siginfo.si_addr));
-  dumper.set_crash_signal(context->siginfo.si_signo);
-  dumper.set_crash_thread(context->tid);
+  const ExceptionHandler::CrashContext* context = NULL;
+  if (blob) {
+    if (blob_size != sizeof(ExceptionHandler::CrashContext))
+      return false;
+    context = reinterpret_cast<const ExceptionHandler::CrashContext*>(blob);
+    dumper.set_crash_address(
+        reinterpret_cast<uintptr_t>(context->siginfo.si_addr));
+    dumper.set_crash_signal(context->siginfo.si_signo);
+    dumper.set_crash_thread(context->tid);
+  }
   MinidumpWriter writer(minidump_path, minidump_fd, context, mappings,
                         appmem, &dumper);
+  // Set desired limit for file size of minidump (-1 means no limit).
+  writer.set_minidump_size_limit(minidump_size_limit);
   if (!writer.Init())
     return false;
   return writer.Dump();
@@ -1478,13 +1525,15 @@ namespace google_breakpad {
 
 bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
                    const void* blob, size_t blob_size) {
-  return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size,
+  return WriteMinidumpImpl(minidump_path, -1, -1,
+                           crashing_process, blob, blob_size,
                            MappingList(), AppMemoryList());
 }
 
 bool WriteMinidump(int minidump_fd, pid_t crashing_process,
                    const void* blob, size_t blob_size) {
-  return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size,
+  return WriteMinidumpImpl(NULL, minidump_fd, -1,
+                           crashing_process, blob, blob_size,
                            MappingList(), AppMemoryList());
 }
 
@@ -1505,7 +1554,8 @@ bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
                    const AppMemoryList& appmem) {
-  return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size,
+  return WriteMinidumpImpl(minidump_path, -1, -1, crashing_process,
+                           blob, blob_size,
                            mappings, appmem);
 }
 
@@ -1513,7 +1563,28 @@ bool WriteMinidump(int minidump_fd, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
                    const AppMemoryList& appmem) {
-  return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size,
+  return WriteMinidumpImpl(NULL, minidump_fd, -1, crashing_process,
+                           blob, blob_size,
+                           mappings, appmem);
+}
+
+bool WriteMinidump(const char* minidump_path, off_t minidump_size_limit,
+                   pid_t crashing_process,
+                   const void* blob, size_t blob_size,
+                   const MappingList& mappings,
+                   const AppMemoryList& appmem) {
+  return WriteMinidumpImpl(minidump_path, -1, minidump_size_limit,
+                           crashing_process, blob, blob_size,
+                           mappings, appmem);
+}
+
+bool WriteMinidump(int minidump_fd, off_t minidump_size_limit,
+                   pid_t crashing_process,
+                   const void* blob, size_t blob_size,
+                   const MappingList& mappings,
+                   const AppMemoryList& appmem) {
+  return WriteMinidumpImpl(NULL, minidump_fd, minidump_size_limit,
+                           crashing_process, blob, blob_size,
                            mappings, appmem);
 }
 
diff --git a/src/client/linux/minidump_writer/minidump_writer.h b/src/client/linux/minidump_writer/minidump_writer.h
index 28bfbdaf..790ff723 100644
--- a/src/client/linux/minidump_writer/minidump_writer.h
+++ b/src/client/linux/minidump_writer/minidump_writer.h
@@ -31,6 +31,7 @@
 #define CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_H_
 
 #include <stdint.h>
+#include <sys/types.h>
 #include <unistd.h>
 
 #include <list>
@@ -102,6 +103,18 @@ bool WriteMinidump(int minidump_fd, pid_t crashing_process,
                    const MappingList& mappings,
                    const AppMemoryList& appdata);
 
+// These overloads also allow passing a file size limit for the minidump.
+bool WriteMinidump(const char* minidump_path, off_t minidump_size_limit,
+                   pid_t crashing_process,
+                   const void* blob, size_t blob_size,
+                   const MappingList& mappings,
+                   const AppMemoryList& appdata);
+bool WriteMinidump(int minidump_fd, off_t minidump_size_limit,
+                   pid_t crashing_process,
+                   const void* blob, size_t blob_size,
+                   const MappingList& mappings,
+                   const AppMemoryList& appdata);
+
 bool WriteMinidump(const char* filename,
                    const MappingList& mappings,
                    const AppMemoryList& appdata,
diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
index 153f3528..6beae05d 100644
--- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc
+++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
@@ -580,4 +580,156 @@ TEST(MinidumpWriterTest, InvalidStackPointer) {
   close(fds[1]);
 }
 
+// Test that limiting the size of the minidump works.
+TEST(MinidumpWriterTest, MinidumpSizeLimit) {
+  static const int kNumberOfThreadsInHelperProgram = 40;
+
+  char number_of_threads_arg[3];
+  sprintf(number_of_threads_arg, "%d", kNumberOfThreadsInHelperProgram);
+
+  string helper_path(GetHelperBinary());
+  if (helper_path.empty()) {
+    FAIL() << "Couldn't find helper binary";
+    exit(1);
+  }
+
+  int fds[2];
+  ASSERT_NE(-1, pipe(fds));
+
+  pid_t child_pid = fork();
+  if (child_pid == 0) {
+    // In child process.
+    close(fds[0]);
+
+    // Pass the pipe fd and the number of threads as arguments.
+    char pipe_fd_string[8];
+    sprintf(pipe_fd_string, "%d", fds[1]);
+    execl(helper_path.c_str(),
+          helper_path.c_str(),
+          pipe_fd_string,
+          number_of_threads_arg,
+          NULL);
+  }
+  close(fds[1]);
+
+  // Wait for all child threads to indicate that they have started
+  for (int threads = 0; threads < kNumberOfThreadsInHelperProgram; threads++) {
+    struct pollfd pfd;
+    memset(&pfd, 0, sizeof(pfd));
+    pfd.fd = fds[0];
+    pfd.events = POLLIN | POLLERR;
+
+    const int r = HANDLE_EINTR(poll(&pfd, 1, 1000));
+    ASSERT_EQ(1, r);
+    ASSERT_TRUE(pfd.revents & POLLIN);
+    uint8_t junk;
+    ASSERT_EQ(read(fds[0], &junk, sizeof(junk)), sizeof(junk));
+  }
+  close(fds[0]);
+
+  // There is a race here because we may stop a child thread before
+  // it is actually running the busy loop. Empirically this sleep
+  // is sufficient to avoid the race.
+  usleep(100000);
+
+  // Child and its threads are ready now.
+
+
+  off_t normal_file_size;
+  int total_normal_stack_size = 0;
+  AutoTempDir temp_dir;
+
+  // First, write a minidump with no size limit.
+  {
+    string normal_dump = temp_dir.path() +
+        "/minidump-writer-unittest.dmp";
+    ASSERT_TRUE(WriteMinidump(normal_dump.c_str(), -1,
+                              child_pid, NULL, 0,
+                              MappingList(), AppMemoryList()));
+    struct stat st;
+    ASSERT_EQ(0, stat(normal_dump.c_str(), &st));
+    ASSERT_GT(st.st_size, 0u);
+    normal_file_size = st.st_size;
+
+    Minidump minidump(normal_dump.c_str());
+    ASSERT_TRUE(minidump.Read());
+    MinidumpThreadList* dump_thread_list = minidump.GetThreadList();
+    ASSERT_TRUE(dump_thread_list);
+    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.
+      MinidumpMemoryRegion* memory = thread->GetMemory();
+      ASSERT_TRUE(memory != NULL);
+      total_normal_stack_size += memory->GetSize();
+    }
+  }
+
+  // Second, write a minidump with a size limit big enough to not trigger
+  // anything.
+  {
+    // Set size limit arbitrarily 1MB larger than the normal file size -- such
+    // that the limiting code will not kick in.
+    const off_t minidump_size_limit = normal_file_size + 1024*1024;
+
+    string same_dump = temp_dir.path() +
+        "/minidump-writer-unittest-same.dmp";
+    ASSERT_TRUE(WriteMinidump(same_dump.c_str(), minidump_size_limit,
+                              child_pid, NULL, 0,
+                              MappingList(), AppMemoryList()));
+    struct stat st;
+    ASSERT_EQ(0, stat(same_dump.c_str(), &st));
+    // Make sure limiting wasn't actually triggered.  NOTE: If you fail this,
+    // first make sure that "minidump_size_limit" above is indeed set to a
+    // large enough value -- the limit-checking code in minidump_writer.cc
+    // does just a rough estimate.
+    ASSERT_EQ(normal_file_size, st.st_size);
+  }
+
+  // Third, write a minidump with a size limit small enough to be triggered.
+  {
+    // Set size limit to the normal file size minus some arbitrary amount --
+    // enough to make the limiting code kick in.
+    const off_t minidump_size_limit = normal_file_size - 64*1024;
+
+    string limit_dump = temp_dir.path() +
+        "/minidump-writer-unittest-limit.dmp";
+    ASSERT_TRUE(WriteMinidump(limit_dump.c_str(), minidump_size_limit,
+                              child_pid, NULL, 0,
+                              MappingList(), AppMemoryList()));
+    struct stat st;
+    ASSERT_EQ(0, stat(limit_dump.c_str(), &st));
+    ASSERT_GT(st.st_size, 0u);
+    // Make sure the file size is at least smaller than the original.
+    EXPECT_LT(st.st_size, normal_file_size);
+
+    Minidump minidump(limit_dump.c_str());
+    ASSERT_TRUE(minidump.Read());
+    MinidumpThreadList* dump_thread_list = minidump.GetThreadList();
+    ASSERT_TRUE(dump_thread_list);
+    int total_limit_stack_size = 0;
+    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.
+      MinidumpMemoryRegion* memory = thread->GetMemory();
+      ASSERT_TRUE(memory != NULL);
+      total_limit_stack_size += memory->GetSize();
+    }
+
+    // Make sure stack size shrunk by at least 1KB per extra thread.  The
+    // definition of kLimitBaseThreadCount here was copied from class
+    // MinidumpWriter in minidump_writer.cc.
+    const unsigned kLimitBaseThreadCount = 20;
+    const unsigned kMinPerExtraThreadStackReduction = 1024;
+    const int min_expected_reduction = (kNumberOfThreadsInHelperProgram -
+        kLimitBaseThreadCount) * kMinPerExtraThreadStackReduction;
+    EXPECT_LT(total_limit_stack_size,
+              total_normal_stack_size - min_expected_reduction);
+  }
+
+  // Kill the helper program.
+  kill(child_pid, SIGKILL);
+}
+
 }  // namespace