Statically allocated classinfo objects are not properly aligned

RESOLVED FIXED in Firefox 24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla25
ARM
Linux
Points:
---

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

Changeset 2ad039920033 added k${class}ClassInfoDataPlace buffers that are used as static allocation location for GenericClassInfo, but no effort is made to make the alignment correct. This causes unaligned read accesses on e.g. arm. I'm actually surprised this doesn't break our armv6 builds. It does, however, break Debian armel.
Hardware: x86_64 → ARM
Assignee: nobody → mh+mozilla
Attachment #782332 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/7506c6ee2e8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment on attachment 782332 [details] [diff] [review]
Properly align statically allocated classinfo objects

[Approval Request Comment]
User impact if declined: Linux ARM builds don't work, and I suspect this also affects sparc and other architectures that don't like misaligned word accesses. It would be nice if this could land in the next ESR for linux distros.
Testing completed (on m-c, etc.): Landed on m-c a couple days ago.
Risk to taking this patch (and alternatives if risky): In practice, the patch only changes the alignment of a data buffer in the resulting binary. Risk of this breaking anything is really low.
String or IDL/UUID changes made by this patch: None.
Attachment #782332 - Flags: approval-mozilla-aurora?
Comment on attachment 782332 [details] [diff] [review]
Properly align statically allocated classinfo objects

low risk and useful for support of linux in the next ESR version, approving.
Attachment #782332 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assuming no QA needed here. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs QA.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.