1490685 - Assertion failure: density > 0.0, at /builds/worker/workspace/build/src/layout/generic/nsImageFrame.cpp:411

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 703546ab6d0c.

Assertion failure: density > 0.0, at /builds/worker/workspace/build/src/layout/generic/nsImageFrame.cpp:411

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007f41beeac030
rsi = 0x00007f41d8f1c8b0   rdi = 0x00007f41d8f1b680
rbp = 0x00007ffe402d72f0   rsp = 0x00007ffe402d7270
r8 = 0x00007f41d8f1c8b0    r9 = 0x00007f41da094740
r10 = 0x00000000ffffffc3   r11 = 0x0000000000000000
r12 = 0x00007ffe402d7280   r13 = 0x00007f41beeac020
r14 = 0x00007ffe402d72a0   r15 = 0x00007f41beeabf60
rip = 0x00007f41c9a5fa7d
OS|Linux|0.0.0 Linux 4.15.0-33-generic #36-Ubuntu SMP Wed Aug 15 16:00:05 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|0
0|0|libxul.so|nsImageFrame::UpdateIntrinsicSize(imgIContainer*)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCOMPtr.h:703546ab6d0cb643028a1ab4fda997b38f38a2e6|819|0x21
0|1|libxul.so|nsImageFrame::OnSizeAvailable(imgIRequest*, imgIContainer*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsImageFrame.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|694|0xf
0|2|libxul.so|nsImageFrame::Notify(imgIRequest*, int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsImageFrame.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|641|0xf
0|3|libxul.so|ReplayImageStatus|hg:hg.mozilla.org/mozilla-central:dom/base/nsImageLoadingContent.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|414|0x13
0|4|libxul.so|nsImageLoadingContent::AddNativeObserver(imgINotificationObserver*)|hg:hg.mozilla.org/mozilla-central:dom/base/nsImageLoadingContent.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|458|0xc
0|5|libxul.so|nsImageFrame::Init(nsIContent*, nsContainerFrame*, nsIFrame*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsImageFrame.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|323|0x18
0|6|libxul.so|nsCSSFrameConstructor::InitAndRestoreFrame(nsFrameConstructorState const&, nsIContent*, nsContainerFrame*, nsIFrame*, bool)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|4793|0x1d
0|7|libxul.so|nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|3832|0x26
0|8|libxul.so|nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|5954|0x16
0|9|libxul.so|nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|9965|0x15
0|10|libxul.so|nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|7588|0x20
0|11|libxul.so|nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|9041|0x1a
0|12|libxul.so|mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&)|hg:hg.mozilla.org/mozilla-central:layout/base/RestyleManager.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|1551|0xf
0|13|libxul.so|mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags)|hg:hg.mozilla.org/mozilla-central:layout/base/RestyleManager.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|3057|0xb
0|14|libxul.so|mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|4295|0x19
0|15|libxul.so|nsIDocument::FlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/nsIPresShell.h:703546ab6d0cb643028a1ab4fda997b38f38a2e6|577|0xf
0|16|libxul.so|nsIDocument::FlushPendingNotifications(mozilla::FlushType)|hg:hg.mozilla.org/mozilla-central:dom/base/nsDocument.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|7502|0x7
0|17|libxul.so|nsDocLoader::DocLoaderIsEmpty(bool)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|698|0x10
0|18|libxul.so|nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|631|0x16
0|19|libxul.so|mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult)|hg:hg.mozilla.org/mozilla-central:netwerk/base/nsLoadGroup.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|629|0x1f
0|20|libxul.so|nsIDocument::DoUnblockOnload()|hg:hg.mozilla.org/mozilla-central:dom/base/nsDocument.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|8422|0x20
0|21|libxul.so|nsDocument::UnblockOnload(bool)|hg:hg.mozilla.org/mozilla-central:dom/base/nsDocument.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|8344|0x5
0|22|libxul.so|mozilla::LoadBlockingAsyncEventDispatcher::~LoadBlockingAsyncEventDispatcher()|hg:hg.mozilla.org/mozilla-central:dom/events/AsyncEventDispatcher.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|127|0x19
0|23|libxul.so|mozilla::LoadBlockingAsyncEventDispatcher::~LoadBlockingAsyncEventDispatcher()|hg:hg.mozilla.org/mozilla-central:dom/events/AsyncEventDispatcher.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|129|0x5
0|24|libxul.so|mozilla::Runnable::Release()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|50|0x9
0|25|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCOMPtr.h:703546ab6d0cb643028a1ab4fda997b38f38a2e6|638|0x5
0|26|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|1161|0x15
0|27|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|519|0x11
0|28|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|97|0xa
0|29|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:703546ab6d0cb643028a1ab4fda997b38f38a2e6|325|0x17
0|30|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:703546ab6d0cb643028a1ab4fda997b38f38a2e6|318|0x8
0|31|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|158|0xd
0|32|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|944|0x11
0|33|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|269|0x5
0|34|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:703546ab6d0cb643028a1ab4fda997b38f38a2e6|325|0x17
0|35|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:703546ab6d0cb643028a1ab4fda997b38f38a2e6|318|0x8
0|36|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|770|0x8
0|37|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|50|0x14
0|38|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:703546ab6d0cb643028a1ab4fda997b38f38a2e6|287|0x11
0|39|libc-2.27.so||||0x21b97
0|40|firefox-bin|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:703546ab6d0cb643028a1ab4fda997b38f38a2e6|164|0x5

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1490685 - Protect against division by zero in a couple places. r=dholbert

I'm glad I added that assertion, because the other callers I'm fixing
were also bogus.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Note that I'm doing this under the assumption that float division by zero is UB, which it is according to https://stackoverflow.com/questions/15277129/c-divide-by-zero.

But if we don't care much about it I should just adjust the assertion to >=. Please double-check me on this :)

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
> Note that I'm doing this under the assumption that float division by zero is UB

FWIW, https://www.quora.com/Why-does-division-by-zero-return-INF-infinite-with-floats-but-makes-the-program-crash-with-integers-in-C++ says:
 - "the C++ standard says "If the second operand of / or % is zero the behavior is undefined"
 - "However, if std::numeric_limits<T>::is_iec559 == true, which it normally is for T = float and T = double, the behavior is overridden by IEC 559 aka IEEE 754, which says ... The default result shall be a correctly signed infinity" (IEE 754 paragraph 7.2)

I'll bet we're pretty well guaranteed to have IEEE754-compliant floats types.  So this probably isn't an issue in practice. Nonetheless, probably reasonable to do some belt-and-suspenders checking I suppose.

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 9008442 [details]
Bug 1490685 - Protect against division by zero in a couple places. r=dholbert

Daniel Holbert [:dholbert] has approved the revision.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to Daniel Holbert [:dholbert] from comment #3)
> I'll bet we're pretty well guaranteed to have IEEE754-compliant floats
> types.  So this probably isn't an issue in practice. Nonetheless, probably
> reasonable to do some belt-and-suspenders checking I suppose.

Given this I talked with Daniel and decided to just adjust the assertion for now, I don't think it's worth the churn.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/643ffcb9063d
Adjust an assertion. r=dholbert

Comment 7

[Daniel Holbert [:dholbert]]

Should've noticed during review -- this didn't end up landing with the testcase as a crashtest. Mind pushing that as a followup?

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Yeah, it was kinda intentional, since my reasoning was that the crashtest wasn't likely to come up ever again. But I guess somebody could mess worse with the density and it'd make sense to have a crashtest that selects an image with 0 density, will do.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4451dbf509b8
Crashtest.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/643ffcb9063d
https://hg.mozilla.org/mozilla-central/rev/4451dbf509b8