Closed Bug 1420525 Opened 2 years ago Closed 2 years ago

UBSan: load of value which is not a valid value 'bool' [@ mozilla::EventStateManager::UpdateCursor]

Categories

(Core :: DOM: Events, defect, P2)

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: tsmith, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-undefined, testcase)

Attachments

(2 files)

Attached file testcase.html
This was found with a Firefox build built with -fsanitize=bool

/dom/events/EventStateManager.cpp:3738:31: runtime error: load of value 163, which is not a valid value for type 'bool'
    #0 mozilla::EventStateManager::UpdateCursor(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsEventStatus*) /dom/events/EventStateManager.cpp:3738:31
    #1 mozilla::EventStateManager::PreHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*) /dom/events/EventStateManager.cpp:726:5
    #2 mozilla::PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) /layout/base/PresShell.cpp:7802:19
    #3 mozilla::PresShell::HandlePositionedEvent(nsIFrame*, mozilla::WidgetGUIEvent*, nsEventStatus*) /layout/base/PresShell.cpp:7596:10
    #4 mozilla::PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) /layout/base/PresShell.cpp:7394:12
    #5 nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) /view/nsViewManager.cpp:812:14
    #6 nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) /view/nsView.cpp:1140:9
    #7 mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) /widget/PuppetWidget.cpp:395:35
    #8 mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(mozilla::WidgetGUIEvent&) /gfx/layers/apz/util/APZCCallbackHelper.cpp:499:21
    #9 mozilla::dom::TabChild::HandleRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) /dom/ipc/TabChild.cpp:1810:3
    #10 mozilla::dom::TabChild::RecvRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) /dom/ipc/TabChild.cpp:1777:3
    #11 mozilla::dom::TabChild::RecvSynthMouseMoveEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) /dom/ipc/TabChild.cpp:1738:8
    #12 mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) /objdir-ff-asan-O2/ipc/ipdl/PBrowserChild.cpp:3442:20
    #13 mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /objdir-ff-asan-O2/ipc/ipdl/PContentChild.cpp:4930:28
    #14 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /ipc/glue/MessageChannel.cpp:2114:25
    #15 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /ipc/glue/MessageChannel.cpp:2044:17
    #16 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /ipc/glue/MessageChannel.cpp:1890:5
    #17 mozilla::ipc::MessageChannel::MessageTask::Run() /ipc/glue/MessageChannel.cpp:1923:15
    #18 mozilla::SchedulerGroup::Runnable::Run() /xpcom/threads/SchedulerGroup.cpp:396:25
    #19 nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1037:14
    #20 NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:513:10
    #21 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:97:21
    #22 RunHandler /ipc/chromium/src/base/message_loop.cc:319:3
    #23 MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:299
    #24 nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:159:27
    #25 XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:877:22
    #26 RunHandler /ipc/chromium/src/base/message_loop.cc:319:3
    #27 MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:299
    #28 XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:703:34
    #29 content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #30 main /browser/app/nsBrowserApp.cpp:280
    #31 __libc_start_main /build/glibc-CxtIbX/glibc-2.26/csu/../csu/libc-start.c:308
    #32 _start (firefox+0x41efc9)
Flags: in-testsuite?
Summary: UBSan: load of value which is not a valid value 'bool' → UBSan: load of value which is not a valid value 'bool' [@ mozilla::EventStateManager::UpdateCursor]
Xidorn wrote much of the Cursor-related code in EventStateManager::UpdateCursor in bug 1200469 so maybe he has an idea of what is going on here?
Flags: needinfo?(xidorn+moz)
Could you provide which revision are you using? It seems EventStateManager.cpp:3738 is no longer in EventStateManager::UpdateCursor now, so it's not clear to me what line causes this.
Flags: needinfo?(twsmith)
OK I guess it was just the last revision 4 days ago. At that point, EventStateManager.cpp:3738 points to
> haveHotspot = framecursor.mHaveHotspot;
Flags: needinfo?(twsmith)
OK, so this is a matter of reading uninitialized value. nsHTMLFramesetFrame::GetCursor doesn't call nsFrame::GetCursor, and it doesn't fill all fields of nsIFrame::Cursor, and thus there are uninitialized values after GetCursor returns.

The fix should be easy. We can just put some initializers into nsIFrame::Cursor.
Flags: needinfo?(xidorn+moz)
Hsin-Yi, do you know anyone on your team with the time to take this while Xidorn is less available (despite him kindly responding here :)?
Flags: needinfo?(htsai)
Priority: -- → P2
Well, I'm responding :) I may leave for pto a day or two before all hands but most of time I should be around.

I expect this to be something trivial so I think I can handle that myself.
(In reply to Xidorn Quan [:xidorn] UTC-6 (less responsive Nov 5 ~ Dec 16) from comment #7)
> Well, I'm responding :) I may leave for pto a day or two before all hands
> but most of time I should be around.
> 
> I expect this to be something trivial so I think I can handle that myself.
Cool! Do feel free to let me know how my team could help when you see needs, Xidorn. :)
Flags: needinfo?(htsai)
Comment on attachment 8932659 [details]
Bug 1420525 - Initialize nsIFrame::Cursor to avoid uninitialized value inside.

https://reviewboard.mozilla.org/r/203710/#review209266
Attachment #8932659 - Flags: review?(tnikkel) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0840b7be4498
Initialize nsIFrame::Cursor to avoid uninitialized value inside. r=tnikkel
Assignee: nobody → xidorn+moz
https://hg.mozilla.org/mozilla-central/rev/0840b7be4498
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.