1413618 - UBSan: layout/base/nsLayoutUtils.cpp:1134:50: runtime error: division by zero [@ GetDisplayPortFromMarginsData]

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Tyson Smith [:tsmith]]

This error is trigger on startup when Firefox is built with: -fsanitize=float-divide-by-zero,integer-divide-by-zero

This could lead to crashes when using different build tools and/or compiler optimization combinations.

/mozilla-central/layout/base/nsLayoutUtils.cpp:1134:50: runtime error: division by zero
    #0 0x7fd6ba76f784 in GetDisplayPortFromMarginsData(nsIContent*, mozilla::DisplayPortMarginsPropertyData*, float) /mozilla-central/layout/base/nsLayoutUtils.cpp:1134:50
    #1 0x7fd6ba719024 in GetDisplayPortImpl(nsIContent*, nsRect*, float, MaxSizeExceededBehaviour) /mozilla-central/layout/base/nsLayoutUtils.cpp:1307:14
    #2 0x7fd6ba718a0a in nsLayoutUtils::GetDisplayPort(nsIContent*, nsRect*, RelativeTo) /mozilla-central/layout/base/nsLayoutUtils.cpp:1346:27
    #3 0x7fd6ba71a1d2 in GetHighResolutionDisplayPort /mozilla-central/layout/base/nsLayoutUtils.cpp:1548:10
    #4 0x7fd6ba71a1d2 in nsLayoutUtils::SetDisplayPortMargins(nsIContent*, nsIPresShell*, mozilla::gfx::MarginTyped<mozilla::ScreenPixel, float> const&, unsigned int, nsLayoutUtils::RepaintMode) /mozilla-central/layout/base/nsLayoutUtils.cpp:1451
    #5 0x7fd6b4ee683b in mozilla::layers::APZCCallbackHelper::InitializeRootDisplayport(nsIPresShell*) /mozilla-central/gfx/layers/apz/util/APZCCallbackHelper.cpp:370:5
    #6 0x7fd6b4f092e2 in applyImpl<mozilla::layers::ChromeProcessController, void (mozilla::layers::ChromeProcessController::*)()> /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1142:12
    #7 0x7fd6b4f092e2 in apply<mozilla::layers::ChromeProcessController, void (mozilla::layers::ChromeProcessController::*)()> /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1148
    #8 0x7fd6b4f092e2 in mozilla::detail::RunnableMethodImpl<mozilla::layers::ChromeProcessController*, void (mozilla::layers::ChromeProcessController::*)(), true, (mozilla::RunnableKind)0>::Run() /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1192
    #9 0x7fd6b1cd89b4 in nsThread::ProcessNextEvent(bool, bool*) /mozilla-central/xpcom/threads/nsThread.cpp:1037:14
    #10 0x7fd6b1d075d0 in NS_ProcessNextEvent(nsIThread*, bool) /mozilla-central/xpcom/threads/nsThreadUtils.cpp:513:10
    #11 0x7fd6b2f873fa in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /mozilla-central/ipc/glue/MessagePump.cpp:97:21
    #12 0x7fd6b2e385f0 in RunInternal /mozilla-central/ipc/chromium/src/base/message_loop.cc:326:10
    #13 0x7fd6b2e385f0 in RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319
    #14 0x7fd6b2e385f0 in MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #15 0x7fd6b9d836b4 in nsBaseAppShell::Run() /mozilla-central/widget/nsBaseAppShell.cpp:158:27
    #16 0x7fd6bf192975 in nsAppStartup::Run() /mozilla-central/toolkit/components/startup/nsAppStartup.cpp:288:30
    #17 0x7fd6bf3cced6 in XREMain::XRE_mainRun() /mozilla-central/toolkit/xre/nsAppRunner.cpp:4675:22
    #18 0x7fd6bf3cf7e9 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /mozilla-central/toolkit/xre/nsAppRunner.cpp:4837:8
    #19 0x7fd6bf3d1585 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /mozilla-central/toolkit/xre/nsAppRunner.cpp:4932:21
    #20 0x517014 in do_main /mozilla-central/browser/app/nsBrowserApp.cpp:231:22
    #21 0x517014 in main /mozilla-central/browser/app/nsBrowserApp.cpp:304
    #22 0x7fd6d600d1c0 in __libc_start_main /build/glibc-CxtIbX/glibc-2.26/csu/../csu/libc-start.c:308
    #23 0x41f7c9 in _start (/mozilla-central/objdir-ff-ubsan/dist/bin/firefox+0x41f7c9)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Based on the time and the line number listed above, I suspect the corresponding line is: https://searchfox.org/mozilla-central/rev/a6cfee725b9e49adda2c81ced98f27de55603d4e/layout/base/nsLayoutUtils.cpp#1102
> float scale = std::min(1.0f, float(budget) / margins.TopBottom());

Looking around the code, it seems there is nothing guarantees that margins.TopBottom() wouldn't be zero, but I suspect std::min is meant to also catch the case when TopBottom() returns zero.

Comment 2

[Tyson Smith [:tsmith]]

Attachment: div_by_zero.txt

Here are a few more. 

My build was based on:
changeset: 397325:fe1794e607cc

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I agree with Xidorn's analysis in comment 1.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1413618 - Avoid possible division by zero.

Review commit: https://reviewboard.mozilla.org/r/210924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210924/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=338040050726b1ae91377a4b285a69f012b18efd

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8940673 [details]
Bug 1413618 - Avoid possible division by zero.

https://reviewboard.mozilla.org/r/210924/#review216654

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9d593d6eced
Avoid possible division by zero. r=xidorn

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 9

[Gerry Chang [:gchang]]

It's too late for 58. Mark 58 won't fix.