1356392 - Use release assert in EnumSerializer::Write

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Andrew McCreight [:mccr8]]

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 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1356392 - Use a release assert in EnumSerializer::Write.

Review commit: https://reviewboard.mozilla.org/r/130092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130092/

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

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

https://reviewboard.mozilla.org/r/130092/#review132700

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31430ca292f2
Use a release assert in EnumSerializer::Write. r=kanru

Comment 5

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/31430ca292f2

Comment 6

[Andrew McCreight [:mccr8]]

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

Comment 7

[Gerry Chang [:gchang]]

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.

Comment 8

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-beta/rev/52424eda8c08

Comment 9

[Andrei Vaida [:avaida]]

(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.