New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 648134 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

EC: Crash reporting

Project Member Reported by drinkcat@chromium.org, Sep 19 2016

Issue description

Now that we fixed  Issue 527904 , we have EC panicinfo in feedback reports.

However, it would be useful to automatically report EC panicinfo to the crash server, just like Chrome/kernel crashes.

As an example, it could allow us to tell the occurrence rate of https://code.google.com/p/chrome-os-partner/issues/detail?id=57544
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/1390071f758b056a686430d34f29cb2dfcd83917

commit 1390071f758b056a686430d34f29cb2dfcd83917
Author: Nicolas Boichat <drinkcat@google.com>
Date: Thu Sep 22 09:00:12 2016

crash: Report EC crashes

Recent kernels include EC panicinfo, let's report these crashes,
subject to the usual consent.

Like some kernel crashes (ramoops, watchdog resets) and unclean
shutdowns, the information is collected on boot.

Since the EC power cycle usually lasts much longer than the AP's,
we check a bit in the EC panicinfo data to see if the crash is
"fresh" (it will be 0 only on the first AP boot after an EC
crash).

BUG=chromium:648134
TEST=EC console: reboot; no crash in /var/spool/crash
TEST=EC console: crash assert; new crash in /var/spool/crash;
     reboot; no new crash => repeat a few times.
TEST=EC console: crash assert; /sbin/crash_sender; crash is
     displayed in chrome://crashes
TEST=cros_run_unit_tests --board elm --packages \
     chromeos-base/crash-reporter

Change-Id: Ib7afd8d048ab6bda844926e2007e6f8726704678
Reviewed-on: https://chromium-review.googlesource.com/388452
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/crash_reporter.cc
[add] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/ec_collector.cc
[add] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/ec_collector.h
[modify] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/crash-reporter.gyp
[add] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/ec_collector_test.cc
[modify] https://crrev.com/1390071f758b056a686430d34f29cb2dfcd83917/crash-reporter/crash_sender

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a6a158b23b7d732f0b32f7f3d93687860e9cc5c0

commit a6a158b23b7d732f0b32f7f3d93687860e9cc5c0
Author: Nicolas Boichat <drinkcat@google.com>
Date: Wed Oct 05 21:51:33 2016

crash: Add signature to EC crashes

Let's simply hash the panicinfo to obtain a somewhat unique
signature for the crash.

BUG=chromium:648134
TEST=EC console: crash assert; /sbin/crash_sender; crash is
     displayed in chrome://crashes; crash/ shows a signature

Change-Id: Ieecb2fde8d5308992b0e37823853a111b0de5c2e
Reviewed-on: https://chromium-review.googlesource.com/394086
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a6a158b23b7d732f0b32f7f3d93687860e9cc5c0/crash-reporter/ec_collector.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a3e6a4c59742a3855afc5590596a0c42ebe5a768

commit a3e6a4c59742a3855afc5590596a0c42ebe5a768
Author: Nicolas Boichat <drinkcat@google.com>
Date: Thu Oct 06 16:11:15 2016

crash: Move HashString function to CrashCollector class

This function can also be useful in the EC collector, let's move it
to the super-class.

BUG=chromium:648134
TEST=emerge-elm -av crash-reporter

Change-Id: Ib7ada7633501d79e147d68ed67c7b8b9c163a3ab
Reviewed-on: https://chromium-review.googlesource.com/394150
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a3e6a4c59742a3855afc5590596a0c42ebe5a768/crash-reporter/kernel_collector.h
[modify] https://crrev.com/a3e6a4c59742a3855afc5590596a0c42ebe5a768/crash-reporter/kernel_collector.cc
[modify] https://crrev.com/a3e6a4c59742a3855afc5590596a0c42ebe5a768/crash-reporter/crash_collector.cc
[modify] https://crrev.com/a3e6a4c59742a3855afc5590596a0c42ebe5a768/crash-reporter/crash_collector.h

Some more TODOs:
 1. Reporting kind of works: https://crash.corp.google.com/browse?stbtiq=6ddc219900000000#0 , but "Process type" is blank, which makes searching for EC crashes impossible... => Already fixed by crash server CL.
 2. It would be nice if the binary data was presented base64-encoded, so that one could more easily copy and paste and for analysis by ec_parse_panicinfo... (this processing could be done on the DUT, arguably).
 3. Include EC version in crash report.
Cc: ivanpe@chromium.org
Owner: drinkcat@chromium.org
Status: Assigned (was: Available)
For 2, I now see that the process of parsing large number of crashes on http://crash is quite slow, as the signatures for similar crashes are often quite different (so one needs to download the file, pipe it to ec_parse_panicinfo, and repeat for lots of crashes).

There are 2 ways to improve on this:
 a. Parse the panic data on DUT itself (ec_parse_panicinfo is a 10kb executable, so we could afford to ship it with every single image)
 b. Have the crash server do the parsing (ivanpe: is that easy/possible to do?)

Sign in to add a comment