1503964 - crash at null in [@ mozilla::a11y::TableAccessible::CellInRowAt]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c:
BuildID=20181101133336
SourceStamp=182a1b088330a2d72310ae2561004d955571e236

==4400==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fddf9630e52 bp 0x7ffffcd0c290 sp 0x7ffffcd0c1a0 T0)
==4400==The signal is caused by a READ memory access.
    #0 0x7fddf9630e51 in mozilla::a11y::TableAccessible::CellInRowAt(mozilla::a11y::Accessible*, int) src/accessible/generic/TableAccessible.cpp:268:36
    #1 0x7fddf96c182b in mozilla::a11y::HTMLTableHeaderCellAccessible::NativeRole() const src/accessible/html/HTMLTableAccessible.cpp:339:17
    #2 0x7fddf96cda2d in Role src/accessible/generic/Accessible-inl.h:26:30
    #3 0x7fddf96cda2d in mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:60
    #4 0x7fddf96cdcb6 in mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:79:5
    #5 0x7fddf96cdcb6 in mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:79:5
    #6 0x7fddf96ce19d in mozilla::a11y::DocAccessibleChildBase::InsertIntoIpcTree(mozilla::a11y::Accessible*, mozilla::a11y::Accessible*, unsigned int) src/accessible/ipc/DocAccessibleChildBase.cpp:92:3
    #7 0x7fddf96779d9 in mozilla::a11y::DocAccessible::DoInitialUpdate() src/accessible/generic/DocAccessible.cpp:1530:17
    #8 0x7fddf95bffb6 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:677:16
    #9 0x7fddf620f9f4 in nsRefreshDriver::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1877:12
    #10 0x7fddf6223163 in TickDriver src/layout/base/nsRefreshDriver.cpp:326:13
    #11 0x7fddf6223163 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301
    #12 0x7fddf6222e31 in mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:319:5
    #13 0x7fddf6225def in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:760:5
    #14 0x7fddf6225def in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:676
    #15 0x7fddf6225722 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:573:9
    #16 0x7fddf6d1db08 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:76:16
    #17 0x7fdded74bef7 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
    #18 0x7fdded4d042d in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2280:28
    #19 0x7fddeccde839 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2255:25
    #20 0x7fddeccda1ba in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2182:17
    #21 0x7fddeccdc3c1 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:2019:5
    #22 0x7fddeccdd287 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:2052:15
    #23 0x7fddeba91811 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1245:14
    #24 0x7fddeba9a55d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:530:10
    #25 0x7fddecce7b9f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #26 0x7fddecbe3efe in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #27 0x7fddecbe3efe in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #28 0x7fddecbe3efe in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #29 0x7fddf5b3dd03 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
    #30 0x7fddfa231fae in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:939:22
    #31 0x7fddecbe3efe in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #32 0x7fddecbe3efe in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #33 0x7fddecbe3efe in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #34 0x7fddfa230fda in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:765:34
    #35 0x55e68b195864 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #36 0x55e68b195864 in main src/browser/app/nsBrowserApp.cpp:301
    #37 0x7fde0f19e82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #38 0x55e68b0baeec in _start (firefox+0x2deec)

Comment 1

[Marco Zehe (:MarcoZ)]

So this tries to evaluate whether the first cell should be evaluated as a column header, and HTMLTableHeaderCellAccessible::NativeRole wants to look at the cell below the current one.

In this test case, that cell is actually a tr with a role "columnheader". Crazy, but that's what we have fuzzers for. ;)

So HTMLTableAccessible::CellInRowAt gets to the point where it thinks this cell that is being asked for is actually a TableRowAccessible. This is unexpected since the role is actually suggesting otherwise. But it's being returned as the generic type for that particular accessible, so TableAccessible::CellInRowAt gets called and lands inside the loop at https://hg.mozilla.org/mozilla-central/annotate/182a1b088330a2d72310ae2561004d955571e236/accessible/generic/TableAccessible.cpp#l268.

The failure is that this row is not implementing the nsIAccessibleTableCell interface. AsTableCell() on the accessible apparently fails, and ColExtent() is then an invalid method call.

However, in a local build, I changed that line to this block:

    if (cell->IsTableCell()) {
      colIdx -= cell->AsTableCell()->ColExtent();
    } else {
      colIdx--;
    }

and it still crashes.

This will need some pondering...

CCÄing Jamie for awareness.

Comment 2

[Marco Zehe (:MarcoZ)]

Attachment: Bug 1503964 - Prevent crashes when dealing with crazy table row and column role constructs, r=jamie

Sometimes, IsTableCell() and AsTableCell() get out of sync. This needs further investigation. For now, wallpaper over this and prevent crashing.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Marco Zehe (:MarcoZ) from comment #2)
> Created attachment 9022124 [details]
> Bug 1503964 - Prevent crashes when dealing with crazy table row and column
> role constructs, r=jamie
> 
> Sometimes, IsTableCell() and AsTableCell() get out of sync. This needs
> further investigation. For now, wallpaper over this and prevent crashing.

I would recommend to remove eTableCell setting from ARIAMap (https://searchfox.org/mozilla-central/search?q=eTableCell&path=ARIAMap.cpp), and move it to the actual implementations (https://searchfox.org/mozilla-central/search?q=AsTableCell&path=). At least it will be less fragile than it's now, and also it should fix the crash.

Comment 4

[Marco Zehe (:MarcoZ)]

This change resulted in over 100 test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d66365bf081c0fa8681cdaea412f9a390d8147f7&selectedJob=209773509. There seem to be a lot of implicit dependencies on these map entries. Will take a while to figure out.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d895d29dbf5e
Prevent crashes when dealing with crazy table row and column role constructs, r=Jamie

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

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