1601998 - AddressSanitizer: SEGV /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:2073:46 in GetStateBits

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev e2394b695d21. Testcase requires the GNOME_ACCESSIBILITY=1 env variable in order to reproduce.

==25542==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000058 (pc 0x7ffa7b841ce7 bp 0x7ffddb772180 sp 0x7ffddb772180 T0)
==25542==The signal is caused by a READ memory access.
==25542==Hint: address points to the zero page.
    #0 0x7ffa7b841ce6 in GetStateBits /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:2073:46
    #1 0x7ffa7b841ce6 in GetPrevInFlow /builds/worker/workspace/build/src/layout/generic/nsSplittableFrame.cpp:113:11
    #2 0x7ffa7b841ce6 in nsSplittableFrame::FirstInFlow() const /builds/worker/workspace/build/src/layout/generic/nsSplittableFrame.cpp:144:40
    #3 0x7ffa7b93ce68 in nsTableFrame::GetCellMap() const /builds/worker/workspace/build/src/layout/tables/nsTableFrame.cpp:642:37
    #4 0x7ffa7e4a2fae in GetEffectiveColSpanAt /builds/worker/workspace/build/src/layout/tables/nsTableWrapperFrame.h:168:46
    #5 0x7ffa7e4a2fae in mozilla::a11y::HTMLTableAccessible::ColExtentAt(unsigned int, unsigned int) /builds/worker/workspace/build/src/accessible/html/HTMLTableAccessible.cpp:621:36
    #6 0x7ffa7e49c6e8 in mozilla::a11y::HTMLTableCellAccessible::ColExtent() const /builds/worker/workspace/build/src/accessible/html/HTMLTableAccessible.cpp:175:17
    #7 0x7ffa7e49ebba in mozilla::a11y::HTMLTableHeaderCellAccessible::NativeRole() const /builds/worker/workspace/build/src/accessible/html/HTMLTableAccessible.cpp:288:53
    #8 0x7ffa7e404037 in Role /builds/worker/workspace/build/src/accessible/generic/Accessible-inl.h:25:30
    #9 0x7ffa7e404037 in Role /builds/worker/workspace/build/src/accessible/base/AccessibleOrProxy.h:98:28
    #10 0x7ffa7e404037 in mozilla::a11y::nsAccUtils::MustPrune(mozilla::a11y::AccessibleOrProxy) /builds/worker/workspace/build/src/accessible/base/nsAccUtils.cpp:399:34
    #11 0x7ffa7e4474bf in mozilla::a11y::Accessible::AppendTextTo(nsTSubstring<char16_t>&, unsigned int, unsigned int) /builds/worker/workspace/build/src/accessible/generic/Accessible.cpp:1923:25
    #12 0x7ffa7e3ccca4 in mozilla::a11y::NotificationController::QueueMutationEvent(mozilla::a11y::AccTreeMutationEvent*) /builds/worker/workspace/build/src/accessible/base/NotificationController.cpp:204:28
    #13 0x7ffa7e3ce10b in mozilla::a11y::TreeMutation::BeforeRemoval(mozilla::a11y::Accessible*, bool) /builds/worker/workspace/build/src/accessible/base/EventTree.cpp:86:21
    #14 0x7ffa7e4636d4 in mozilla::a11y::DocAccessible::ContentRemoved(mozilla::a11y::Accessible*) /builds/worker/workspace/build/src/accessible/generic/DocAccessible.cpp:2064:6
    #15 0x7ffa7e45b1ef in mozilla::a11y::DocAccessible::ContentRemoved(nsIContent*) /builds/worker/workspace/build/src/accessible/generic/DocAccessible.cpp:2093:5
    #16 0x7ffa7e463bd1 in mozilla::a11y::DocAccessible::RecreateAccessible(nsIContent*) /builds/worker/workspace/build/src/accessible/generic/DocAccessible.cpp:1467:3
    #17 0x7ffa7b7897a4 in DisconnectMap /builds/worker/workspace/build/src/layout/generic/nsImageFrame.cpp:245:17
    #18 0x7ffa7b7897a4 in nsImageFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsImageFrame.cpp:260:3
    #19 0x7ffa7b7b8861 in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsLineBox.cpp:380:14
    #20 0x7ffa7b578de2 in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:370:3
    #21 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #22 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #23 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #24 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #25 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #26 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #27 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #28 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #29 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #30 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #31 0x7ffa7b7b8861 in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsLineBox.cpp:380:14
    #32 0x7ffa7b578de2 in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:370:3
    #33 0x7ffa7b7b8861 in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsLineBox.cpp:380:14
    #34 0x7ffa7b578de2 in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:370:3
    #35 0x7ffa7b7b8861 in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsLineBox.cpp:380:14
    #36 0x7ffa7b578de2 in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:370:3
    #37 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #38 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #39 0x7ffa7b6ba1a9 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:51:12
    #40 0x7ffa7b57959e in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:215:11
    #41 0x7ffa7b5e5b12 in Destroy /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:655:5
    #42 0x7ffa7b5e5b12 in nsContainerFrame::RemoveFrame(mozilla::layout::FrameChildListID, nsIFrame*) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:169:19
    #43 0x7ffa7b41963e in RemoveFrame /builds/worker/workspace/build/src/layout/base/nsFrameManager.cpp:116:18
    #44 0x7ffa7b41963e in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7614:5
    #45 0x7ffa7b4066ef in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:8627:7
    #46 0x7ffa7b399180 in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:1536:25
    #47 0x7ffa7b3a4be5 in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:3082:9
    #48 0x7ffa7b356f58 in ProcessPendingRestyles /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:3161:3
    #49 0x7ffa7b356f58 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4069:39
    #50 0x7ffa7b2dae27 in FlushPendingNotifications /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/PresShell.h:1452:5
    #51 0x7ffa7b2dae27 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:2021:22
    #52 0x7ffa7b2eb1a1 in TickDriver /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:374:13
    #53 0x7ffa7b2eb1a1 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:351:7
    #54 0x7ffa7b2eacb9 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:367:5
    #55 0x7ffa7b2ea013 in RunRefreshDrivers /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:820:5
    #56 0x7ffa7b2ea013 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:740:16
    #57 0x7ffa7b2e7555 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:538:20
    #58 0x7ffa71e9a99a in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1250:14
    #59 0x7ffa71ea1e41 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #60 0x7ffa730e3f9f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
    #61 0x7ffa72febfa2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #62 0x7ffa72febfa2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #63 0x7ffa72febfa2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #64 0x7ffa7ad5c888 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
    #65 0x7ffa7ebc4e1f in nsAppStartup::Run() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:276:30
    #66 0x7ffa7ee21109 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4608:22
    #67 0x7ffa7ee23107 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4745:8
    #68 0x7ffa7ee24850 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4826:21
    #69 0x558b756f3397 in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:218:22
    #70 0x558b756f3397 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:339:16
    #71 0x7ffa95275b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: 471a5d036d3ecab267d64e95c9036b4ea9190825.ico

Comment 2

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

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Hi Jamie, any thoughts on this bug's severity?

Comment 4

[James Teh [:Jamie]]

I haven't seen any reports of this in the wild, so p2 instead of p1.

This is similar to bug 1572811. We try to query the frame tree, but it's probably mid destruction at this point. We didn't do this before bug 686400 because before that change, we would have already destroyed the entire subtree when layout told us the frame tree was being reconstructed.

This is tricky. On one hand, we deliberately don't remove the a11y subtree when the frame tree is reconstructed; that's the whole point of bug 686400 and is necessary so we don't pointlessly rebuild the a11y subtree. On the other hand, we want to remove the a11y subtree as soon as layout asks us to for image maps (as in this bug) or as soon as anonymous content gets removed (as in bug 1572811), but to do that, we need to fire events on the parent, which sometimes means we query the frame tree of something that's being reconstructed.

Two solutions I can think of:

1. We need to know when a frame subtree is mid destruction. That would probably require tracking reconstruction in a11y and having Accessible::GetFrame() check whether there's any frame pending reconstruction in that Accessible's ancestry. That seems pretty brittle/ugly/unperformant to me. Unless there's some other way?
2. Ensure that all a11y ContentRemoved calls that might happen during frame tree destruction are queued. The challenge there is that by that point, the DOM nodes are detached from the tree and we might depend on that state somewhere to figure out how to react to the removal. I'm not sure.

Comment 5

[James Teh [:Jamie]]

For this particular case, it's probably easiest to have nsImageFrame::DisconnectMap schedule RecreateAccessible, rather than calling it sync. That will require nsAccessibilityService::RecreateAccessible to take an argument specifying whether to schedule.

Comment 6

[James Teh [:Jamie]]

Attachment: Bug 1601998: When disconnecting an image map, schedule Accessible recreation instead of doing it synchronously.

When we remove an Accessible, we need to fire events.
However, we sometimes query the frame tree in figuring out what events to fire.
When disconnecting an image map, the frame tree may be mid destruction, so this isn't safe and can cause crashes.
Scheduling the call to RecreateAccessible ensures this only happens when it's safe.

Comment 7

[Timea Cernea [:tbabos][inactive]]

Removing regression-window wanted based on Comment 4

Comment 8

[James Teh [:Jamie]]

Fixed by bug 1597916.