Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Use release assert in EnumSerializer::Write

RESOLVED FIXED in Firefox 54

Status

()

Core
IPC
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
mozilla55
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 months ago
It seems slightly goofy to send a value we know will just kill the other process. This also seems like a decent indication that we're sending uninitialized values, which is bad. I can't imagine the overhead is too high.
Comment hidden (mozreview-request)

Comment 3

3 months ago
mozreview-review
Comment on attachment 8858134 [details]
Bug 1356392 - Use a release assert in EnumSerializer::Write.

https://reviewboard.mozilla.org/r/130092/#review132700
Attachment #8858134 - Flags: review?(kchen) → review+

Comment 4

3 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/31430ca292f2
Use a release assert in EnumSerializer::Write. r=kanru

Comment 5

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/31430ca292f2
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 6

3 months ago
Comment on attachment 8858134 [details]
Bug 1356392 - Use a release assert in EnumSerializer::Write.

Approval Request Comment
[Feature/Bug causing the regression]: none
[User impact if declined]: this will cause some crashes to be easier to analyze, and paper over some security issues
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes, it did not turn up any new issues.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it just runs a check we are already running earlier
[String changes made/needed]: none
Attachment #8858134 - Flags: approval-mozilla-beta?
(Assignee)

Updated

3 months ago
Depends on: 1357404

Updated

3 months ago
status-firefox54: --- → affected

Comment 7

3 months ago
Comment on attachment 8858134 [details]
Bug 1356392 - Use a release assert in EnumSerializer::Write.

Help debug crash issue. Beta54+. Should be in 54 beta 1.
Attachment #8858134 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 8

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/52424eda8c08
status-firefox54: affected → fixed
(In reply to Andrew McCreight [:mccr8] from comment #6)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: yes, it did not turn up any new
> issues.
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Andrew's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.