[css-grid] Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch)

RESOLVED FIXED in Firefox 53

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: truber, Assigned: mats)

Tracking

(Blocks: 1 bug, {assertion, testcase})

52 Branch
mozilla55
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 wontfix, firefox-esr52 wontfix, firefox53 fixed, firefox54 fixed, firefox55 fixed)

Details

Attachments

(3 attachments)

Created attachment 8850131 [details]
testcase.html

The attached testcase asserts in mozilla-central rev e03e0c60462.

Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch), at /home/worker/workspace/build/src/layout/generic/nsGridContainerFrame.cpp:474
#01: nsGridContainerFrame::Reflow at layout/generic/nsGridContainerFrame.cpp:6196
#02: nsBlockReflowContext::ReflowBlock at layout/generic/nsBlockReflowContext.cpp:307
#03: nsBlockFrame::ReflowBlockFrame at layout/generic/nsBlockFrame.cpp:3470
#04: nsBlockFrame::ReflowLine at layout/generic/nsBlockFrame.cpp:2831
#05: nsBlockFrame::ReflowDirtyLines at layout/generic/nsBlockFrame.cpp:2370
#06: nsBlockFrame::Reflow at layout/generic/nsBlockFrame.cpp:1250
#07: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:900
#08: nsCanvasFrame::Reflow at layout/generic/nsCanvasFrame.cpp:722
#09: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:900
#10: nsHTMLScrollFrame::ReflowScrolledFrame at layout/generic/nsGfxScrollFrame.cpp:556
#11: nsHTMLScrollFrame::ReflowContents at layout/generic/nsGfxScrollFrame.cpp:686
#12: nsHTMLScrollFrame::Reflow at layout/generic/nsGfxScrollFrame.cpp:1041
#13: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:943
#14: mozilla::ViewportFrame::Reflow at layout/generic/ViewportFrame.cpp:330
#15: mozilla::PresShell::DoReflow at layout/generic/ReflowOutput.h:282
Flags: in-testsuite?
(Assignee)

Updated

2 years ago
Assignee: nobody → mats
OS: Unspecified → All
Priority: -- → P2
Hardware: Unspecified → All
Summary: Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch) → [css-grid] Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch)
(Assignee)

Comment 1

2 years ago
I can't reproduce the bug in a Linux64 debug build from the exact same rev
as yours (which is currently m-c tip), using a fresh profile.
(Assignee)

Comment 2

2 years ago
Daniel, can you reproduce this assertion?
Flags: needinfo?(dholbert)
Comment hidden (obsolete)
Sorry, my clownshoes are showing -- I forgot that the build I tested in had "--disable-debug" in its mozconfig, so of course assertions were disabled.

I can reproduce this in a Linux64 debug build that I just downloaded from treeherder -- the "target.tar.bz2" tarball here:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=01125b5142e587fb8d8bd40c8dbaf6aaef71a9f9&filter-searchStr=linux%20x64%20debug%20tc%20B&selectedJob=85835007

(Direct link: https://queue.taskcluster.net/v1/task/MuZ5TB9zRb2g0XjQ-uSi2g/runs/0/artifacts/public/build/target.tar.bz2 )

Mats, if your tested local build is definitely a debug build & you still can't reproduce, maybe try that ^^ tarball as an extra sanity check?
Flags: needinfo?(jschwartzentruber) → needinfo?(mats)
Ah, the plot thickens -- my *locally-built* enable-debug/disable-optimize build does not reproduce the issue.

The main distinction between my just-tested local build & the downloaded tarball from comment 4 is "disable-optimize" (local bulid) vs "enable-optimize" (treeherder build -- technically unspecified, but this is the default).

So maybe this depends on some subtly-different initialization / out-of-order instructions that only happen in enable-optimize builds...?
(Assignee)

Comment 6

2 years ago
Thanks Daniel, I indeed have disable-optimize in my .mozconfig.
I'll rebuild with enable-optimize and see if that helps.
Flags: needinfo?(mats)
(Reporter)

Comment 7

2 years ago
Thanks Daniel. I only try taskcluster builds usually (mozilla-central.latest.firefox.linux64-debug). I didn't realize those are optimized.
(Assignee)

Comment 8

2 years ago
Nope, that build didn't reproduce it either.  Hmm, it might be a gcc/clang difference...
(Assignee)

Comment 9

2 years ago
Yep, now I can reproduce it.  gcc + enable-optimize did it.
(Assignee)

Comment 10

2 years ago
Created attachment 8850517 [details] [diff] [review]
fix

It's just a bogus assertion.
When mArray is used it contains all grid container children, including
placeholders.  aGridItemCount always excludes placeholders.
Attachment #8850517 - Flags: review?(dholbert)
(Assignee)

Comment 11

2 years ago
Created attachment 8850518 [details] [diff] [review]
crashtest
(Assignee)

Comment 12

2 years ago
(BTW, I'm amazed how much faster enable-optimize makes the build, even with enable-debug.
It feels like an order of magnitude or so.  It does make it less debuggable though,
with gdb frequently displaying <optimized out> when trying to print variables.
It seems worth investigating if adding -Og and/or -ggdb helps with that though...)
(In reply to Mats Palmgren (:mats) from comment #9)
> Yep, now I can reproduce it.  gcc + enable-optimize did it.

Glad to hear it. I can't reproduce with *clang* + enable-optimize, FWIW. (I left that building overnight to test in the morning, & just tried it.  But I guess that's just because clang & gcc perform different optimizations.
D'oh, I just now noticed the "CLANG_CRASH_BUG" exclusion-ifdef around the condition -- that definitely explains why I'm not hitting it. :)  (I'm using clang 3.8.1 on this machine, which satisfies the CLANG_CRASH_BUG condition.)
Comment on attachment 8850517 [details] [diff] [review]
fix

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

r=me
Attachment #8850517 - Flags: review?(dholbert) → review+
(Assignee)

Updated

2 years ago
Flags: in-testsuite? → in-testsuite+

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3670c556728b
https://hg.mozilla.org/mozilla-central/rev/18e9870067e4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I'm going to assume that this isn't worth backporting. Feel free to set things back to affected and make the requests if you feel otherwise, though.
Blocks: 1151204
status-firefox52: --- → wontfix
status-firefox53: --- → wontfix
status-firefox54: --- → wontfix
status-firefox-esr52: --- → wontfix
Version: Trunk → 52 Branch
I agree. It's a debug-only change, so there's no user benefit that we'd gain from uplifting this.
...though actually, this is in a piece of code that I want to move around over in Bug 812687 (and I'll want to uplift the patches over there).

So it would make for an easier/cleaner uplift over there, if we uplifted this bug's patches over here.

I think we don't need to bother with uplift approval over here, though -- this bug only changes the inside of a MOZ_ASSERT expression, so it's debug-only and hence NPOTB.
status-firefox53: wontfix → fixed
status-firefox54: wontfix → fixed
You need to log in before you can comment on or make changes to this bug.