If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

error: static_assert failed "DOM size changed" with ASan builds

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM
--
critical
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

(Assignee)

Description

4 months ago
I'm seeing the following error on ASan builds locally and the fuzzing build server:

> In file included from objdir/dom/base/Unified_cpp_dom_base1.cpp:101:
> dom/base/Element.cpp:183:1: error: static_assert failed "DOM size changed"
> ASSERT_ELEMENT_SIZE(Element, 120);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> dom/base/Element.cpp:178:3: note: expanded from macro 'ASSERT_ELEMENT_SIZE'
>   static_assert(sizeof(void*) != 8 || a == b, "DOM size changed"); \
>   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> dom/base/Element.cpp:183:1: note: in instantiation of template
>     class 'CheckElementSize<120, 128>' requested here

After investigating this with Bobby, we found that the check to decide about EXTRA_DOM_ELEMENT_BYTES in dom/base/Element.cpp isn't correct:

http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/dom/base/Element.cpp#169

The correct check is MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED and that is influenced also by a combination of disabled profiling and nightly builds in such a way that only the ASan builds we have on TC seem to fulfill this. So this bug isn't related to ASan itself. Patch coming after try runs.
(Assignee)

Comment 1

4 months ago
Green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a40b9cddf6ccb2584aa3d817f481432312aad1cf
Comment hidden (mozreview-request)

Comment 3

4 months ago
mozreview-review
Comment on attachment 8869111 [details]
Bug 1365954 - Fix check for EXTRA_DOM_ELEMENT_BYTES.

https://reviewboard.mozilla.org/r/140740/#review144608

r=me with that fixed.

::: dom/base/Element.cpp:167
(Diff revision 1)
>  // We need different numbers on debug and opt to deal with the owning thread
>  // pointer that comes with the non-threadsafe refcount on FragmentOrElement.

This comment is out of date, please update it.
Attachment #8869111 - Flags: review?(bobbyholley) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 5

4 months ago
mozreview-review
Comment on attachment 8869111 [details]
Bug 1365954 - Fix check for EXTRA_DOM_ELEMENT_BYTES.

https://reviewboard.mozilla.org/r/140740/#review144610

Keeping r+ by last comment
Attachment #8869111 - Flags: review+

Comment 6

4 months ago
Pushed by choller@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33ef54dce8af
Fix check for EXTRA_DOM_ELEMENT_BYTES. r=bholley

Comment 7

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/33ef54dce8af
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Duplicate of this bug: 1366221
You need to log in before you can comment on or make changes to this bug.