Minidumps never contain MD_LINUX_DSO_DEBUG info when breakpad is in a shared library

Minidumps never contain MD_LINUX_DSO_DEBUG info when breakpad is in a shared
library

[glandium]

Note that the unittest actually passes without the patch, because it's an
executable, and as such, _DYNAMIC points to the right place... The only way to
actually test for the original bug would be to build the breakpad code as a
shared lib...

[glandium]

With the new iteration, the unittest exhibits the bug if the non-unittest and
build parts of the patch are not applied.

[Ted Mielczarek]

https://breakpad.appspot.com/422002/diff/9001/Makefile.am
File Makefile.am (right):

https://breakpad.appspot.com/422002/diff/9001/Makefile.am#newcode278
Makefile.am:278: src/client/linux/linux_client_unittest_shlib
Does this mean that "make check" is going to try to run both of these?

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.cc (right):

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.cc:139: LinuxDumper::ReadAuxv() {
Not sure why these functions are formatted like this, but Google style puts the
return value on the same line as the function name. Can you fix it while you're
here?

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.h (right):

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.h:152: const
wasteful_vector<elf_aux_val_t> &auxv() { return auxv_; }
Snuggle the & against the type name (I know the other examples above are the
opposite way).

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc (right):

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc:290: void*
linux_gate_loc = reinterpret_cast<void *>(dumper.auxv()[AT_SYSINFO_EHDR]);
Does this line still fit in 80 chars? (hard to tell here).

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/minidump_writer.cc (right):

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/minidump_writer.cc:916: ElfW(Phdr) *phdr =
reinterpret_cast<ElfW(Phdr) *>(dumper_->auxv()[AT_PHDR]);
ElfW(Phdr)* phdr

https://breakpad.appspot.com/422002/diff/9001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/minidump_writer.cc:917: char *base;
char* base

[Ted Mielczarek]

LGTM

http://breakpad.appspot.com/422002/diff/18001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.cc (right):

http://breakpad.appspot.com/422002/diff/18001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.cc:177: linux_gate_loc =
reinterpret_cast<void *>(auxv_[AT_SYSINFO_EHDR]);
Could just merge this up to the same line as the variable declaration.

[glandium]

http://breakpad.appspot.com/422002/diff/18001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.cc (right):

http://breakpad.appspot.com/422002/diff/18001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.cc:177: linux_gate_loc =
reinterpret_cast<void *>(auxv_[AT_SYSINFO_EHDR]);
On 2012/09/18 19:24:34, Ted Mielczarek wrote:
> Could just merge this up to the same line as the variable declaration.

That would make it > 80 characters. Is there a point making it a single
declaration, on two lines?

[Ted Mielczarek]

On 2012/09/18 20:39:56, glandium wrote:
> That would make it > 80 characters. Is there a point making it a single
> declaration, on two lines?

Google style lets you continue it on the next line indented 4 characters. I
think it's just slightly simpler than repeating the variable name in the
assignment. Pretty nitpicky, indeed.

[Ted Mielczarek]

This LGTM. I was a little worried that this might not work right with
LinuxCoreDumper, but it doesn't seem to be doing anything different than the
rest of the LinuxDumper implemenation in that regard.