Closed Bug 1424304 Opened 7 years ago Closed 7 years ago

Crash reporting is broken on Fennec

Categories

(Toolkit :: Crash Reporting, defect, P1)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- unaffected
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: gsvelto, Assigned: gsvelto)

References

Details

Attachments

(1 file)

This has been the case for over a month, since bug 1402153 landed. When a crash occurs the minidump is generated correctly however breakpad does not call the MinidumpCallback which in turn prevents the crash reporter activity to be launched. The patch in bug 1402153 did not change the code in any way or form save for making some non-public functions static and apparently, if either copy_file() or MinidumpCallback() are declared static then the issue presents itself. I didn't investigate further but since the same functions work just fine on Linux I smell a compiler bug in Fennec.
Comment on attachment 8935819 [details] Bug 1424304 - Workaround to allow the crashreporter to launch correctly on Fennec; .mielczarek https://reviewboard.mozilla.org/r/206686/#review212522 It's unfortunate that we don't have any automated testing that would have caught this. :-(
Attachment #8935819 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2) > It's unfortunate that we don't have any automated testing that would have > caught this. :-( Yeah, with bug 379290 we'll be able to write pseudo-integration tests for the desktop crash reporter at least. For Fennec we can probably think of something similar.
Updating the bug to reflect the fact that it was the introduction of clang that caused this weird behavior, the exception-handler code was fine.
Depends on: 1163171
Pushed by gsvelto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/60bf585f638c Workaround to allow the crashreporter to launch correctly on Fennec; r=ted.mielczarek
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee: nobody → gsvelto
Comment on attachment 8935819 [details] Bug 1424304 - Workaround to allow the crashreporter to launch correctly on Fennec; .mielczarek Approval Request Comment [Feature/Bug causing the regression]: Bug 1163171 [User impact if declined]: The crash reporter does not come up when Fennec crashes [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Yes, launch Fennec, force it to crash using `kill -ABRT` and check that the crash reporter is shown [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: This affects only two line of codes that change the visibility of the exception handler functions, it contains no functional changes. This seems sufficient to work around the issue. [String changes made/needed]: None
Attachment #8935819 - Flags: approval-mozilla-beta?
Comment on attachment 8935819 [details] Bug 1424304 - Workaround to allow the crashreporter to launch correctly on Fennec; .mielczarek We definitely want Fennec crash data. Please uplift for 58.0b12!
Attachment #8935819 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Note, we're still getting plenty of crash data for fennec 58, so this must not be broken for all cases.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #9) > Note, we're still getting plenty of crash data for fennec 58, so this must > not be broken for all cases. It depends on the build, if we're not building 58 with clang then it should be fine. However when I tested recent betas on my phone the crashreporter wouldn't come up which lead me to believe that the build was fully affected. It's possible that this is an intermittent issue and I was just unlucky enough to see it all the time.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: