Closed Bug 1316649 Opened 9 years ago Closed 9 years ago

Null-deref in WritingModes::IsVertical with grid layout

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

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

People

(Reporter: truber, Assigned: MatsPalmgren_bugz)

References

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
Flags: needinfo?(mats)
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+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: