1474774 - Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Attempted to subtract [n - nscoord_MAX]), at src/obj-firefox/dist/include/nsCoord.h:208

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c:
BuildID=20180710230738
SourceStamp=3edc9c3ae818490ed36b8bfc8ffdfc9e222b41db

Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Attempted to subtract [n - nscoord_MAX]), at src/obj-firefox/dist/include/nsCoord.h:208

#0 NSCoordSaturatingSubtract(int, int, int) src/obj-firefox/dist/include/nsCoord.h:222:14
#1 nsIFrame::InlinePrefISizeData::ForceBreak(mozilla::StyleClear) src/layout/generic/nsFrame.cpp:5440:5
#2 nsBlockFrame::GetPrefISize(gfxContext*) src/layout/generic/nsBlockFrame.cpp:886:8
#3 nsLayoutUtils::IntrinsicForAxis(mozilla::PhysicalAxis, gfxContext*, nsIFrame*, nsLayoutUtils::IntrinsicISizeType, mozilla::Maybe<mozilla::LogicalSize> const&, unsigned int, int) src/layout/base/nsLayoutUtils.cpp
#4 nsLayoutUtils::IntrinsicForContainer(gfxContext*, nsIFrame*, nsLayoutUtils::IntrinsicISizeType, unsigned int) src/layout/base/nsLayoutUtils.cpp:5498:10
#5 nsHTMLButtonControlFrame::GetPrefISize(gfxContext*) src/layout/forms/nsHTMLButtonControlFrame.cpp:169:14
#6 nsFrame::ShrinkWidthToFit(gfxContext*, int, nsIFrame::ComputeSizeFlags) src/layout/generic/nsFrame.cpp:6318:25
#7 nsContainerFrame::ComputeAutoSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) src/layout/generic/nsContainerFrame.cpp:862:27
#8 nsFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) src/layout/generic/nsFrame.cpp:5556:24
#9 mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::LogicalSize const&, nsMargin const*, nsMargin const*, mozilla::LayoutFrameType) src/layout/generic/ReflowInput.cpp:2480:17
#10 mozilla::ReflowInput::Init(nsPresContext*, mozilla::LogicalSize const*, nsMargin const*, nsMargin const*) src/layout/generic/ReflowInput.cpp:414:3
#11 mozilla::ReflowInput::ReflowInput(nsPresContext*, mozilla::ReflowInput const&, nsIFrame*, mozilla::LogicalSize const&, mozilla::LogicalSize const*, unsigned int) src/layout/generic/ReflowInput.cpp:246:5
#12 void mozilla::Maybe<mozilla::ReflowInput>::emplace<nsPresContext*&, mozilla::ReflowInput const&, nsIFrame*&, mozilla::LogicalSize&>(nsPresContext*&, mozilla::ReflowInput const&, nsIFrame*&, mozilla::LogicalSize&) src/obj-firefox/dist/include/mozilla/Maybe.h:599:32
#13 nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:862:23
#14 nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4183:15
#15 nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3983:5
#16 nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3857:9
#17 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2841:5
#18 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2377:7
#19 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1239:3
#20 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
#21 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3488:11
#22 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2838:5
#23 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2377:7
#24 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1239:3
#25 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#26 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:769:5
#27 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#28 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:580:3
#29 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:703:3
#30 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1080:3
#31 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
#32 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:338:7
#33 mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8994:11
#34 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9167:24
#35 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4339:11
#36 nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1960:16
#37 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:303:7
#38 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:322:5
#39 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:767:5
#40 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:679:16
#41 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:579:9
#42 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16
#43 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
#44 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2181:28
#45 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2134:25
#46 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2064:17
#47 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1910:5
#48 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1943:15
#49 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1051:14
#50 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#51 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#52 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#53 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#54 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
#55 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:920:22
#56 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#57 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#58 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#59 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:746:34
#60 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#61 main src/browser/app/nsBrowserApp.cpp:287:18
#62 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#63 _start (firefox+0x423724)

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/b0BlKgsTK6AG8qBAnFG0cA/index.html

Comment 2

[Mats Palmgren (inactive)]

InlinePrefISizeData::ForceBreak does:
NSCoordSaturatingSubtract(mCurrentLine, mTrailingWhitespace, nscoord_MAX);
mTrailingWhitespace is nscoord_MAX and came from:
https://searchfox.org/mozilla-central/rev/a8773ba2a8f015e0525f219a7e55087b04d30258/layout/generic/nsTextFrame.cpp#8476
(the testcase has word-spacing:15946245.3ch)

I tend to think we should simply remove this assertion...

Comment 3

[Mats Palmgren (inactive)]

Attachment: Bug 1474774 - Remove nscoord assertion that produce false positives. r=dholbert

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc593b46778c
Remove nscoord assertion that produce false positives.  r=dholbert

Comment 5

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29083 for changes under testing/web-platform/tests

Comment 6

[Jonathan Kew [:jfkthame]]

I'm a bit sad to lose this assertion, as it was capable of pointing out genuine issues - as it happens, it just showed up in bug 1710116, where it was the result of failure to appropriately use saturating addition earlier in the code. That resulted in values greater than nscoord_MAX, which isn't likely to end well... this assertion was the canary that let us know things were broken.

Comment 7

[Mats Palmgren (inactive)]

I disagree that bug 1710116 is a "genuine issue". It's a synthetic testcase using inline-size: 3579845520.820976vw which will never occur on any real web sites.

There are two ways to go about nscoord overflow that I think are reasonable:
A. Make nscoord a class type with overloaded arithmetic operators that systematically checks for overflow. I.e. make nscoord behave as a saturated integer type.
-or-
B. Don't try to prevent integer overflow anywhere, but add std::max(0, value) in a few strategic places where a positive value is expected, such as the result of calculating an intrinsic size or whatnot.

I firmly believe that B is the right choice. It just doesn't matter what the layout result is when nonsensical values like inline-size: 3579845520.820976vw are used. Take for example BasicTableLayoutStrategy::DistributeISizeToColumns - in this function we should simply remove all the NSCoordSaturatingAdd calls and then we can do a std::max(0, result) at the end (if we feel it's worth it). I just don't think that the performance penalty for doing A is worth it to handle these synthetic testcases.

And I don't think that sprinkling NSCoordSaturatingAdd calls in mostly random places is a reasonable solution.

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/bc593b46778c

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot