1786095 - Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Bogus row index?), at src/layout/tables/nsCellMap.cpp:361

Status: RESOLVED DUPLICATE of bug 1789461

(Core :: Disability Access APIs, defect)

Description

[Daniel Holbert [:dholbert]]

Bug 1472024 has evolved to be focused on a new testcase (the one that currently repro's the fatal assertion).

There's an older testcase there that doesn't seem to repro the assertion anymore:
https://bugzilla.mozilla.org/attachment.cgi?id=8988616
...but we do have a failure captured in pernosco:
https://pernos.co/debug/xeJgrRrjAUR7DBx5A-MWbg/index.html
...so I'm filing this bug on investigating that older failure (assuming there might still be something we can/should do to mitigate that).

+++ This bug was initially created as a clone of Bug #1472024 +++

Found while fuzzing m-c 20180628-b429b9fb68f1 (--enable-debug --enable-fuzzing)

Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Bogus row index?), at src/layout/tables/nsCellMap.cpp:361

#0 nsTableCellMap::GetEffectiveColSpan(int, int) const src/layout/tables/nsCellMap.cpp:361:3
#1 mozilla::a11y::HTMLTableAccessible::ColExtentAt(unsigned int, unsigned int) src/accessible/html/HTMLTableAccessible.cpp:677:22
#2 mozilla::a11y::HTMLTableCellAccessible::ColExtent() const src/accessible/html/HTMLTableAccessible.cpp:196:17
#3 mozilla::a11y::HTMLTableHeaderCellAccessible::NativeRole() const src/accessible/html/HTMLTableAccessible.cpp:332:53
#4 mozilla::a11y::Accessible::Role() const src/accessible/generic/Accessible-inl.h:26:30
#5 mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:60:28
#6 mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:79:5
#7 mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:79:5
#8 mozilla::a11y::DocAccessibleChildBase::SerializeTree(mozilla::a11y::Accessible*, nsTArray<mozilla::a11y::AccessibleData>&) src/accessible/ipc/DocAccessibleChildBase.cpp:79:5
#9 mozilla::a11y::DocAccessibleChildBase::InsertIntoIpcTree(mozilla::a11y::Accessible*, mozilla::a11y::Accessible*, unsigned int) src/accessible/ipc/DocAccessibleChildBase.cpp:92:3
#10 mozilla::a11y::DocAccessible::DoInitialUpdate() src/accessible/generic/DocAccessible.cpp:1519:17
#11 mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:666:16
#12 nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1868:12
#13 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7
#14 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:320:5
#15 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:760:5
#16 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:673:35
#17 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:574:9
#18 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16
#19 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
#20 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1988:28
#21 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2134:25
#22 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2064:17
#23 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1910:5
#24 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1943:15
#25 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1051:14
#26 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#27 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#28 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#29 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#30 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
#31 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:896:22
#32 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#33 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#34 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#35 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:722:34
#36 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#37 main src/browser/app/nsBrowserApp.cpp:287:18
#38 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#39 _start (firefox+0x4236e4)

Comment 1

[Daniel Holbert [:dholbert]]

I have some analysis about what's going on here in bug 1472024 comment 7 and bug 1472024 comment 12.

Quoting bug 1472024 comment #7:

I took a look here.

In the original pernsoco trace (which is for the original testcase from comment 0, now obsolete), there seems to be a mixup about which table is getting queried. The testcase has a table with a position:absolute tbody child. That tbody moves out-of-flow since it's position:absolute, and the frame construction table-fixup gives it an (also-out-of-flow) table parent.

The issue here seems to be that the accessibility code is getting the row index from a cell inside that out-of-flow table (which is legitimately in row #1 i.e. the 2nd row there), and then asking for more information about that row (row #1, 2nd row) from the other table. And that fails, because that table is essentially empty, except that it has the "placeholder frame" for the out-of-flow table.

So: we need to find out why we're querying the wrong table frame there.

Quoting bug 1472024 comment #15:

I can't reproduce the crash with the original testcase from comment 0 [even when running with GNOME_ACCESSIBILITY=1 But I do have a slightly better understanding of what was going on there, from poking through pernosco a bit more. Essentially, in that pernosco trace, it looks like the a11y function HTMLTableCellAccessible::Table is assuming that it can walk up the parent chain from a table-cell in order to find the table -- but in this case, the cell really lives in an out-of-flow subtree (position:absolute) so it's actually in a different anonymous out-of-flow table that gets created via table-fixup -- and its true table frame "owner" is not its DOM ancestor <table> element. So, we end up querying the wrong table frame (the one associated with its ancestor table element) for information about the cell, using its index, and we fail this fatal assertion about the index being out of bounds.

Comment 2

[Daniel Holbert [:dholbert]]

Tyson, I notice you marked the testcase in question (attachment 8988616 [details]) obsolete 11 days ago, when you posted the newer testcase in bug 1472024 comment 5.

I'm guessing you marked it as obsolete because that testcase used to reliably reproduce the issue, but doesn't anymore -- is that correct? (I'm guessing so, but want to double-check.)

FWIW I tried loading/repeatedly-reloading that testcase in a current debug build as well as in a build from a debug build from 1 year ago (2021-08-19)[1], with GNOME_ACCESSIBILITY=1, and I wasn't able to repro any crash.

[1] That's the oldest debug build that I can test via mozregression, since we only preserve debug-build artifacts for a year, per bug 1773091

Comment 3

[Daniel Holbert [:dholbert]]

I'm asking because the pernosco trace here (originally from bug 1472024 comment 2) is from 2 years ago -- 2020-10-26 -- and I'm wondering if this was somehow fixed between then and the oldest debug build that I can test (2021-08-19) vs. if maybe the issue still persists and it just requires additional configuration or hammering.

Comment 4

[Tyson Smith [:tsmith] (PTO)]

(In reply to Daniel Holbert [:dholbert] from comment #2)

Tyson, I notice you marked the testcase in question (attachment 8988616 [details]) obsolete 11 days ago, when you posted the newer testcase in bug 1472024 comment 5.

I'm guessing you marked it as obsolete because that testcase used to reliably reproduce the issue, but doesn't anymore -- is that correct? (I'm guessing so, but want to double-check.)

Yes that is correct. As triggers of this assertion are resolved I will open/update bugs with new test cases as they are found by fuzzers.

Comment 5

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Bugmon was unable to identify a testcase that reproduces this issue.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 6

[James Teh [:Jamie] (away returning 28 Jan)]

Marking s4 for now, but we should raise this if this becomes a fuzzblocker again and/or if this causes issues for users.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

This bug prevents fuzzing from making progress; however, it has low severity. It is important for fuzz blocker bugs to be addressed in a timely manner (see here why?).
:Jamie, could you increase the severity?

For more information, please visit auto_nag documentation.

Comment 8

[James Teh [:Jamie] (away returning 28 Jan)]

:tsmith, given comment 5, is this no longer a fuzz blocker?

Comment 11

[Tyson Smith [:tsmith] (PTO)]

(In reply to James Teh [:Jamie] from comment #8)

:tsmith, given comment 5, is this no longer a fuzz blocker?

Correct. In fact the fuzzers are no longer reporting this assertion with this stack.

Comment 12

[Daniel Holbert [:dholbert]]

I think we can dupe this forward to bug 1789461, which has a testcase and a more-recent pernosco session. Looks like the same assertion with a similar backtrace.