crash-reporter: Generate kernel crash signatures for server-side grouping of similar crashes

BUG=5868
TEST=KernelCrash, UserCrash, CrashSender, unittests, and manual inspection of sent report.

Change-Id: I31991895c9ac719ac1832d588ae3360500ef0c26

Review URL: http://codereview.chromium.org/4018008
diff --git a/crash_reporter/Makefile b/crash_reporter/Makefile
index fc2d680..0523c19 100644
--- a/crash_reporter/Makefile
+++ b/crash_reporter/Makefile
@@ -17,8 +17,10 @@
 	unclean_shutdown_collector_test \
 	user_collector_test
 
+LDCONFIG = $(shell $(PKG_CONFIG) --libs libpcrecpp)
+
 # -lglib-2.0 is needed by libbase.a now.
-COMMON_LIBS = -lbase -lpthread -lglib-2.0 -lgflags -lrt
+COMMON_LIBS = -lbase -lpthread -lglib-2.0 -lgflags -lrt $(LDCONFIG)
 REPORTER_LIBS = $(COMMON_LIBS) -lmetrics
 
 TEST_LIBS = $(COMMON_LIBS) -lgtest -lgmock
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index 0391fa5..5ea06e2 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -39,7 +39,9 @@
 // number of core files or minidumps reaches this number.
 const int CrashCollector::kMaxCrashDirectorySize = 32;
 
-CrashCollector::CrashCollector() : forced_crash_directory_(NULL) {
+CrashCollector::CrashCollector()
+    : forced_crash_directory_(NULL),
+      lsb_release_(kLsbRelease) {
 }
 
 CrashCollector::~CrashCollector() {
@@ -247,12 +249,17 @@
   return !any_errors;
 }
 
+void CrashCollector::AddCrashMetaData(const std::string &key,
+                                      const std::string &value) {
+  extra_metadata_.append(StringPrintf("%s=%s\n", key.c_str(), value.c_str()));
+}
+
 void CrashCollector::WriteCrashMetaData(const FilePath &meta_path,
                                         const std::string &exec_name,
                                         const std::string &payload_path) {
   std::map<std::string, std::string> contents;
-  if (!ReadKeyValueFile(FilePath(std::string(kLsbRelease)), '=', &contents)) {
-    logger_->LogError("Problem parsing %s", kLsbRelease);
+  if (!ReadKeyValueFile(FilePath(std::string(lsb_release_)), '=', &contents)) {
+    logger_->LogError("Problem parsing %s", lsb_release_);
     // Even though there was some failure, take as much as we could read.
   }
   std::string version("unknown");
@@ -262,10 +269,11 @@
   }
   int64 payload_size = -1;
   file_util::GetFileSize(FilePath(payload_path), &payload_size);
-  std::string meta_data = StringPrintf("exec_name=%s\n"
+  std::string meta_data = StringPrintf("%sexec_name=%s\n"
                                        "ver=%s\n"
                                        "payload_size=%lld\n"
                                        "done=1\n",
+                                       extra_metadata_.c_str(),
                                        exec_name.c_str(),
                                        version.c_str(),
                                        payload_size);
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index 97bf0d6..b4cabf9 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -40,6 +40,7 @@
   FRIEND_TEST(CrashCollectorTest, GetCrashDirectoryInfo);
   FRIEND_TEST(CrashCollectorTest, FormatDumpBasename);
   FRIEND_TEST(CrashCollectorTest, Initialize);
+  FRIEND_TEST(CrashCollectorTest, MetaData);
   FRIEND_TEST(CrashCollectorTest, ReadKeyValueFile);
   FRIEND_TEST(CrashCollectorTest, Sanitize);
 
@@ -87,6 +88,11 @@
                         char separator,
                         std::map<std::string, std::string> *dictionary);
 
+  // Add non-standard meta data to the crash metadata file.  Call
+  // before calling WriteCrashMetaData.  Key must not contain "=" or
+  // "\n" characters.  Value must not contain "\n" characters.
+  void AddCrashMetaData(const std::string &key, const std::string &value);
+
   // Write a file of metadata about crash.
   void WriteCrashMetaData(const FilePath &meta_path,
                           const std::string &exec_name,
@@ -95,7 +101,9 @@
   CountCrashFunction count_crash_function_;
   IsFeedbackAllowedFunction is_feedback_allowed_function_;
   SystemLogging *logger_;
+  std::string extra_metadata_;
   const char *forced_crash_directory_;
+  const char *lsb_release_;
 };
 
 #endif  // _CRASH_REPORTER_CRASH_COLLECTOR_H_
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index 2dbf2bb..ffab06a 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -218,6 +218,30 @@
   EXPECT_TRUE(i != dictionary.end() && i->second == "");
 }
 
+TEST_F(CrashCollectorTest, MetaData) {
+  FilePath meta_file = test_dir_.Append("generated.meta");
+  FilePath lsb_release = test_dir_.Append("lsb-release");
+  FilePath payload_file = test_dir_.Append("payload-file");
+  std::string contents;
+  collector_.lsb_release_ = lsb_release.value().c_str();
+  const char kLsbContents[] = "CHROMEOS_RELEASE_VERSION=version\n";
+  ASSERT_TRUE(
+      file_util::WriteFile(lsb_release,
+                           kLsbContents, strlen(kLsbContents)));
+  const char kPayload[] = "foo";
+  ASSERT_TRUE(
+      file_util::WriteFile(payload_file,
+                           kPayload, strlen(kPayload)));
+  collector_.AddCrashMetaData("foo", "bar");
+  collector_.WriteCrashMetaData(meta_file, "kernel", payload_file.value());
+  EXPECT_TRUE(file_util::ReadFileToString(meta_file, &contents));
+  EXPECT_EQ("foo=bar\n"
+            "exec_name=kernel\n"
+            "ver=version\n"
+            "payload_size=3\n"
+            "done=1\n", contents);
+}
+
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index 2f8e987..07dcd9c 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -18,6 +18,8 @@
 #pragma GCC diagnostic ignored "-Wstrict-aliasing"
 DEFINE_bool(init, false, "Initialize crash logging");
 DEFINE_bool(clean_shutdown, false, "Signal clean shutdown");
+DEFINE_string(generate_kernel_signature, "",
+              "Generate signature from given kcrash file");
 DEFINE_bool(crash_test, false, "Crash test");
 DEFINE_int32(pid, -1, "Crashing PID");
 DEFINE_int32(signal, -1, "Signal causing crash");
@@ -146,6 +148,25 @@
   return 0;
 }
 
+// Interactive/diagnostics mode for generating kernel crash signatures.
+static int GenerateKernelSignature(KernelCollector *kernel_collector) {
+  std::string kcrash_contents;
+  std::string signature;
+  if (!file_util::ReadFileToString(FilePath(FLAGS_generate_kernel_signature),
+                                   &kcrash_contents)) {
+    fprintf(stderr, "Could not read file.\n");
+    return 1;
+  }
+  if (!kernel_collector->ComputeKernelStackSignature(
+          kcrash_contents,
+          &signature,
+          true)) {
+    fprintf(stderr, "Signature could not be generated.\n");
+    return 1;
+  }
+  printf("Kernel crash signature is \"%s\".\n", signature.c_str());
+  return 0;
+}
 
 int main(int argc, char *argv[]) {
   google::ParseCommandLineFlags(&argc, &argv, true);
@@ -185,5 +206,9 @@
     return 0;
   }
 
+  if (!FLAGS_generate_kernel_signature.empty()) {
+    return GenerateKernelSignature(&kernel_collector);
+  }
+
   return HandleUserCrash(&user_collector);
 }
diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender
index 9d34e34..ebb380e 100644
--- a/crash_reporter/crash_sender
+++ b/crash_reporter/crash_sender
@@ -201,6 +201,7 @@
   local hwclass="$(get_hardware_class)"
   local payload_extension="${kind}"
   local write_payload_size="$(get_key_value "${meta_path}" "payload_size")"
+  local sig="$(get_key_value "${meta_path}" "sig")"
   [ "${kind}" = "minidump" ] && payload_extension="dmp"
   local report_payload="$(get_base "${meta_path}").${payload_extension}"
   local send_payload_size="$(stat --printf=%s "${report_payload}")"
@@ -214,6 +215,7 @@
     lecho "  URL: ${url}"
     lecho "  Board: ${board}"
     lecho "  HWClass: ${hwclass}"
+    [ "${sig}" != "undefined" ] && lecho "  Sig: ${sig}"
   fi
   lecho "  Exec name: ${exec_name}"
   if is_mock; then
@@ -235,6 +237,17 @@
   local report_id="${TMP_DIR}/report_id"
   local curl_stderr="${TMP_DIR}/curl_stderr"
 
+  local extra_key1="write_payload_size"
+  local extra_value1="${write_payload_size}"
+  local extra_key2="send_payload_size"
+  local extra_value2="${send_payload_size}"
+  if [ "${kind}" = "kcrash" ]; then
+    extra_key1="sig"
+    extra_value1="${sig}"
+    extra_key2="sig2"
+    extra_value2="${sig}"
+  fi
+
   set +e
   curl "${url}" \
     -F "prod=${CHROMEOS_PRODUCT}" \
@@ -243,8 +256,8 @@
     -F "board=${board}" \
     -F "hwclass=${hwclass}" \
     -F "exec_name=${exec_name}" \
-    -F "write_payload_size=${write_payload_size}" \
-    -F "send_payload_size=${send_payload_size}" \
+    -F "${extra_key1}=${extra_value1}" \
+    -F "${extra_key2}=${extra_value2}" \
     -F "guid=<${CONSENT_ID}" -o "${report_id}" 2>"${curl_stderr}"
   curl_result=$?
   set -e
diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc
index c6bb33f..a95d757 100644
--- a/crash_reporter/kernel_collector.cc
+++ b/crash_reporter/kernel_collector.cc
@@ -9,12 +9,21 @@
 #include "base/string_util.h"
 #include "crash-reporter/system_logging.h"
 
-static const char kKernelExecName[] = "kernel";
-static const char kPreservedDumpPath[] = "/sys/kernel/debug/preserved/kcrash";
-const pid_t kKernelPid = 0;
-const uid_t kRootUid = 0;
-
 const char KernelCollector::kClearingSequence[] = " ";
+static const char kDefaultKernelStackSignature[] =
+    "kernel-UnspecifiedStackSignature";
+static const char kKernelExecName[] = "kernel";
+const pid_t kKernelPid = 0;
+static const char kKernelSignatureKey[] = "sig";
+// Byte length of maximum human readable portion of a kernel crash signature.
+static const int kMaxHumanStringLength = 40;
+static const char kPreservedDumpPath[] = "/sys/kernel/debug/preserved/kcrash";
+const uid_t kRootUid = 0;
+// Time in seconds from the final kernel log message for a call stack
+// to count towards the signature of the kcrash.
+static const int kSignatureTimestampWindow = 2;
+// Kernel log timestamp regular expression.
+static const std::string kTimestampRegex("^<.*>\\[\\s*(\\d+\\.\\d+)\\]");
 
 KernelCollector::KernelCollector()
     : is_enabled_(false),
@@ -66,6 +75,171 @@
   return true;
 }
 
+// Hash a string to a number.  We define our own hash function to not
+// be dependent on a C++ library that might change.  This function
+// uses basically the same approach as tr1/functional_hash.h but with
+// a larger prime number (16127 vs 131).
+static unsigned HashString(const std::string &input) {
+  unsigned hash = 0;
+  for (size_t i = 0; i < input.length(); ++i)
+    hash = hash * 16127 + input[i];
+  return hash;
+}
+
+void KernelCollector::ProcessStackTrace(
+    pcrecpp::StringPiece kernel_dump,
+    bool print_diagnostics,
+    unsigned *hash,
+    float *last_stack_timestamp) {
+  pcrecpp::RE line_re("(.+)", pcrecpp::MULTILINE());
+  pcrecpp::RE stack_trace_start_re(kTimestampRegex + " Call Trace:$");
+  // Match lines such as the following and grab out "error_code".
+  // <4>[ 6066.849504]  [<7937bcee>] error_code+0x66/0x6c
+  pcrecpp::RE stack_entry_re(kTimestampRegex +
+                             "  \\[<.*>\\]([\\s\\?]+)([^\\+ ]+)");
+  std::string line;
+  std::string hashable;
+
+  *hash = 0;
+  *last_stack_timestamp = 0;
+
+  while (line_re.FindAndConsume(&kernel_dump, &line)) {
+    std::string certainty;
+    std::string function_name;
+    if (stack_trace_start_re.PartialMatch(line, last_stack_timestamp)) {
+      if (print_diagnostics) {
+        printf("Stack trace starting. Clearing any prior traces.\n");
+      }
+      hashable.clear();
+    } else if (stack_entry_re.PartialMatch(line,
+                                           last_stack_timestamp,
+                                           &certainty,
+                                           &function_name)) {
+      bool is_certain = certainty.find('?') == std::string::npos;
+      if (print_diagnostics) {
+        printf("@%f: stack entry for %s (%s)\n",
+               *last_stack_timestamp,
+               function_name.c_str(),
+               is_certain ? "certain" : "uncertain");
+      }
+      // Do not include any uncertain (prefixed by '?') frames in our hash.
+      if (!is_certain)
+        continue;
+      if (!hashable.empty())
+        hashable.append("|");
+      hashable.append(function_name);
+    }
+  }
+
+  *hash = HashString(hashable);
+
+  if (print_diagnostics) {
+    printf("Hash based on stack trace: \"%s\" at %f.\n",
+           hashable.c_str(), *last_stack_timestamp);
+  }
+}
+
+bool KernelCollector::FindCrashingFunction(
+    pcrecpp::StringPiece kernel_dump,
+    bool print_diagnostics,
+    float stack_trace_timestamp,
+    std::string *crashing_function) {
+  pcrecpp::RE eip_re(kTimestampRegex + " EIP: \\[<.*>\\] ([^\\+ ]+).*",
+                     pcrecpp::MULTILINE());
+  float timestamp = 0;
+  while (eip_re.FindAndConsume(&kernel_dump, &timestamp, crashing_function)) {
+    if (print_diagnostics) {
+      printf("@%f: found crashing function %s\n",
+             timestamp,
+             crashing_function->c_str());
+    }
+  }
+  if (timestamp == 0) {
+    if (print_diagnostics) {
+      printf("Found no crashing function.\n");
+    }
+    return false;
+  }
+  if (stack_trace_timestamp != 0 &&
+      abs(stack_trace_timestamp - timestamp) > kSignatureTimestampWindow) {
+    if (print_diagnostics) {
+      printf("Found crashing function but not within window.\n");
+    }
+    return false;
+  }
+  if (print_diagnostics) {
+    printf("Found crashing function %s\n", crashing_function->c_str());
+  }
+  return true;
+}
+
+bool KernelCollector::FindPanicMessage(pcrecpp::StringPiece kernel_dump,
+                                       bool print_diagnostics,
+                                       std::string *panic_message) {
+  // Match lines such as the following and grab out "Fatal exception"
+  // <0>[  342.841135] Kernel panic - not syncing: Fatal exception
+  pcrecpp::RE kernel_panic_re(kTimestampRegex +
+                              " Kernel panic[^\\:]*\\:\\s*(.*)",
+                              pcrecpp::MULTILINE());
+  float timestamp = 0;
+  while (kernel_panic_re.FindAndConsume(&kernel_dump,
+                                        &timestamp,
+                                        panic_message)) {
+    if (print_diagnostics) {
+      printf("@%f: panic message %s\n",
+             timestamp,
+             panic_message->c_str());
+    }
+  }
+  if (timestamp == 0) {
+    if (print_diagnostics) {
+      printf("Found no panic message.\n");
+    }
+    return false;
+  }
+  return true;
+}
+
+bool KernelCollector::ComputeKernelStackSignature(
+    const std::string &kernel_dump,
+    std::string *kernel_signature,
+    bool print_diagnostics) {
+  unsigned stack_hash = 0;
+  float last_stack_timestamp = 0;
+  std::string human_string;
+
+  ProcessStackTrace(kernel_dump,
+                    print_diagnostics,
+                    &stack_hash,
+                    &last_stack_timestamp);
+
+  if (!FindCrashingFunction(kernel_dump,
+                            print_diagnostics,
+                            last_stack_timestamp,
+                            &human_string)) {
+    if (!FindPanicMessage(kernel_dump, print_diagnostics, &human_string)) {
+      if (print_diagnostics) {
+        printf("Found no human readable string, using empty string.\n");
+      }
+      human_string.clear();
+    }
+  }
+
+  if (human_string.empty() && stack_hash == 0) {
+    if (print_diagnostics) {
+      printf("Found neither a stack nor a human readable string, failing.\n");
+    }
+    return false;
+  }
+
+  human_string = human_string.substr(0, kMaxHumanStringLength);
+  *kernel_signature = StringPrintf("%s-%s-%08X",
+                                   kKernelExecName,
+                                   human_string.c_str(),
+                                   stack_hash);
+  return true;
+}
+
 bool KernelCollector::Collect() {
   std::string kernel_dump;
   FilePath root_crash_directory;
@@ -75,9 +249,19 @@
   if (kernel_dump.empty()) {
     return false;
   }
-  logger_->LogInfo("Received prior crash notification from kernel");
+  std::string signature;
+  if (!ComputeKernelStackSignature(kernel_dump, &signature, false)) {
+    signature = kDefaultKernelStackSignature;
+  }
 
-  if (is_feedback_allowed_function_()) {
+  bool feedback = is_feedback_allowed_function_();
+
+  logger_->LogInfo("Received prior crash notification from "
+                   "kernel (signature %s) (%s)",
+                   signature.c_str(),
+                   feedback ? "handling" : "ignoring");
+
+  if (feedback) {
     count_crash_function_();
 
     if (!GetCreatedCrashDirectoryByEuid(kRootUid,
@@ -101,16 +285,15 @@
       return true;
     }
 
+    AddCrashMetaData(kKernelSignatureKey, signature);
     WriteCrashMetaData(
         root_crash_directory.Append(
             StringPrintf("%s.meta", dump_basename.c_str())),
         kKernelExecName,
         kernel_crash_path.value());
 
-    logger_->LogInfo("Collected kernel crash diagnostics into %s",
+    logger_->LogInfo("Stored kcrash to %s",
                      kernel_crash_path.value().c_str());
-  } else {
-    logger_->LogInfo("Crash not saved since metrics disabled");
   }
   if (!ClearPreservedDump()) {
     return false;
diff --git a/crash_reporter/kernel_collector.h b/crash_reporter/kernel_collector.h
index e6cd573..708fd1b 100644
--- a/crash_reporter/kernel_collector.h
+++ b/crash_reporter/kernel_collector.h
@@ -5,6 +5,8 @@
 #ifndef _CRASH_REPORTER_KERNEL_COLLECTOR_H_
 #define _CRASH_REPORTER_KERNEL_COLLECTOR_H_
 
+#include <pcrecpp.h>
+
 #include <string>
 
 #include "base/file_path.h"
@@ -34,6 +36,11 @@
   // a dump (even if there were problems storing the dump), false otherwise.
   bool Collect();
 
+  // Compute a stack signature string from a kernel dump.
+  bool ComputeKernelStackSignature(const std::string &kernel_dump,
+                                   std::string *kernel_signature,
+                                   bool print_diagnostics);
+
  private:
   friend class KernelCollectorTest;
   FRIEND_TEST(KernelCollectorTest, ClearPreservedDump);
@@ -43,6 +50,18 @@
   bool LoadPreservedDump(std::string *contents);
   bool ClearPreservedDump();
 
+  void ProcessStackTrace(pcrecpp::StringPiece kernel_dump,
+                         bool print_diagnostics,
+                         unsigned *hash,
+                         float *last_stack_timestamp);
+  bool FindCrashingFunction(pcrecpp::StringPiece kernel_dump,
+                            bool print_diagnostics,
+                            float stack_trace_timestamp,
+                            std::string *crashing_function);
+  bool FindPanicMessage(pcrecpp::StringPiece kernel_dump,
+                        bool print_diagnostics,
+                        std::string *panic_message);
+
   bool is_enabled_;
   FilePath preserved_dump_path_;
   static const char kClearingSequence[];
diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc
index 44a5ba0..93b253d 100644
--- a/crash_reporter/kernel_collector_test.cc
+++ b/crash_reporter/kernel_collector_test.cc
@@ -135,19 +135,18 @@
   SetUpSuccessfulCollect();
   s_metrics = false;
   ASSERT_TRUE(collector_.Collect());
-  ASSERT_NE(std::string::npos,
-            logging_.log().find("Crash not saved since metrics disabled"));
+  ASSERT_NE(std::string::npos, logging_.log().find("(ignoring)"));
   ASSERT_EQ(0, s_crashes);
 
   CheckPreservedDumpClear();
 }
 
-
 TEST_F(KernelCollectorTest, CollectOK) {
   SetUpSuccessfulCollect();
   ASSERT_TRUE(collector_.Collect());
   ASSERT_EQ(1, s_crashes);
-  static const char kNamePrefix[] = "Collected kernel crash diagnostics into ";
+  ASSERT_NE(std::string::npos, logging_.log().find("(handling)"));
+  static const char kNamePrefix[] = "Stored kcrash to ";
   size_t pos = logging_.log().find(kNamePrefix);
   ASSERT_NE(std::string::npos, pos);
   pos += strlen(kNamePrefix);
@@ -165,6 +164,113 @@
   CheckPreservedDumpClear();
 }
 
+TEST_F(KernelCollectorTest, ComputeKernelStackSignature) {
+  const char kBugToPanic[] =
+      "<4>[ 6066.829029]  [<79039d16>] ? run_timer_softirq+0x165/0x1e6\n"
+      "<4>[ 6066.829029]  [<790340af>] ignore_old_stack+0xa6/0x143\n"
+      "<0>[ 6066.829029] EIP: [<b82d7c15>] ieee80211_stop_tx_ba_session+"
+          "0xa3/0xb5 [mac80211] SS:ESP 0068:7951febc\n"
+      "<0>[ 6066.829029] CR2: 00000000323038a7\n"
+      "<4>[ 6066.845422] ---[ end trace 12b058bb46c43500 ]---\n"
+      "<0>[ 6066.845747] Kernel panic - not syncing: Fatal exception "
+          "in interrupt\n"
+      "<0>[ 6066.846902] Call Trace:\n"
+      "<4>[ 6066.846902]  [<7937a07b>] ? printk+0x14/0x19\n"
+      "<4>[ 6066.949779]  [<79379fc1>] panic+0x3e/0xe4\n"
+      "<4>[ 6066.949971]  [<7937c5c5>] oops_end+0x73/0x81\n"
+      "<4>[ 6066.950208]  [<7901b260>] no_context+0x10d/0x117\n";
+  std::string signature;
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kBugToPanic, &signature, false));
+  EXPECT_EQ("kernel-ieee80211_stop_tx_ba_session-DE253569", signature);
+
+  const char kPCButNoStack[] =
+      "<0>[ 6066.829029] EIP: [<b82d7c15>] ieee80211_stop_tx_ba_session+";
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kPCButNoStack, &signature, false));
+  EXPECT_EQ("kernel-ieee80211_stop_tx_ba_session-00000000", signature);
+
+  const char kStackButNoPC[] =
+      "<4>[ 6066.829029]  [<790340af>] __do_softirq+0xa6/0x143\n";
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kStackButNoPC, &signature, false));
+  EXPECT_EQ("kernel--83615F0A", signature);
+
+  const char kMissingEverything[] =
+      "<4>[ 6066.829029]  [<790340af>] ? __do_softirq+0xa6/0x143\n";
+  EXPECT_FALSE(
+      collector_.ComputeKernelStackSignature(kMissingEverything,
+                                             &signature,
+                                             false));
+
+  const char kBreakmeBug[] =
+      "<4>[  180.492137]  [<790970c6>] ? handle_mm_fault+0x67f/0x96d\n"
+      "<4>[  180.492137]  [<790dcdfe>] ? proc_reg_write+0x5f/0x73\n"
+      "<4>[  180.492137]  [<790e2224>] ? write_breakme+0x0/0x108\n"
+      "<4>[  180.492137]  [<790dcd9f>] ? proc_reg_write+0x0/0x73\n"
+      "<4>[  180.492137]  [<790ac0aa>] vfs_write+0x85/0xe4\n"
+      "<0>[  180.492137] Code: c6 44 05 b2 00 89 d8 e8 0c ef 09 00 85 c0 75 "
+      "0b c7 00 00 00 00 00 e9 8e 00 00 00 ba e6 75 4b 79 89 d8 e8 f1 ee 09 "
+      "00 85 c0 75 04 <0f> 0b eb fe ba 58 47 49 79 89 d8 e8 dd ee 09 00 85 "
+      "c0 75 0a 68\n"
+      "<0>[  180.492137] EIP: [<790e22a4>] write_breakme+0x80/0x108 SS:ESP "
+          "0068:aa3e9efc\n"
+      "<4>[  180.501800] ---[ end trace 2a6b72965e1b1523 ]---\n"
+      "<0>[  180.502026] Kernel panic - not syncing: Fatal exception\n"
+      "<4>[  180.502026] Call Trace:\n"
+      "<4>[  180.502806]  [<79379aba>] ? printk+0x14/0x1a\n"
+      "<4>[  180.503033]  [<79379a00>] panic+0x3e/0xe4\n"
+      "<4>[  180.503287]  [<7937c005>] oops_end+0x73/0x81\n"
+      "<4>[  180.503520]  [<790055dd>] die+0x58/0x5e\n"
+      "<4>[  180.503538]  [<7937b96c>] do_trap+0x8e/0xa7\n"
+      "<4>[  180.503555]  [<79003d70>] ? do_invalid_op+0x0/0x80\n";
+
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kBreakmeBug, &signature, false));
+  EXPECT_EQ("kernel-write_breakme-122AB3CD", signature);
+
+  const char kPCLineTooOld[] =
+      "<4>[  174.492137]  [<790970c6>] ignored_function+0x67f/0x96d\n"
+      "<4>[  175.492137]  [<790970c6>] ignored_function2+0x67f/0x96d\n"
+      "<0>[  174.492137] EIP: [<790e22a4>] write_breakme+0x80/0x108 SS:ESP "
+          "0068:aa3e9efc\n"
+      "<4>[  180.501800] ---[ end trace 2a6b72965e1b1523 ]---\n"
+      "<4>[  180.502026] Call Trace:\n"
+      "<0>[  180.502026] Kernel panic - not syncing: Fatal exception\n"
+      "<4>[  180.502806]  [<79379aba>] printk+0x14/0x1a\n";
+
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kPCLineTooOld, &signature, false));
+  EXPECT_EQ("kernel-Fatal exception-ED4C84FE", signature);
+
+  // Panic without EIP line.
+  const char kExamplePanicOnly[] =
+      "<0>[   87.485611] Kernel panic - not syncing: Testing panic\n"
+      "<4>[   87.485630] Pid: 2825, comm: bash Tainted: G         "
+          "C 2.6.32.23+drm33.10 #1\n"
+      "<4>[   87.485639] Call Trace:\n"
+      "<4>[   87.485660]  [<8133f71d>] ? printk+0x14/0x17\n"
+      "<4>[   87.485674]  [<8133f663>] panic+0x3e/0xe4\n"
+      "<4>[   87.485689]  [<810d062e>] write_breakme+0xaa/0x124\n";
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kExamplePanicOnly,
+                                             &signature,
+                                             false));
+  EXPECT_EQ("kernel-Testing panic-E0FC3552", signature);
+
+  // Long message.
+  const char kTruncatedMessage[] =
+      "<0>[   87.485611] Kernel panic - not syncing: 01234567890123456789"
+          "01234567890123456789X\n";
+  EXPECT_TRUE(
+      collector_.ComputeKernelStackSignature(kTruncatedMessage,
+                                             &signature,
+                                             false));
+  EXPECT_EQ("kernel-0123456789012345678901234567890123456789-00000000",
+            signature);
+
+}
+
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index bc414ab..8d99b47 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -332,6 +332,8 @@
                              container_dir)) {  // temporary directory
     // Note we leave the container directory for inspection.
     conversion_result = false;
+  } else {
+    file_util::Delete(container_dir, true);
   }
 
   WriteCrashMetaData(
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 9dc0d60..db707f5 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -42,8 +42,6 @@
     pid_ = getpid();
   }
  protected:
-  void TestEnableOK(bool generate_diagnostics);
-
   void ExpectFileEquals(const char *golden,
                         const char *file_path) {
     std::string contents;