Closed
Bug 1396267
Opened 7 years ago
Closed 7 years ago
crash near null in [@ mozilla::a11y::IDRefsIterator::IDRefsIterator]
Categories
(Core :: Disability Access APIs, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: tsmith, Assigned: eeejay)
References
(Blocks 2 open bugs)
Details
(Keywords: crash, csectype-nullptr, testcase)
Attachments
(2 files)
985 bytes,
text/html
|
Details | |
2.27 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
Found with m-c: BuildID=20170901232924 SourceStamp=5278dfcf5eb9f58eaf06ad1ce67e7fd4aba34772 ==24590==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000001c (pc 0x7f4d35009c12 bp 0x7ffc4da9bdb0 sp 0x7ffc4da9bdb0 T0) ==24590==The signal is caused by a READ memory access. ==24590==Hint: address points to the zero page. #0 0x7f4d35009c11 in GetBoolFlag src/dom/base/nsINode.h:1624:12 #1 0x7f4d35009c11 in IsInUncomposedDoc src/dom/base/nsINode.h:556 #2 0x7f4d35009c11 in mozilla::a11y::IDRefsIterator::IDRefsIterator(mozilla::a11y::DocAccessible*, nsIContent*, nsIAtom*) src/accessible/base/AccIterator.cpp:260 #3 0x7f4d350bd6e8 in mozilla::a11y::DocAccessible::DoARIAOwnsRelocation(mozilla::a11y::Accessible*) src/accessible/generic/DocAccessible.cpp:2081:18 #4 0x7f4d3502b59c in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:813:18 #5 0x7f4d31e02b8c in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1886:12 #6 0x7f4d31e11c0b in TickDriver src/layout/base/nsRefreshDriver.cpp:337:13 #7 0x7f4d31e11c0b in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:307 #8 0x7f4d31e118f4 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:328:5 #9 0x7f4d31e13f1b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:770:5 #10 0x7f4d31e13f1b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:683 #11 0x7f4d31e0f547 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:529:20 #12 0x7f4d2b2a604d in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1039:14 #13 0x7f4d2b2ab688 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:521:10 #14 0x7f4d2c0462f1 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21 #15 0x7f4d2bfa6a0b in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #16 0x7f4d2bfa6a0b in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #17 0x7f4d2bfa6a0b in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #18 0x7f4d317210ef in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27 #19 0x7f4d35850201 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:288:30 #20 0x7f4d35a31f01 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4635:22 #21 0x7f4d35a33af8 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4799:8 #22 0x7f4d35a34f2b in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4894:21 #23 0x4eb673 in do_main src/browser/app/nsBrowserApp.cpp:236:22 #24 0x4eb673 in main src/browser/app/nsBrowserApp.cpp:309 #25 0x7f4d48c1582f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #26 0x41d1c8 in _start (/home/user/workspace/browsers/m-c-1504308564-asan-opt/firefox+0x41d1c8)
Flags: in-testsuite?
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → eitan
Assignee | ||
Comment 1•7 years ago
|
||
This is what happens: 1. Body is restyled with "display: none". a. This ultimately triggers a content insertion with a null container, and the child being the document element. Apparently this is a legitimate case. b. This causes us to queue a content insertion with the owner being the doc accessible as fallback (this also seems to be intended) 2. Document element (document.documentElement) is unparented. 3. Insertion from stage 1 is processed. This calls UpdateRootElIfNeeded which fails to find a nsIContent to associate with the doc accessible because the document element was removed in stage 2. 4. aria-owns relocation is processed on the doc accessible that now does not have an associated content. I think we will need to put in a stronger check before doing a relocation to make sure the accessible still has an associated content node.
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8906771 -
Flags: review?(surkov.alexander)
Comment 5•7 years ago
|
||
Comment on attachment 8906771 [details] [diff] [review] Check that owner has content before aria-owns relocation. r?surkov Review of attachment 8906771 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/base/NotificationController.cpp @@ +814,1 @@ > mDocument->DoARIAOwnsRelocation(mRelocations[idx]); I wonder if it can be a document's case: if the document doesn't have a body and etc, then there's no place to put aria-owns attribute
Comment 6•7 years ago
|
||
(In reply to alexander :surkov from comment #5) > ::: accessible/base/NotificationController.cpp > @@ +814,1 @@ > > mDocument->DoARIAOwnsRelocation(mRelocations[idx]); > > I wonder if it can be a document's case: if the document doesn't have a body > and etc, then there's no place to put aria-owns attribute yeah, I should read bug comments before looking at the patch :)
Comment 7•7 years ago
|
||
Comment on attachment 8906771 [details] [diff] [review] Check that owner has content before aria-owns relocation. r?surkov Review of attachment 8906771 [details] [diff] [review]: ----------------------------------------------------------------- that makes sense, so r=me.
Attachment #8906771 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/32268f6138e0 Check that owner has content before aria-owns relocation. r=surkov
Keywords: checkin-needed
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32268f6138e0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
status-firefox55:
--- → wontfix
status-firefox56:
--- → wontfix
status-firefox-esr52:
--- → wontfix
Flags: in-testsuite? → in-testsuite+
Version: Trunk → 48 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•