Use mozilla::AlignedStorage2 for IPDL union members

RESOLVED FIXED in Firefox 43

Status

()

Core
IPC
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jld, Assigned: jld)

Tracking

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Currently we're using char arrays to hold the contents of an IPDL union and reinterpret_cast'ing them in accessors.  This can misalign the contents, if they need more alignment than the enum used for the discriminant (likely on 64-bit platforms); that could cause crashes on strict-alignment platforms like sparc64, and performance loss on others (like x86_64).

Fortunately, we have mozilla::AlignedStorage2, which takes care of this.  And, as of bug 1187073, it's marked so that our static analysis propagates storage annotations through it; for an example of why this is important, see bug 1198979.
(Assignee)

Comment 1

2 years ago
(Taking this because I already have a patch.)
Assignee: nobody → jld
(Assignee)

Comment 2

2 years ago
Created attachment 8656883 [details] [diff] [review]
bug1201329-aligned2ipdl-hg0.diff
Attachment #8656883 - Flags: review?(wmccloskey)
Comment on attachment 8656883 [details] [diff] [review]
bug1201329-aligned2ipdl-hg0.diff

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

Thanks Jed. Sometimes it's nice to include an example of how the generated code changes (as a patch). This one's pretty simple though.
Attachment #8656883 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 4

2 years ago
Created attachment 8656960 [details] [diff] [review]
ipdl-example-change.diff

An example of the changes in the generated code, including some examples of recursive datatypes.  (No changes to generated .cpp files were observed.)
Attachment #8656960 - Flags: checkin-
(Assignee)

Comment 5

2 years ago
(Even though the patch is already reviewed (thanks!), the header diff might still be interesting for observers / the historical record.)

Comment 6

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3eef8cb5eb51
https://hg.mozilla.org/mozilla-central/rev/3eef8cb5eb51
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43

Updated

2 years ago
Duplicate of this bug: 1026499
You need to log in before you can comment on or make changes to this bug.