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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Christian Holler (:decoder)]

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.

Comment 1

[Christian Holler (:decoder)]

Green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a40b9cddf6ccb2584aa3d817f481432312aad1cf

Comment 2

[Christian Holler (:decoder)]

Attachment: Bug 1365954 - Fix check for EXTRA_DOM_ELEMENT_BYTES.

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

Comment 3

[Bobby Holley (:bholley)]

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.

Comment 4

[Christian Holler (:decoder)]

Comment on attachment 8869111 [details]
Bug 1365954 - Fix check for EXTRA_DOM_ELEMENT_BYTES.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140740/diff/1-2/

Comment 5

[Christian Holler (:decoder)]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33ef54dce8af
Fix check for EXTRA_DOM_ELEMENT_BYTES. r=bholley

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/33ef54dce8af