Ignore broken units in Context<R>::from_sections.

[mstange]

Fixes #132. r? @philipc

I don't know if I'm using the right words in the commit message, how to find out what the type of the "broken" units is or what this means about the validity of the DWARF data. I also don't know how to slim the testcase down into something that could become a real test.

[coveralls]

Coverage decreased (-0.1%) to 73.591% when pulling 3958c70 on mstange:ignore-broken-units into 988f7a8 on gimli-rs:master.

[philipc]

how to find out what the type of the "broken" units is or what this means about the validity of the DWARF data

Do you mean how to find out the manner in which they are broken?

From the debug output you gave in the gimli issue, we can see that the length of the unit is 7 bytes. This is enough to contain the DWARF version number, the .debug_abbrev offset, and the address size. There are 0 bytes of entries (we could do a better job in the Debug implementation for EndianReader to make this clearer). This is unusual, and I can't see any point for it existing, but we can easily ignore it, so we should.

I think this PR is fine. Another option would be to only ignore units that match this specific instance of brokenness (Error::MissingUnitDie), but I don't know which approach is best.

For example, we could also easily fail parsing if the DWARF actually was corrupted, and if that happened do users prefer to find out the DWARF is corrupted, or do they prefer a best-effort answer? I think ideally the addr2line utility would print a warning about corruption and continue on, but the crate API doesn't allow that currently.

[philipc]

Thanks!

[mstange]

how to find out what the type of the "broken" units is or what this means about the validity of the DWARF data

Do you mean how to find out the manner in which they are broken?

Yes, and whether it's "valid" brokenness, and if not, if this means that there's a bug in dsymutil or in llvm.

From the debug output you gave in the gimli issue, we can see that the length of the unit is 7 bytes. This is enough to contain the DWARF version number, the .debug_abbrev offset, and the address size. There are 0 bytes of entries (we could do a better job in the Debug implementation for EndianReader to make this clearer). This is unusual, and I can't see any point for it existing, but we can easily ignore it, so we should.

I see, thanks!

For example, we could also easily fail parsing if the DWARF actually was corrupted, and if that happened do users prefer to find out the DWARF is corrupted, or do they prefer a best-effort answer? I think ideally the addr2line utility would print a warning about corruption and continue on, but the crate API doesn't allow that currently.

From my very limited viewpoint, I'm mostly interested in a best-effort answer. I'd only care about corruption errors if there was some certainty that the corruption was in fact responsible for inaccurate or incomplete answers. Then those errors might enable me to find bugs in other parts of the pipeline, and fixing those bugs might give me more accurate and more complete answers.