Closed Bug 1316649 Opened 3 years ago Closed 3 years ago

Null-deref in WritingModes::IsVertical with grid layout

Categories

(Core :: Layout, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed

People

(Reporter: truber, Assigned: mats)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(3 files)

Attached file testcase.html
Attached testcase cases a null dereference in WritingModes::IsVertical in m-c 058b48a72e50.

==23305==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f702837b10e bp 0x7ffd9ecf9e90 sp 0x7ffd9ecf9e30 T0)
    #0 0x7f702837b10d in IsVertical obj-firefox/dist/include/mozilla/WritingModes.h:241:39
    #1 0x7f702837b10d in ComputedISize obj-firefox/dist/include/mozilla/ReflowInput.h:407
    #2 0x7f702837b10d in ComputedSize obj-firefox/dist/include/mozilla/ReflowInput.h:442
    #3 0x7f702837b10d in mozilla::ReflowInput::ComputeContainingBlockRectangle(nsPresContext*, mozilla::ReflowInput const*) const layout/generic/ReflowInput.cpp:2043
    #4 0x7f702836eff8 in mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::LogicalSize const&, nsMargin const*, nsMargin const*, nsIAtom*) layout/generic/ReflowInput.cpp:2196:9
    #5 0x7f7028366f2c in mozilla::ReflowInput::Init(nsPresContext*, mozilla::LogicalSize const*, nsMargin const*, nsMargin const*) layout/generic/ReflowInput.cpp:399:3
    #6 0x7f70284f893f in MeasuringReflow(nsIFrame*, mozilla::ReflowInput const*, nsRenderingContext*, mozilla::LogicalSize const&, int, int) layout/generic/nsGridContainerFrame.cpp:3724:15
    #7 0x7f70284fcba1 in ContentContribution(nsGridContainerFrame::GridItemInfo const&, nsGridContainerFrame::GridReflowInput const&, nsRenderingContext*, mozilla::WritingMode, mozilla::LogicalAxis, nsLayoutUtils::IntrinsicISizeType, int, unsigned int) layout/generic/nsGridContainerFrame.cpp:3789:12
    #8 0x7f70284f579c in MinContentContribution layout/generic/nsGridContainerFrame.cpp:3843:15
    #9 0x7f70284f579c in nsGridContainerFrame::Tracks::ResolveIntrinsicSizeStep1(nsGridContainerFrame::GridReflowInput&, nsGridContainerFrame::TrackSizingFunctions const&, int, SizingConstraint, nsGridContainerFrame::LineRange const&, nsGridContainerFrame::GridItemInfo const&) layout/generic/nsGridContainerFrame.cpp:4003

Debug build hits the assertion:
###!!! ASSERTION: no containing block: 'nullptr != cbrs', file layout/generic/ReflowInput.cpp, line 2190
Good catch, Jesse.  I'm happy to see that someone is still doing DOM fuzz testing. :-)
Assignee: nobody → mats
Flags: needinfo?(mats)
I'm pretty sure this is a regression from bug 984869.
That made us skip the grid/flex/columnset frame here:
http://searchfox.org/mozilla-central/rev/846adaea6ccd1428780ed47d0d94da18478acdbb/layout/generic/nsFrame.cpp#6575,6589,6592
and return the ButtonControlFrame instead.
I think that's correct though - I don't see any reason why we shouldn't
do the same as a block wrapper there.
But, it makes us take the else-branch here:
http://searchfox.org/mozilla-central/rev/846adaea6ccd1428780ed47d0d94da18478acdbb/layout/generic/ReflowInput.cpp#489,498
and the GridContainer reflow code sometimes use a "dummy" parent ReflowInput
to do a "measuring reflow" for intrinsic sizing.  This dummy ReflowInput
has a null mCBReflowInput, so that's why we crash on a null pointer later.

I suspect flexbox could also crash, but if I change the testcase to use
display:flex I only get some assertions, not a crash.
Blocks: 984869, 1217086
Severity: normal → critical
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Attached patch fixSplinter Review
Attachment #8809636 - Flags: review?(dholbert)
Attached patch crashtestSplinter Review
(I added tests for Flexbox and ColumnSet layout, but I've commented out
the flexbox tests for now due to assertions.  I'll file a separate bug
about that.)
Comment on attachment 8809636 [details] [diff] [review]
fix

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

r=me
Attachment #8809636 - Flags: review?(dholbert) → review+
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/66639f197fa2
https://hg.mozilla.org/mozilla-central/rev/9d1b72a21a39
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.