Linux: Call memset() in a couple places in ExceptionHandler to avoid uninit memory reads under Valgrind.

Also move private static variables into the .cc file.

BUG=chromium:332335
R=ivanpe@chromium.org

Review URL: https://breakpad.appspot.com/5734002

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

2 files changed, 32 insertions, 35 deletions

diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc
index f30e4594..db2f1e62 100644
--- a/src/client/linux/handler/exception_handler.cc
+++ b/src/client/linux/handler/exception_handler.cc
@@ -68,6 +68,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/limits.h>
+#include <pthread.h>
 #include <sched.h>
 #include <signal.h>
 #include <stdio.h>
@@ -150,6 +151,7 @@ void InstallAlternateStackLocked() {
       old_stack.ss_size < kSigStackSize) {
     new_stack.ss_sp = malloc(kSigStackSize);
     new_stack.ss_size = kSigStackSize;
+    memset(new_stack.ss_sp, 0, kSigStackSize);
 
     if (sys_sigaltstack(&new_stack, NULL) == -1) {
       free(new_stack.ss_sp);
@@ -186,13 +188,13 @@ void RestoreAlternateStackLocked() {
   stack_installed = false;
 }
 
-}  // namespace
+// The global exception handler stack. This is need because there may exist
+// multiple ExceptionHandler instances in a process. Each will have itself
+// registered in this stack.
+std::vector<ExceptionHandler*>* g_handler_stack_ = NULL;
+pthread_mutex_t g_handler_stack_mutex_ = PTHREAD_MUTEX_INITIALIZER;
 
-// We can stack multiple exception handlers. In that case, this is the global
-// which holds the stack.
-std::vector<ExceptionHandler*>* ExceptionHandler::handler_stack_ = NULL;
-pthread_mutex_t ExceptionHandler::handler_stack_mutex_ =
-    PTHREAD_MUTEX_INITIALIZER;
+}  // namespace
 
 // Runs before crashing: normal context.
 ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor,
@@ -212,30 +214,30 @@ ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor,
   if (!IsOutOfProcess() && !minidump_descriptor_.IsFD())
     minidump_descriptor_.UpdatePath();
 
-  pthread_mutex_lock(&handler_stack_mutex_);
-  if (!handler_stack_)
-    handler_stack_ = new std::vector<ExceptionHandler*>;
+  pthread_mutex_lock(&g_handler_stack_mutex_);
+  if (!g_handler_stack_)
+    g_handler_stack_ = new std::vector<ExceptionHandler*>;
   if (install_handler) {
     InstallAlternateStackLocked();
     InstallHandlersLocked();
   }
-  handler_stack_->push_back(this);
-  pthread_mutex_unlock(&handler_stack_mutex_);
+  g_handler_stack_->push_back(this);
+  pthread_mutex_unlock(&g_handler_stack_mutex_);
 }
 
 // Runs before crashing: normal context.
 ExceptionHandler::~ExceptionHandler() {
-  pthread_mutex_lock(&handler_stack_mutex_);
+  pthread_mutex_lock(&g_handler_stack_mutex_);
   std::vector<ExceptionHandler*>::iterator handler =
-      std::find(handler_stack_->begin(), handler_stack_->end(), this);
-  handler_stack_->erase(handler);
-  if (handler_stack_->empty()) {
-    delete handler_stack_;
-    handler_stack_ = NULL;
+      std::find(g_handler_stack_->begin(), g_handler_stack_->end(), this);
+  g_handler_stack_->erase(handler);
+  if (g_handler_stack_->empty()) {
+    delete g_handler_stack_;
+    g_handler_stack_ = NULL;
     RestoreAlternateStackLocked();
     RestoreHandlersLocked();
   }
-  pthread_mutex_unlock(&handler_stack_mutex_);
+  pthread_mutex_unlock(&g_handler_stack_mutex_);
 }
 
 // Runs before crashing: normal context.
@@ -295,7 +297,7 @@ void ExceptionHandler::RestoreHandlersLocked() {
 // static
 void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
   // All the exception signals are blocked at this point.
-  pthread_mutex_lock(&handler_stack_mutex_);
+  pthread_mutex_lock(&g_handler_stack_mutex_);
 
   // Sometimes, Breakpad runs inside a process where some other buggy code
   // saves and restores signal handlers temporarily with 'signal'
@@ -322,13 +324,13 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
       // default one to avoid an infinite loop here.
       signal(sig, SIG_DFL);
     }
-    pthread_mutex_unlock(&handler_stack_mutex_);
+    pthread_mutex_unlock(&g_handler_stack_mutex_);
     return;
   }
 
   bool handled = false;
-  for (int i = handler_stack_->size() - 1; !handled && i >= 0; --i) {
-    handled = (*handler_stack_)[i]->HandleSignal(sig, info, uc);
+  for (int i = g_handler_stack_->size() - 1; !handled && i >= 0; --i) {
+    handled = (*g_handler_stack_)[i]->HandleSignal(sig, info, uc);
   }
 
   // Upon returning from this signal handler, sig will become unmasked and then
@@ -342,7 +344,7 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
     RestoreHandlersLocked();
   }
 
-  pthread_mutex_unlock(&handler_stack_mutex_);
+  pthread_mutex_unlock(&g_handler_stack_mutex_);
 
   if (info->si_pid || sig == SIGABRT) {
     // This signal was triggered by somebody sending us the signal with kill().
@@ -398,6 +400,8 @@ bool ExceptionHandler::HandleSignal(int sig, siginfo_t* info, void* uc) {
     sys_prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
   }
   CrashContext context;
+  // Fill in all the holes in the struct to make Valgrind happy.
+  memset(&context, 0, sizeof(context));
   memcpy(&context.siginfo, info, sizeof(siginfo_t));
   memcpy(&context.context, uc, sizeof(struct ucontext));
 #if defined(__aarch64__)
@@ -449,7 +453,7 @@ bool ExceptionHandler::GenerateDump(CrashContext *context) {
   // of caution than smash it into random locations.
   static const unsigned kChildStackSize = 16000;
   PageAllocator allocator;
-  uint8_t* stack = (uint8_t*) allocator.Alloc(kChildStackSize);
+  uint8_t* stack = reinterpret_cast<uint8_t*>(allocator.Alloc(kChildStackSize));
   if (!stack)
     return false;
   // clone() needs the top-most address. (scrub just to be safe)
diff --git a/src/client/linux/handler/exception_handler.h b/src/client/linux/handler/exception_handler.h
index bb88b950..591c3108 100644
--- a/src/client/linux/handler/exception_handler.h
+++ b/src/client/linux/handler/exception_handler.h
@@ -30,15 +30,13 @@
 #ifndef CLIENT_LINUX_HANDLER_EXCEPTION_HANDLER_H_
 #define CLIENT_LINUX_HANDLER_EXCEPTION_HANDLER_H_
 
-#include <string>
-#include <vector>
-
-#include <pthread.h>
 #include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <sys/ucontext.h>
 
+#include <string>
+
 #include "client/linux/crash_generation/crash_generation_client.h"
 #include "client/linux/handler/minidump_descriptor.h"
 #include "client/linux/minidump_writer/minidump_writer.h"
@@ -129,7 +127,7 @@ class ExceptionHandler {
   ExceptionHandler(const MinidumpDescriptor& descriptor,
                    FilterCallback filter,
                    MinidumpCallback callback,
-                   void *callback_context,
+                   void* callback_context,
                    bool install_handler,
                    const int server_fd);
   ~ExceptionHandler();
@@ -228,6 +226,7 @@ class ExceptionHandler {
 
   // Report a crash signal from an SA_SIGINFO signal handler.
   bool HandleSignal(int sig, siginfo_t* info, void* uc);
+
  private:
   // Save the old signal handlers and install new ones.
   static bool InstallHandlersLocked();
@@ -258,12 +257,6 @@ class ExceptionHandler {
   // believes are never read.
   volatile HandlerCallback crash_handler_;
 
-  // The global exception handler stack. This is need becuase there may exist
-  // multiple ExceptionHandler instances in a process. Each will have itself
-  // registered in this stack.
-  static std::vector<ExceptionHandler*> *handler_stack_;
-  static pthread_mutex_t handler_stack_mutex_;
-
   // We need to explicitly enable ptrace of parent processes on some
   // kernels, but we need to know the PID of the cloned process before we
   // can do this. We create a pipe which we can use to block the