Fix a couple of bugs where we generate incorrect minidump files on...

Fix a couple of bugs where we generate incorrect minidump files on
Linux (e.g. use the same stream id twice, truncate /proc files, ...)

Add support for writing out the current list of DSOs.

If we recognize a stackframe that was generated when the seccomp sandbox
intercepted a system call, clean up the stack in the crash dump. This
allows for proper backtraces of sandboxed applications.

Don't dump information about threads that don't have a valid stack
pointer. There isn't really anything we can dump in this case anyway, and
the data would be completely uninteresting.

BUG=none
TEST=trigger a crash dump in a sandbox'd process and verify the minidump with
minidump-2-core

[markus]

Please let me know, if this looks OK.

A lot of this code is specific to adding support for breakpad in the seccomp
sandbox. This is probably not super-important for anybody other than Chrome. But
I don't think it should cause any problems.

There is a remaining TODO for me to figure out what to do about the call to
tkill(). Sending signals is currently not allowed in the sandbox, and I don't
think I really want to add support for it.

I think, we can probably reset the signal disposition to SIGDFL and then return
from the signal handler. But not sure if that's something we want to do for the
stand-alone version of breakpad.

For now, I am playing it safe and I call _exit(), if tkill() didn't do anything.

[Lei Zhang (chromium)]

http://breakpad.appspot.com/150001/diff/1/2
File src/client/linux/handler/exception_handler.cc (right):

http://breakpad.appspot.com/150001/diff/1/2#newcode272
Line 272: tgkill(getpid(), syscall(__NR_gettid), sig); // TODO(markus): mask
signal and return to caller
nit: over 80 columns.

http://breakpad.appspot.com/150001/diff/1/4
File src/client/linux/minidump_writer/minidump_writer.cc (right):

http://breakpad.appspot.com/150001/diff/1/4#newcode476
Line 476: return false;
Do we really want to fail here or can we call NullifyDirectoryEntry() instead?

http://breakpad.appspot.com/150001/diff/1/4#newcode935
Line 935: if (entry->found && i)
Oh, good catch!

http://breakpad.appspot.com/150001/diff/1/4#newcode1021
Line 1021: static const unsigned kBufSize = 1024 - 2*sizeof(void*);
Do we want to have a upper limit on how much we can read in as a sanity check?
Maybe say, 64KB?

http://breakpad.appspot.com/150001/diff/1/5
File src/common/linux/memory.h (right):

http://breakpad.appspot.com/150001/diff/1/5#newcode152
Line 152: if (sz >= 0 && sz <= used_) {
isn't |sz| always >= 0 since it's unsigned?

[markus]

http://breakpad.appspot.com/150001/diff/1/4
File src/client/linux/minidump_writer/minidump_writer.cc (right):

http://breakpad.appspot.com/150001/diff/1/4#newcode476
Line 476: return false;
You are right. NullifyDirectoryEntry() is probably going to work just as well.
In practice, I would expect that the function only ever fails when we can't
write to disk, but no harm in have a more gentle error handling policy.

http://breakpad.appspot.com/150001/diff/1/4#newcode1021
Line 1021: static const unsigned kBufSize = 1024 - 2*sizeof(void*);
I'd prefer if we didn't have an upper limit. With binaries the size and
complexity of Chrome, it is quite conceivable that these files could grow large.
But if they do, then that probably means they have useful data. Partial data
(e.g. in the case of /proc/$PID/maps) is often pretty useless.

And I can't really picture a situation where these files would grow completely
unbounded.

But feel free to try and convince me otherwise. Maybe, I am missing something.

http://breakpad.appspot.com/150001/diff/1/5
File src/common/linux/memory.h (right):

http://breakpad.appspot.com/150001/diff/1/5#newcode152
Line 152: if (sz >= 0 && sz <= used_) {
Good call. I like to be explicit, as this is potentially dangerous to get wrong.
But a useless test is just asking for trouble the next time the compiler
changes. No need to get bombarded with pointless compiler warnings.

I just changed it into a comment. That should do just fine.

[Lei Zhang (chromium)]

LGTM. I'll commit both CLs after TGIF and then roll deps on the Chromium side.

Assuming the Chromium DEPS roll sticks,  you can then land the breakpad.gyp
change.

[kmixter]

I know this has already been landed, but a few comments.

Is the DSO_DEBUG stream only useful for gdb/minidump-2-core - in other words,
should stack traces through DSOs should already be walkable in the processor
without this?

http://breakpad.appspot.com/150001/diff/7001/8002
File src/client/linux/minidump_writer/linux_dumper.cc (right):

http://breakpad.appspot.com/150001/diff/7001/8002#newcode88
Line 88: #if defined(__i386)
won't this fail to compile on other architectures?

http://breakpad.appspot.com/150001/diff/7001/8003
File src/client/linux/minidump_writer/minidump_writer.cc (right):

http://breakpad.appspot.com/150001/diff/7001/8003#newcode393
Line 393: dumper_.CopyFromProcess(&dyn, crashing_tid_, _DYNAMIC+i++,
sizeof(dyn));
Why not just put ++i in the for loop?  _DYNAMIC points to .dynamic section,
right?  That section's virtual address varies by process, so this really assumes
the dumping process is the same or forked from the crashing process.  May be
good to comment on this assumption since most (all?) of the rest of the dumper
avoids it.  The alternative would be to find it in procfs (not sure where) or
read the sections in crashing process' the ELF file.