Add ARM support

kernel_collector.cc

Index: kernel_collector.cc
diff --git a/kernel_collector.cc b/kernel_collector.cc
index 4fa51e26e7d2752e494defbca0f5e870323af38d..ad9fc3da1eaaa92041ff5ea14678154837ea35e5 100644
--- a/kernel_collector.cc
+++ b/kernel_collector.cc
@@ -92,11 +92,19 @@ void KernelCollector::ProcessStackTrace(
     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;
 
@@ -144,10 +152,11 @@ bool KernelCollector::FindCrashingFunction(
     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,