Utility to generate minidump symbols for developer diagnostics and fix 64b elf errors

Utility to generate minidump symbols for developer diagnostics

BUG=4882, 4886

[kmixter1, 2010-08-04 18:22:38]

Originally sent this to Sosa, but since he's out, Darin can you take a look?

[petkov, 2010-08-04 18:57:05]

a bunch of nits...

http://codereview.chromium.org/2825054/diff/3001/4001
File generate_breakpad_symbols (right):

http://codereview.chromium.org/2825054/diff/3001/4001#newcode23
generate_breakpad_symbols:23: DEFINE_boolean dryrun ${FLAGS_FALSE} "Run without
actually uploading."
Option not needed?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode27
generate_breakpad_symbols:27: DEFINE_boolean yes ${FLAGS_FALSE} "Answer yes to
all prompts."
Unused parameter?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode37
generate_breakpad_symbols:37: function cleanup() {
Remove () for consistency?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode63
generate_breakpad_symbols:63: if file "${elf}" | grep -q "ELF 64-bit"; then
What happens if you run this on an ARM sysroot?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode65
generate_breakpad_symbols:65: if echo "${elf}" | grep -q /usr/lib64; then
Does

if [[ "${elf}" == */usr/lib64/* ]]; then

work?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode120
generate_breakpad_symbols:120: local
symbol_path="${SYSROOT}/usr/lib/debug/breakpad"
It might be nice to somehow print a message once where the generated symbol
files are located.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode134
generate_breakpad_symbols:134: local upload_url="$2"
It seems that the URL is ignored because the utility just dumps symbols -- here
and other places. Search and fix.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode162
generate_breakpad_symbols:162: function main() {
Remove () for consistency?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode184
generate_breakpad_symbols:184: ! process_file "${debug_file}" "${upload_url}"
What's the ! for?

http://codereview.chromium.org/2825054/diff/3001/4002
File upload_symbols (right):

http://codereview.chromium.org/2825054/diff/3001/4002#newcode58
upload_symbols:58: # Upload the current symbol file to given URL.
Update a given symbol file to given URL.

http://codereview.chromium.org/2825054/diff/3001/4002#newcode111
upload_symbols:111: if ! ./generate_breakpad_symbols --board=${FLAGS_board}
${verbosity}; then
Use dirname $0 rather than ./ ?

[kmixter1, 2010-08-05 01:16:40]

PTAL

http://codereview.chromium.org/2825054/diff/3001/4001
File generate_breakpad_symbols (right):

http://codereview.chromium.org/2825054/diff/3001/4001#newcode23
generate_breakpad_symbols:23: DEFINE_boolean dryrun ${FLAGS_FALSE} "Run without
actually uploading."
On 2010/08/04 18:57:06, petkov wrote:
> Option not needed?
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode27
generate_breakpad_symbols:27: DEFINE_boolean yes ${FLAGS_FALSE} "Answer yes to
all prompts."
On 2010/08/04 18:57:06, petkov wrote:
> Unused parameter?
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode37
generate_breakpad_symbols:37: function cleanup() {
On 2010/08/04 18:57:06, petkov wrote:
> Remove () for consistency?
> 

Added it elsewhere for consistency.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode63
generate_breakpad_symbols:63: if file "${elf}" | grep -q "ELF 64-bit"; then
On 2010/08/04 18:57:06, petkov wrote:
> What happens if you run this on an ARM sysroot?

It evals to false.  The output of file on an ARM executable is:

ELF 32-bit LSB executable, ARM, version 1 (SYSV), for GNU/Linux 2.6.16,
dynamically linked (uses shared libs), stripped

If you're asking about dump_syms, it works ok.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode65
generate_breakpad_symbols:65: if echo "${elf}" | grep -q /usr/lib64; then
On 2010/08/04 18:57:06, petkov wrote:
> Does
> 
> if [[ "${elf}" == */usr/lib64/* ]]; then
> 
> work?
> 

That's news to me.  You're such a bash pro.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode120
generate_breakpad_symbols:120: local
symbol_path="${SYSROOT}/usr/lib/debug/breakpad"
On 2010/08/04 18:57:06, petkov wrote:
> It might be nice to somehow print a message once where the generated symbol
> files are located.
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode134
generate_breakpad_symbols:134: local upload_url="$2"
On 2010/08/04 18:57:06, petkov wrote:
> It seems that the URL is ignored because the utility just dumps symbols --
here
> and other places. Search and fix.
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode162
generate_breakpad_symbols:162: function main() {
On 2010/08/04 18:57:06, petkov wrote:
> Remove () for consistency?
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4001#newcode184
generate_breakpad_symbols:184: ! process_file "${debug_file}" "${upload_url}"
On 2010/08/04 18:57:06, petkov wrote:
> What's the ! for?
> 

Ignores the return value.

http://codereview.chromium.org/2825054/diff/3001/4002
File upload_symbols (right):

http://codereview.chromium.org/2825054/diff/3001/4002#newcode58
upload_symbols:58: # Upload the current symbol file to given URL.
On 2010/08/04 18:57:06, petkov wrote:
> Update a given symbol file to given URL.
> 

Done.

http://codereview.chromium.org/2825054/diff/3001/4002#newcode111
upload_symbols:111: if ! ./generate_breakpad_symbols --board=${FLAGS_board}
${verbosity}; then
On 2010/08/04 18:57:06, petkov wrote:
> Use dirname $0 rather than ./ ?
> 

Done.

[petkov, 2010-08-05 15:55:45]

I think there's a new convention to prefix standalone script utilities with
cros_

LGTM, otherwise.

http://codereview.chromium.org/2825054/diff/3001/4001
File generate_breakpad_symbols (right):

http://codereview.chromium.org/2825054/diff/3001/4001#newcode65
generate_breakpad_symbols:65: if echo "${elf}" | grep -q /usr/lib64; then
On 2010/08/05 01:16:40, kmixter1 wrote:
> On 2010/08/04 18:57:06, petkov wrote:
> > Does
> > 
> > if [[ "${elf}" == */usr/lib64/* ]]; then
> > 
> > work?
> > 
> 
> That's news to me.  You're such a bash pro.

I hate bash -- so I guess it doesn't work?

http://codereview.chromium.org/2825054/diff/3001/4001#newcode184
generate_breakpad_symbols:184: ! process_file "${debug_file}" "${upload_url}"
On 2010/08/05 01:16:40, kmixter1 wrote:
> On 2010/08/04 18:57:06, petkov wrote:
> > What's the ! for?
> > 
> 
> Ignores the return value.

I thought it inverts the result. OK, if it ignores...