689178 - Remove crash-reporter hooks to externally provide library mappings

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Mike Hommey [:glandium]]

Currently, our custom Android linker feeds the crash reporter with the information it needs to find our ashmem-backed libraries. As we're going to move to dynamically loading libraries when they are needed, the full list won't be available when the crashreporter is initialized like it currently is.

However, the linker keeps all the necessary info in the DT_DEBUG info that is mainly used for debuggers. This info is currently kept in a MD_LINUX_DSO_DEBUG section of the minidump (which is currently not enabled on android builds), but this info could be used to fill the modules list.

Comment 1

[Mike Hommey [:glandium]]

This is the complete patch queue for breakpad:
https://breakpad.appspot.com/550002/
https://breakpad.appspot.com/560002/
https://breakpad.appspot.com/562002/
https://breakpad.appspot.com/575002/
https://breakpad.appspot.com/571003/
https://breakpad.appspot.com/578002/
https://breakpad.appspot.com/580002/
https://breakpad.appspot.com/581002/
https://breakpad.appspot.com/582002/
https://breakpad.appspot.com/573002/
https://breakpad.appspot.com/586002/
https://breakpad.appspot.com/587002/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Remove crash-reporter hooks to externally provide library mappings

This needs to land at the same time as bug 725231.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Disable elfhack workaround from bug 822584

With breakpad taking mappings from data provided by the linker, that workaround is not needed anymore.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 745865 [details] [diff] [review]
Remove crash-reporter hooks to externally provide library mappings

Review of attachment 745865 [details] [diff] [review]:
-----------------------------------------------------------------

Guess I should review your upstream patches!

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 745919 [details] [diff] [review]
Disable elfhack workaround from bug 822584

Review of attachment 745919 [details] [diff] [review]:
-----------------------------------------------------------------

Are you sure you're never going to need this again? I guess it's not a lot of code to put back if you do.

Comment 7

[Mike Hommey [:glandium]]

Review ping on upstream bugs:
https://breakpad.appspot.com/560002/
https://breakpad.appspot.com/562002/
https://breakpad.appspot.com/575002/
https://breakpad.appspot.com/571003/
https://breakpad.appspot.com/578002/
https://breakpad.appspot.com/580002/
https://breakpad.appspot.com/573002/
https://breakpad.appspot.com/586002/
https://breakpad.appspot.com/587002/

Comment 8

[(not currently active) Ted Mielczarek]

Can you give me the ordering of how they apply? I've reviewed a few of them but I can't ever remember what order they apply in, so I'm never sure if I can land them without reviewing others first. Sorry for the ridiculous delay.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)
> Can you give me the ordering of how they apply?

The exact order in which they appear in comment 7.

Comment 10

[Mike Hommey [:glandium]]

Attachment: Bug 689178 - Remove crash-reporter hooks to externally provide library mappings.

This is a rebase of a 7-year-old patch that was r=ted. The hooks are not
used as of bug 725231.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Bug 689178 - Remove remainder of elfhack workaround from bug 822584.

This is a rebase of a 7-year-old patch that was r=ted. The main part of
the patch was actually already removed in bug 1389598.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50c02f803046
Remove crash-reporter hooks to externally provide library mappings. r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/39a6155cb418
Remove remainder of elfhack workaround from bug 822584. r=froydnj

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/50c02f803046
https://hg.mozilla.org/mozilla-central/rev/39a6155cb418