Closed
Bug 1424304
Opened 7 years ago
Closed 7 years ago
Crash reporting is broken on Fennec
Categories
(Toolkit :: Crash Reporting, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox57 | --- | unaffected |
firefox58 | --- | fixed |
firefox59 | --- | fixed |
People
(Reporter: gsvelto, Assigned: gsvelto)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
ted
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
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 hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 3•7 years ago
|
||
(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.
Assignee | ||
Comment 4•7 years ago
|
||
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
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gsvelto
Assignee | ||
Comment 7•7 years ago
|
||
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.
Assignee | ||
Comment 10•7 years ago
|
||
(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.
Comment 11•7 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•