Fix a couple of bugs where we generate incorrect minidump files on...

src/client/linux/minidump_writer/linux_dumper.cc

 // Copyright (c) 2009, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 57 matching lines @@
   if (sys_ptrace(PTRACE_ATTACH, pid, NULL, NULL) != 0 &&
       errno != 0) {
     return false;
   }
   while (sys_waitpid(pid, NULL, __WALL) < 0) {
     if (errno != EINTR) {
       sys_ptrace(PTRACE_DETACH, pid, NULL, NULL);
       return false;
     }
   }
+#if defined(__i386) || defined(__x86_64)
+  // On x86, the stack pointer is NULL or -1, when executing trusted code in
+  // the seccomp sandbox. Not only does this cause difficulties down the line
+  // when trying to dump the thread's stack, it also results in the minidumps
+  // containing information about the trusted threads. This information is
+  // generally completely meaningless and just pollutes the minidumps.
+  // We thus test the stack pointer and exclude any threads that are part of
+  // the seccomp sandbox's trusted code.
+  user_regs_struct regs;
+  if (sys_ptrace(PTRACE_GETREGS, pid, NULL, &regs) == -1 ||
+#if defined(__i386)

[kmixter, 2010/08/16 17:21:34]

won't this fail to compile on other architectures?

+      !regs.esp
+#elif defined(__x86_64)
+      !regs.rsp
+#endif
+      ) {
+    sys_ptrace(PTRACE_DETACH, pid, NULL, NULL);
+    return false;
+  }
+#endif
   return true;
 }
 
 // Resume a thread by detaching from it.
 static bool ResumeThread(pid_t pid) {
   return sys_ptrace(PTRACE_DETACH, pid, NULL, NULL) >= 0;
 }
 
 inline static bool IsMappedFileOpenUnsafe(
     const google_breakpad::MappingInfo* mapping) {
@@ skipping 16 matching lines @@
 }
 
 bool LinuxDumper::Init() {
   return EnumerateThreads(&threads_) &&
          EnumerateMappings(&mappings_);
 }
 
 bool LinuxDumper::ThreadsSuspend() {
   if (threads_suspended_)
     return true;
-  bool good = true;
-  for (size_t i = 0; i < threads_.size(); ++i)
-    good &= SuspendThread(threads_[i]);
+  for (size_t i = 0; i < threads_.size(); ++i) {
+    if (!SuspendThread(threads_[i])) {
+      // If the thread either disappeared before we could attach to it, or if
+      // it was part of the seccomp sandbox's trusted code, it is OK to
+      // silently drop it from the minidump.
+      memmove(&threads_[i], &threads_[i+1],
+              (threads_.size() - i - 1) * sizeof(threads_[i]));
+      threads_.resize(threads_.size() - 1);
+      --i;
+    }
+  }
   threads_suspended_ = true;
-  return good;
+  return threads_.size() > 0;
 }
 
 bool LinuxDumper::ThreadsResume() {
   if (!threads_suspended_)
     return false;
   bool good = true;
   for (size_t i = 0; i < threads_.size(); ++i)
     good &= ResumeThread(threads_[i]);
   threads_suspended_ = false;
   return good;
@@ skipping 324 matching lines @@
   for (size_t i = 0; i < mappings_.size(); ++i) {
     const uintptr_t start = static_cast<uintptr_t>(mappings_[i]->start_addr);
     if (addr >= start && addr - start < mappings_[i]->size)
       return mappings_[i];
   }
 
   return NULL;
 }
 
 }  // namespace google_breakpad