feat(minidump): Get CFI info from eh_frame even if err in debug_frame

[calixteman]

In case of error in .debug_frame section the CFI info in .eh_frame weren't retrieved.

[jan-auer]

Thank you! Patch looks good, thanks for ensuring that the error is eventually returned.

However, are you sure that the caller can swallow the error and still use the partially written CFI? Generally, the writer can fail at places that may leave invalid or partial records behind, which may lead to errors when feeding them into Breakpad.

I was briefly thinking about mentioning this in the description of the process method. Since it's quite uncertain that the CFI is still valid, however, I am now leaning towards not doing that. What do you think?

[calixteman]

Thank you! Patch looks good, thanks for ensuring that the error is eventually returned.

However, are you sure that the caller can swallow the error and still use the partially written CFI? Generally, the writer can fail at places that may leave invalid or partial records behind, which may lead to errors when feeding them into Breakpad.

I'd say that's ok.
Most of the CFI for firefox at least are in the eh_frame section (the same gcc & clang).
So in this case skipping CFI from debug_frame section is not bad.
The idea here is just to let the user decides what he wants to do: skip everything in case of error or keep what he has.
FYI, I fixed a bug in gimli which was causing trouble because of a "bad" debug_frame section:
gimli-rs/gimli@53c802a

I was briefly thinking about mentioning this in the description of the process method. Since it's quite uncertain that the CFI is still valid, however, I am now leaning towards not doing that. What do you think?

From my pov, CFI here is valid but incomplete so not so bad.

[jan-auer]

FYI, I fixed a bug in gimli which was causing trouble

Thanks, I'll bump gimli as soon as it is released.

CFI here is valid but incomplete so not so bad

Okay, I'm merging this now. From the interface, I still think that if we error at the wrong point, we might have emitted only a partial record (i.e. stopped in the middle of printing a register name). In that case, you'd be left with invalid CFI.

As you said, based on the error, the user can then decide if they still want to use the written CFI or not. 👍