1538768 - UBSan: load of value 228, which is not a valid value for type 'bool' [@ mozilla::layers::WebRenderScrollDataCollection::AppendRoot]

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Tyson Smith [:tsmith]]

The fuzzers started reporting this on Saturday. It appears to be triggered on start up. This blocks fuzzing WR on ASan+UBSan builds

Report from m-c 20190325-4a692c812a3f

/src/gfx/layers/wr/WebRenderCommandBuilder.cpp:1024:9: runtime error: load of value 228, which is not a valid value for type 'bool'
    #0 0x7f87c544912a in mozilla::layers::WebRenderScrollDataCollection::AppendRoot(mozilla::Maybe<mozilla::layers::ScrollMetadata>&, mozilla::wr::RenderRootArray<mozilla::layers::WebRenderScrollData>&) /src/gfx/layers/wr/WebRenderCommandBuilder.cpp:1024:9
    #1 0x7f87c5466ac8 in mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands(mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&, nsDisplayList*, nsDisplayListBuilder*, mozilla::wr::RenderRootArray<mozilla::layers::WebRenderScrollData>&, WrFiltersHolder&&) /src/gfx/layers/wr/WebRenderCommandBuilder.cpp:1567:21
    #2 0x7f87c548bc23 in mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer(nsDisplayList*, nsDisplayListBuilder*, WrFiltersHolder&&, mozilla::layers::WebRenderBackgroundData*) /src/gfx/layers/wr/WebRenderLayerManager.cpp:327:30
    #3 0x7f87cda2b71d in nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) /src/layout/painting/nsDisplayList.cpp:2765:18
    #4 0x7f87ccf5582e in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /src/layout/base/nsLayoutUtils.cpp:3883:12
    #5 0x7f87ccde9003 in mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) /src/layout/base/PresShell.cpp:6059:5
    #6 0x7f87cc55232f in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) /src/view/nsViewManager.cpp:461:19
    #7 0x7f87cc55112c in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) /src/view/nsViewManager.cpp:396:33
    #8 0x7f87cc556bc6 in nsViewManager::ProcessPendingUpdates() /src/view/nsViewManager.cpp:1022:5
    #9 0x7f87ccd3018b in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:2031:11
    #10 0x7f87ccd42719 in TickDriver /src/layout/base/nsRefreshDriver.cpp:342:13
    #11 0x7f87ccd42719 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /src/layout/base/nsRefreshDriver.cpp:319
    #12 0x7f87ccd42008 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:336:5
    #13 0x7f87ccd4624f in RunRefreshDrivers /src/layout/base/nsRefreshDriver.cpp:777:5
    #14 0x7f87ccd4624f in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:697
    #15 0x7f87ccd3f7c0 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /src/layout/base/nsRefreshDriver.cpp:502:20
    #16 0x7f87c1fff511 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1180:14
    #17 0x7f87c200791d in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:482:10
    #18 0x7f87c32d4a9f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #19 0x7f87c31aa80e in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #20 0x7f87c31aa80e in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #21 0x7f87c31aa80e in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #22 0x7f87cc64d693 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #23 0x7f87d09362f0 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:271:30
    #24 0x7f87d0c33cda in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4589:22
    #25 0x7f87d0c36708 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4727:8
    #26 0x7f87d0c37f99 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4811:21
    #27 0x55f0887f864c in do_main /src/browser/app/nsBrowserApp.cpp:212:22
    #28 0x55f0887f864c in main /src/browser/app/nsBrowserApp.cpp:291

Comment 1

[Tyson Smith [:tsmith]]

For reference the builds that are affected can be found here:
https://tools.taskcluster.net/index/gecko.v2.mozilla-central.latest.firefox/linux64-asan-opt

Comment 2

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

This is likely from the document splitting patch. I can look tomorrow.

Comment 3

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

I think the problem here is that RenderRootArray's default constructor doesn't initialize the underlying array so it holds garbage. We should get rid of that constructor and make sure all the users use the other constructor that explicitly initializes the array.

Comment 4

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

Actually after some reading it looks like in this scenario the array members are "default initialized" which means calling the class constructor if it's a class, leaving it undefined for primitive types, etc. So that's why the bool array gets filled with garbage but other uses of RenderRootArray mostly don't run into problems.

Still, this is a footgun of sorts that we should fix. I'm trying to figure out what the best fix is.

Comment 5

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

Attachment: Bug 1538768 - Ensure that RenderRootArray<bool> is properly initialized to false on creation. r?dthayer

Comment 6

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

Attachment: Bug 1538768 - Also zero-initialize primitive types in NonDefaultRenderRootArray. r?dthayer

Depends on D24950

Comment 7

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

Attachment: Bug 1538768 - Add a handful of test jobs for QR on ASAN builds. r?jrmuizel

This runs crashtests and reftests on ASAN builds with WR enabled, so
that we catch any ASAN regressions prior to landing without incurring
too great of a test load hit.

Depends on D24951

Comment 8

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

https://treeherder.mozilla.org/#/jobs?repo=try&author=kgupta%40mozilla.com&group_state=expanded&fromchange=215e8e44a23f53b7fe0d58ecf9bca5b0064aa026&tochange=68d90eb241316f0e698adeb6776b623e102b2752

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2c826ffcfef
Ensure that RenderRootArray<bool> is properly initialized to false on creation. r=dthayer
https://hg.mozilla.org/integration/autoland/rev/dca4a1fbc5e2
Also zero-initialize primitive types in NonDefaultRenderRootArray. r=dthayer
https://hg.mozilla.org/integration/autoland/rev/da4c9acc07e1
Add a handful of test jobs for QR on ASAN builds. r=jrmuizel

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/e2c826ffcfef
https://hg.mozilla.org/mozilla-central/rev/dca4a1fbc5e2
https://hg.mozilla.org/mozilla-central/rev/da4c9acc07e1