Script to generate post mortem of all crashes on device

Script to generate post mortem of all crashes on device

BUG=4887
TEST=tested run_remote_tests and image_to_live still work.  Ran script with machine with no crashes, as well as on a machine with a slew of powerd CHECK(false)'s.

Change-Id: Iffb6571d30d99d876f41972f92a7149a716035ee

[petkov, 2010-08-29 07:47:38]

Mostly nits, one bug about module_count, I think. LGTM otherwise.

http://codereview.chromium.org/3276002/diff/8001/9001
File cros_show_stacks (right):

http://codereview.chromium.org/3276002/diff/8001/9001#newcode3
cros_show_stacks:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
2010

http://codereview.chromium.org/3276002/diff/8001/9001#newcode37
cros_show_stacks:37: # Clean up remove access and temp files.
typo: remote

http://codereview.chromium.org/3276002/diff/8001/9001#newcode43
cros_show_stacks:43: # Return kind of crash
Echos "kcrash" or "minidump"?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode44
cros_show_stacks:44: get_kind() {
function

http://codereview.chromium.org/3276002/diff/8001/9001#newcode72
cros_show_stacks:72: if [ ${modules} -gt 0 ]; then
Do you mean module_count?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode104
cros_show_stacks:104: remote_sh "ls -1 ${remote_crash_patterns}" 2>/dev/null
space after 2>

http://codereview.chromium.org/3276002/diff/8001/9001#newcode123
cros_show_stacks:123: echo "" > "${filesfrom}"
is this so >> works? or? maybe "touch" instead (and add a comment?).

http://codereview.chromium.org/3276002/diff/8001/9001#newcode139
cros_show_stacks:139: echo "" > "${modules_file}"
touch?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode141
cros_show_stacks:141: dump=$(remove_quotes "${dump}")
where's remove_quotes defined?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode145
cros_show_stacks:145: ${MINIDUMP_DUMP} "${dump}" 2>/dev/null | grep code_file |
\
space after 2>
shell style -- pipes on separate lines:
... \
  | ... \
  | ...

http://codereview.chromium.org/3276002/diff/8001/9001#newcode146
cros_show_stacks:146: cut -d'=' -f2 | sed -e 's/^ \"//; s/\"$//' >>
"${modules_file}"
I assume you can't use remove_quotes...

You could replace cut | sed with something along the lines:

sed s/.+= "(.+)"/\1/

(you may need a few escaping \)

http://codereview.chromium.org/3276002/diff/8001/9001#newcode159
cros_show_stacks:159: ${MINIDUMP_STACKWALK} "${dump}" "${FLAGS_breakpad_root}"
2>/dev/null
space after 2>

http://codereview.chromium.org/3276002/diff/8001/9001#newcode168
cros_show_stacks:168: main $@
main "$@"

http://codereview.chromium.org/3276002/diff/8001/9003
File remote_access.sh (right):

http://codereview.chromium.org/3276002/diff/8001/9003#newcode23
remote_access.sh:23: # Copies a list of $1 to $2 on
finish?

http://codereview.chromium.org/3276002/diff/8001/9003#newcode60
remote_access.sh:60: if [[ -n "${FLAGS_board}" ]]; then
[ -n ... ] && return  :-)

http://codereview.chromium.org/3276002/diff/8001/9003#newcode65
remote_access.sh:65: if [[ -z "${FLAGS_board}" ]]; then
[ -z ... ] && check_board

[sosa, 2010-08-29 16:20:18]

mostly nits here as well

http://codereview.chromium.org/3276002/diff/8001/9001
File cros_show_stacks (right):

http://codereview.chromium.org/3276002/diff/8001/9001#newcode43
cros_show_stacks:43: # Return kind of crash
period at EOL

http://codereview.chromium.org/3276002/diff/8001/9001#newcode59
cros_show_stacks:59: local text_file="/build/${FLAGS_board}/${module}"
 Why are they in the root level of the SYSROOT?  shouldn't it be in package? 
build/board/->packages<-/module?  If not can you put them in /build/board/etc?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode61
cros_show_stacks:61: if [ -f ${text_file} ] && [ -f ${debug_file} ]; then
quotes around variables

http://codereview.chromium.org/3276002/diff/8001/9001#newcode66
cros_show_stacks:66: warn "Some modules are missing debug information:"
Seems like you want to just print this at the end i.e. collect list of these and
print them all out at end so the output doesn't get all jumbled.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode72
cros_show_stacks:72: if [ ${modules} -gt 0 ]; then
or -n "${modules}"

http://codereview.chromium.org/3276002/diff/8001/9001#newcode80
cros_show_stacks:80: FLAGS "$@" || exit 1
usage instead of exit 1?

http://codereview.chromium.org/3276002/diff/8001/9001#newcode94
cros_show_stacks:94: # duplicate crashes (in case cryptohome is not mounted)
below.
Did will/cmasone change something?  I thought /home/chronos is always == to
/mnt/stateful_partition/home/chronos (same with user) as it is a bound mount
point.  It's only a matter of timing ... i.e. before login /home/chronos/user
points to something different than after.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode111
cros_show_stacks:111: if echo "${unique_crashes}" | grep -v -q ${crash_short};
then
quotes around crash_short

http://codereview.chromium.org/3276002/diff/8001/9001#newcode135
cros_show_stacks:135: [ -n "$FLAGS_board" ] ||  die "--board is required."
{} around var

http://codereview.chromium.org/3276002/diff/8001/9001#newcode142
cros_show_stacks:142: if [ $(get_kind "${dump}") == "minidump" ]; then
single =?

http://codereview.chromium.org/3276002/diff/8001/9003
File remote_access.sh (right):

http://codereview.chromium.org/3276002/diff/8001/9003#newcode64
remote_access.sh:64: FLAGS_board=$(echo "${REMOTE_OUT}" | cut -d= -f2)
spaces between -d = and -f 2

[kmixter1, 2010-08-31 02:07:25]

PTAL

http://codereview.chromium.org/3276002/diff/8001/9001
File cros_show_stacks (right):

http://codereview.chromium.org/3276002/diff/8001/9001#newcode3
cros_show_stacks:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
On 2010/08/29 07:47:38, petkov wrote:
> 2010
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode37
cros_show_stacks:37: # Clean up remove access and temp files.
On 2010/08/29 07:47:38, petkov wrote:
> typo: remote
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode43
cros_show_stacks:43: # Return kind of crash
On 2010/08/29 07:47:38, petkov wrote:
> Echos "kcrash" or "minidump"?
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode43
cros_show_stacks:43: # Return kind of crash
On 2010/08/29 16:20:18, sosa wrote:
> period at EOL

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode44
cros_show_stacks:44: get_kind() {
On 2010/08/29 07:47:38, petkov wrote:
> function
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode59
cros_show_stacks:59: local text_file="/build/${FLAGS_board}/${module}"
On 2010/08/29 16:20:18, sosa wrote:
>  Why are they in the root level of the SYSROOT?  shouldn't it be in package? 
> build/board/->packages<-/module?  If not can you put them in /build/board/etc?


A module in this case is a shared obj or exec.  Like /bin/dd or
/usr/lib/dbus.so.  Text in this case refers to code.  Tried to doc better.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode61
cros_show_stacks:61: if [ -f ${text_file} ] && [ -f ${debug_file} ]; then
On 2010/08/29 16:20:18, sosa wrote:
> quotes around variables

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode66
cros_show_stacks:66: warn "Some modules are missing debug information:"
On 2010/08/29 16:20:18, sosa wrote:
> Seems like you want to just print this at the end i.e. collect list of these
and
> print them all out at end so the output doesn't get all jumbled.

The list is included below this line in output and generated by the warn 2 lines
lower.  Could put the list of missing modules in a variable but this seemed
simpler.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode72
cros_show_stacks:72: if [ ${modules} -gt 0 ]; then
On 2010/08/29 07:47:38, petkov wrote:
> Do you mean module_count?
> 

I did. Funny thing is this worked.  Fixed.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode80
cros_show_stacks:80: FLAGS "$@" || exit 1
On 2010/08/29 16:20:18, sosa wrote:
> usage instead of exit 1?

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode94
cros_show_stacks:94: # duplicate crashes (in case cryptohome is not mounted)
below.
On 2010/08/29 16:20:18, sosa wrote:
> Did will/cmasone change something?  I thought /home/chronos is always == to
> /mnt/stateful_partition/home/chronos (same with user) as it is a bound mount
> point.  It's only a matter of timing ... i.e. before login /home/chronos/user
> points to something different than after.

I didn't test this when logged in with a crash from before logging in, so you
might be right.  Prefer to leave as is because we handle it correctly if ever
there is a way to see the original non-mounted directory.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode104
cros_show_stacks:104: remote_sh "ls -1 ${remote_crash_patterns}" 2>/dev/null
On 2010/08/29 07:47:38, petkov wrote:
> space after 2>
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode111
cros_show_stacks:111: if echo "${unique_crashes}" | grep -v -q ${crash_short};
then
On 2010/08/29 16:20:18, sosa wrote:
> quotes around crash_short

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode123
cros_show_stacks:123: echo "" > "${filesfrom}"
On 2010/08/29 07:47:38, petkov wrote:
> is this so >> works? or? maybe "touch" instead (and add a comment?).
> 

Don't think it's necessary - removed.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode135
cros_show_stacks:135: [ -n "$FLAGS_board" ] ||  die "--board is required."
On 2010/08/29 16:20:18, sosa wrote:
> {} around var

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode139
cros_show_stacks:139: echo "" > "${modules_file}"
On 2010/08/29 07:47:38, petkov wrote:
> touch?
> 

Removed.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode141
cros_show_stacks:141: dump=$(remove_quotes "${dump}")
On 2010/08/29 07:47:38, petkov wrote:
> where's remove_quotes defined?
> 

common.sh

http://codereview.chromium.org/3276002/diff/8001/9001#newcode142
cros_show_stacks:142: if [ $(get_kind "${dump}") == "minidump" ]; then
On 2010/08/29 16:20:18, sosa wrote:
> single =?

They're equivalent though I guess = is more portable.  Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode145
cros_show_stacks:145: ${MINIDUMP_DUMP} "${dump}" 2>/dev/null | grep code_file |
\
On 2010/08/29 07:47:38, petkov wrote:
> space after 2>
> shell style -- pipes on separate lines:
> ... \
>   | ... \
>   | ...
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode146
cros_show_stacks:146: cut -d'=' -f2 | sed -e 's/^ \"//; s/\"$//' >>
"${modules_file}"
On 2010/08/29 07:47:38, petkov wrote:
> I assume you can't use remove_quotes...
> 
> You could replace cut | sed with something along the lines:
> 
> sed s/.+= "(.+)"/\1/
> 
> (you may need a few escaping \)
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode159
cros_show_stacks:159: ${MINIDUMP_STACKWALK} "${dump}" "${FLAGS_breakpad_root}"
2>/dev/null
On 2010/08/29 07:47:38, petkov wrote:
> space after 2>
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9001#newcode168
cros_show_stacks:168: main $@
On 2010/08/29 07:47:38, petkov wrote:
> main "$@"
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9003
File remote_access.sh (right):

http://codereview.chromium.org/3276002/diff/8001/9003#newcode23
remote_access.sh:23: # Copies a list of $1 to $2 on
On 2010/08/29 07:47:38, petkov wrote:
> finish?
> 

Done.

http://codereview.chromium.org/3276002/diff/8001/9003#newcode60
remote_access.sh:60: if [[ -n "${FLAGS_board}" ]]; then
On 2010/08/29 07:47:38, petkov wrote:
> [ -n ... ] && return  :-)
> 

I still don't see this as more readable, but don't feel strongly about it.

http://codereview.chromium.org/3276002/diff/8001/9003#newcode64
remote_access.sh:64: FLAGS_board=$(echo "${REMOTE_OUT}" | cut -d= -f2)
On 2010/08/29 16:20:18, sosa wrote:
> spaces between -d = and -f 2

Done.

http://codereview.chromium.org/3276002/diff/8001/9003#newcode65
remote_access.sh:65: if [[ -z "${FLAGS_board}" ]]; then
On 2010/08/29 07:47:38, petkov wrote:
> [ -z ... ] && check_board
> 

check_board is in autotest_lib.sh, so I couldn't use it here anyway.  Wrote it
out instead.

[petkov, 2010-08-31 05:23:50]

LGTM

You may want to get this in as is and refactor later, if needed.

http://codereview.chromium.org/3276002/diff/5005/15001
File cros_show_stacks (right):

http://codereview.chromium.org/3276002/diff/5005/15001#newcode61
cros_show_stacks:61: for module in $(cat ${modules_file} | sort | uniq); do
no need for cat:

sort ${modules_file} | uniq

http://codereview.chromium.org/3276002/diff/5005/15001#newcode82
cros_show_stacks:82: function main() {
this routine feels a bit long -- consider breaking it up.