Add ARM support

kernel_collector.cc

 // Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "crash-reporter/kernel_collector.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "crash-reporter/system_logging.h"
@@ skipping 74 matching lines @@
     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:$");
+  pcrecpp::RE stack_trace_start_re(kTimestampRegex +
+        " (Call Trace|Backtrace):$");
   // Match lines such as the following and grab out "error_code".
-  // <4>[ 6066.849504]  [<7937bcee>] error_code+0x66/0x6c
+  // <4>[ 6066.849504]  [<7937bcee>] ? error_code+0x66/0x6c
+  // The ? may or may not be present
+
+  // For ARM:
+  // <4>[ 3498.731164] [<c0057220>] (__bug+0x20/0x2c) from [<c018062c>]
+  // (write_breakme+0xdc/0x1bc)
   pcrecpp::RE stack_entry_re(kTimestampRegex +
-                             "  \\[<.*>\\]([\\s\\?]+)([^\\+ ]+)");
+    "\\s+\\[<[[:xdigit:]]+>\\]"      // Matches "  [<7937bcee>]"

[kmixter1, 2011/03/07 23:16:58]

This went from hard to understand to really opaque. :)

Adding more to the regular expression for every new kind of architecture could
get really hard to maintain.  Could we initialize kernel collector with an
architecture code and have it pick dynamically which regexps to use based on
that?  This way we can keep the regexps simple and still allow us to test and
compile for all architecture at once.

[sjg, 2011/03/15 19:39:35]

My schange here were:

a. to allow 1 or 2 spaces at the start, instead of requiring 2.

b. Change .* to be more specific, so it matches hex digits (which I argue is
sensible anyway)

The intent in the kernel is that the format is the same for each arch, but I
would argue that the original expression here was too specific. I don't think we
want to have different specs for different archs. Anyway, to handle ARM we would
need to change the code also, since the first line includes a 'Stack backtrace;'
string too!

[kmixter1, 2011/03/17 16:53:30]

Yeah, I didn't closely enough - it was already opaque. :)  It's fine - thanks
for adding the comments as well.

+    "([\\s\\?(]+)"                   // Matches " ? ("
+    "([^\\+ )]+)");                  // Matches until \ + space )
   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)) {
@@ skipping 27 matching lines @@
     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: \\[<.*>\\] ([^\\+ ]+).*",
+  std::string reg;
+  pcrecpp::RE eip_re(kTimestampRegex + " (EIP: \\[<.*>\\]|PC is at) ([^\\+ ]+).*",

[kmixter1, 2011/03/07 23:16:58]

Same comment here.

[sjg, 2011/03/15 19:39:35]

I think this is fair enough, and the kernel will not like start calling EIP the
PC on x86 :-)

Do you know where I can find out the current architecture, preferably in the
cros lib?

[kmixter1, 2011/03/17 16:53:30]

You can't rely on libcros/Chrome since it's not necessarily running.  I didn't
see anything in libbase or libchromeos to do it.  You might check src/common or
consider adding it there.  If you don't want to, you can add it in
crash_collector.cc.  You might either want to use uname(2) to get a machine
string and convert that to an enumerator, or you could just rely on the
compiler:

#if defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
return X86;
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARM_FAMILY)
return ARM;

                      pcrecpp::MULTILINE());
   float timestamp = 0;
-  while (eip_re.FindAndConsume(&kernel_dump, &timestamp, crashing_function)) {
+  while (eip_re.FindAndConsume(&kernel_dump, &timestamp, &reg, 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");
     }
@@ skipping 137 matching lines @@
 
     logger_->LogInfo("Stored kcrash to %s",
                      kernel_crash_path.value().c_str());
   }
   if (!ClearPreservedDump()) {
     return false;
   }
 
   return true;
 }