search

rust-minidump / minidump-writer

Rust rewrite of breakpad's minidump_writer
MIT License
68 stars 17 forks source link

Minidumps without the main program mapping are created if the main program is missing a build id #122

Open afranchuk opened 5 months ago

afranchuk commented 5 months ago

As a result of https://bugzilla.mozilla.org/show_bug.cgi?id=1676109, any mappings without a build id are removed from the minidump. This behavior isn't great: if the main program doesn't have a build id, it will be removed, which invalidates assumptions in e.g. the minidump crate that the main module is the first mapping. I think at the time the assumption was that sane toolchains should always add build ids nowadays. I found this because the android linker does not (at least not when invoked by cargo)!

I would argue that the main module should never be removed. One could also argue that other modules shouldn't be removed either: the original bug had to do with saving space from "useless" mappings, however nowadays we do more with the mappings than just checking module versions/debug info. For instance, we use mappings to determine whether memory accesses would have been valid or not at crash time.

Furthermore, I think that the decision to remove such mappings should at least rest with the user of the crate, instead of being baked-in.

gabrielesvelto commented 5 months ago

Yes, that's a good idea, and I suppose we could tweak the stack walker to react properly to these mappings (e.g. refuse to unwind through a mapped file which is not executable). With the old stack walker we'd happily unwind through those mappings, yielding some absolutely garbage stacks.

We now also have the list of file descriptors in a stream, so we can certainly build more synergies between that and the non-executable file mappings.