Closed Bug 1474774 Opened 6 years ago Closed 4 years ago

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

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
90 Branch
Tracking Flags:
Tracking Status
firefox-esr68 --- wontfix
firefox-esr78 --- wontfix
firefox63 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- fixed

People

(Reporter: tsmith, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

testcase.html
139 bytes, text/html
Details
Bug 1474774 - Remove nscoord assertion that produce false positives. r=dholbert
48 bytes, text/x-phabricator-request
Details | Review
Attached file 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)
Flags: in-testsuite?
Priority: -- → P3

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...

Assignee: nobody → mats
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc593b46778c Remove nscoord assertion that produce false positives. r=dholbert

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.

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.

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

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox90: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
status-firefox-esr78: affected → wontfix
Flags: in-testsuite? → in-testsuite+
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: