Closed Bug 1425091 Opened 2 years ago Closed 2 years ago

crash near null in [@ mozilla::EditorBase::MoveNode]

Categories

(Core :: DOM: Editor, defect, P1)

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: tsmith, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(5 files)

Attached file testcase.html
==17559==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f07d63a8dfc bp 0x7fff1b7367d0 sp 0x7fff1b7367d0 T0)
==17559==The signal is caused by a READ memory access.
==17559==Hint: address points to the zero page.
    #0 0x7f07d63a8dfb in get includes/src/obj-firefox/dist/include/mozilla/RefPtr.h:287:27
    #1 0x7f07d63a8dfb in operator-> includes/src/obj-firefox/dist/include/mozilla/RefPtr.h:319
    #2 0x7f07d63a8dfb in NodeType includes/src/dom/base/nsINode.h:591
    #3 0x7f07d63a8dfb in nsINode::Length() const includes/src/dom/base/nsINode.cpp:2672
    #4 0x7f07da5da9f4 in mozilla::EditorBase::MoveNode(nsIContent*, nsINode*, int) includes/src/editor/libeditor/EditorBase.cpp:1899:46
    #5 0x7f07da6622cb in mozilla::HTMLEditRules::WillMakeList(mozilla::dom::Selection*, nsTSubstring<char16_t> const*, bool, nsTSubstring<char16_t> const*, bool*, bool*, nsTSubstring<char16_t> const*) includes/src/editor/libeditor/HTMLEditRules.cpp:3732:24
    #6 0x7f07da64acda in mozilla::HTMLEditRules::WillDoAction(mozilla::dom::Selection*, mozilla::RulesInfo*, bool*, bool*) includes/src/editor/libeditor/HTMLEditRules.cpp:656:14
    #7 0x7f07da70cdc9 in mozilla::HTMLEditor::MakeOrChangeList(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&) includes/src/editor/libeditor/HTMLEditor.cpp:1914:24
    #8 0x7f07da7fb78b in nsListCommand::ToggleState(mozilla::HTMLEditor*) includes/src/editor/composer/nsComposerCommands.cpp:332:23
    #9 0x7f07da7f88e7 in nsBaseStateUpdatingCommand::DoCommand(char const*, nsISupports*) includes/src/editor/composer/nsComposerCommands.cpp:105:10
    #10 0x7f07d8669440 in nsControllerCommandTable::DoCommand(char const*, nsISupports*) includes/src/dom/commandhandler/nsControllerCommandTable.cpp:147:26
    #11 0x7f07d865f316 in nsBaseCommandController::DoCommand(char const*) includes/src/dom/commandhandler/nsBaseCommandController.cpp:136:25
    #12 0x7f07d8665e94 in nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) includes/src/dom/commandhandler/nsCommandManager.cpp:212:22
    #13 0x7f07d8baa35b in nsHTMLDocument::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) includes/src/dom/html/nsHTMLDocument.cpp:3282:18
    #14 0x7f07d7f2842a in mozilla::dom::HTMLDocumentBinding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) includes/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:848:21
    #15 0x7f07d82f0727 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) includes/src/dom/bindings/BindingUtils.cpp:3042:13
    #16 0x7f07dee87cb4 in CallJSNative includes/src/js/src/jscntxtinlines.h:291:15
    #17 0x7f07dee87cb4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) includes/src/js/src/vm/Interpreter.cpp:473
    #18 0x7f07dee6dd96 in CallFromStack includes/src/js/src/vm/Interpreter.cpp:528:12
    #19 0x7f07dee6dd96 in Interpret(JSContext*, js::RunState&) includes/src/js/src/vm/Interpreter.cpp:3096
    #20 0x7f07dee5a500 in js::RunScript(JSContext*, js::RunState&) includes/src/js/src/vm/Interpreter.cpp:423:12
    #21 0x7f07dee881ec in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) includes/src/js/src/vm/Interpreter.cpp:495:15
    #22 0x7f07dee88d12 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) includes/src/js/src/vm/Interpreter.cpp:541:10
    #23 0x7f07df98490c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) includes/src/js/src/jsapi.cpp:2995:12
    #24 0x7f07d7c15a3f in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) includes/src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8
    #25 0x7f07d87956f3 in HandleEvent<mozilla::dom::EventTarget *> includes/src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12
    #26 0x7f07d87956f3 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) includes/src/dom/events/EventListenerManager.cpp:1108
    #27 0x7f07d8797642 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) includes/src/dom/events/EventListenerManager.cpp:1286:20
    #28 0x7f07d87822cd in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) includes/src/dom/events/EventDispatcher.cpp:486:14
    #29 0x7f07d878591b in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) includes/src/dom/events/EventDispatcher.cpp:826:9
    #30 0x7f07d878753c in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) includes/src/dom/events/EventDispatcher.cpp:892:12
    #31 0x7f07d63990af in nsINode::DispatchEvent(nsIDOMEvent*, bool*) includes/src/dom/base/nsINode.cpp:1361:5
    #32 0x7f07d870fba3 in mozilla::AsyncEventDispatcher::Run() includes/src/dom/events/AsyncEventDispatcher.cpp:70:12
    #33 0x7f07d5e8c7bf in nsContentUtils::RemoveScriptBlocker() includes/src/dom/base/nsContentUtils.cpp:5666:15
    #34 0x7f07d629aa1f in nsDocument::EndUpdate(unsigned int) includes/src/dom/base/nsDocument.cpp:5442:3
    #35 0x7f07d8ba7ffc in nsHTMLDocument::EndUpdate(unsigned int) includes/src/dom/html/nsHTMLDocument.cpp:2455:15
    #36 0x7f07d63a04eb in ~mozAutoDocUpdate includes/src/dom/base/mozAutoDocUpdate.h:40:18
    #37 0x7f07d63a04eb in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) includes/src/dom/base/nsINode.cpp:2545
    #38 0x7f07d6b363f3 in InsertBefore includes/src/dom/base/nsINode.h:1850:12
    #39 0x7f07d6b363f3 in AppendChild includes/src/dom/base/nsINode.h:1854
    #40 0x7f07d6b363f3 in mozilla::dom::NodeBinding::appendChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) includes/src/obj-firefox/dom/bindings/NodeBinding.cpp:891
    #41 0x7f07d82f0727 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) includes/src/dom/bindings/BindingUtils.cpp:3042:13
    #42 0x7f07dee87cb4 in CallJSNative includes/src/js/src/jscntxtinlines.h:291:15
    #43 0x7f07dee87cb4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) includes/src/js/src/vm/Interpreter.cpp:473
    #44 0x7f07dee6dd96 in CallFromStack includes/src/js/src/vm/Interpreter.cpp:528:12
    #45 0x7f07dee6dd96 in Interpret(JSContext*, js::RunState&) includes/src/js/src/vm/Interpreter.cpp:3096
    #46 0x7f07dee5a500 in js::RunScript(JSContext*, js::RunState&) includes/src/js/src/vm/Interpreter.cpp:423:12
    #47 0x7f07dee881ec in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) includes/src/js/src/vm/Interpreter.cpp:495:15
    #48 0x7f07dee88d12 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) includes/src/js/src/vm/Interpreter.cpp:541:10
    #49 0x7f07df98490c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) includes/src/js/src/jsapi.cpp:2995:12
    #50 0x7f07d7c10a4e in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) includes/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #51 0x7f07d87cee03 in Call<nsISupports *> includes/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #52 0x7f07d87cee03 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) includes/src/dom/events/JSEventHandler.cpp:215
    #53 0x7f07d8795731 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) includes/src/dom/events/EventListenerManager.cpp:1111:51
    #54 0x7f07d8797642 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) includes/src/dom/events/EventListenerManager.cpp:1286:20
    #55 0x7f07d878202f in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) includes/src/dom/events/EventDispatcher.cpp:462:16
    #56 0x7f07d878591b in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) includes/src/dom/events/EventDispatcher.cpp:826:9
    #57 0x7f07dad90961 in nsDocumentViewer::LoadComplete(nsresult) includes/src/layout/base/nsDocumentViewer.cpp:1070:7
    #58 0x7f07de104202 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) includes/src/docshell/base/nsDocShell.cpp:7906:21
    #59 0x7f07de10012a in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) includes/src/docshell/base/nsDocShell.cpp:7699:7
    #60 0x7f07de107f2f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) includes/src/docshell/base/nsDocShell.cpp
    #61 0x7f07d4fdb9e7 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) includes/src/uriloader/base/nsDocLoader.cpp:1319:3
    #62 0x7f07d4fdabf1 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) includes/src/uriloader/base/nsDocLoader.cpp:862:14
    #63 0x7f07d4fd7884 in nsDocLoader::DocLoaderIsEmpty(bool) includes/src/uriloader/base/nsDocLoader.cpp:751:9
    #64 0x7f07d4fd98bc in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) includes/src/uriloader/base/nsDocLoader.cpp:633:5
    #65 0x7f07d4fda7dc in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) includes/src/uriloader/base/nsDocLoader.cpp
    #66 0x7f07d32a14ca in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) includes/src/netwerk/base/nsLoadGroup.cpp:629:28
    #67 0x7f07d62c2e77 in DoUnblockOnload includes/src/dom/base/nsDocument.cpp:9149:18
    #68 0x7f07d62c2e77 in nsDocument::UnblockOnload(bool) includes/src/dom/base/nsDocument.cpp:9071
    #69 0x7f07d629f00a in nsDocument::DispatchContentLoadedEvents() includes/src/dom/base/nsDocument.cpp:5711:3
    #70 0x7f07d631e804 in applyImpl<nsDocument, void (nsDocument::*)()> includes/src/obj-firefox/dist/include/nsThreadUtils.h:1142:12
    #71 0x7f07d631e804 in apply<nsDocument, void (nsDocument::*)()> includes/src/obj-firefox/dist/include/nsThreadUtils.h:1148
    #72 0x7f07d631e804 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() includes/src/obj-firefox/dist/include/nsThreadUtils.h:1192
    #73 0x7f07d30b4b54 in mozilla::SchedulerGroup::Runnable::Run() includes/src/xpcom/threads/SchedulerGroup.cpp:396:25
    #74 0x7f07d30db42e in nsThread::ProcessNextEvent(bool, bool*) includes/src/xpcom/threads/nsThread.cpp:1033:14
    #75 0x7f07d30f6d90 in NS_ProcessNextEvent(nsIThread*, bool) includes/src/xpcom/threads/nsThreadUtils.cpp:508:10
    #76 0x7f07d3f7e21a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) includes/src/ipc/glue/MessagePump.cpp:97:21
    #77 0x7f07d3ed51a9 in RunInternal includes/src/ipc/chromium/src/base/message_loop.cc:326:10
    #78 0x7f07d3ed51a9 in RunHandler includes/src/ipc/chromium/src/base/message_loop.cc:319
    #79 0x7f07d3ed51a9 in MessageLoop::Run() includes/src/ipc/chromium/src/base/message_loop.cc:299
    #80 0x7f07da484dda in nsBaseAppShell::Run() includes/src/widget/nsBaseAppShell.cpp:157:27
    #81 0x7f07debb8b0b in XRE_RunAppShell() includes/src/toolkit/xre/nsEmbedFunctions.cpp:865:22
    #82 0x7f07d3ed51a9 in RunInternal includes/src/ipc/chromium/src/base/message_loop.cc:326:10
    #83 0x7f07d3ed51a9 in RunHandler includes/src/ipc/chromium/src/base/message_loop.cc:319
    #84 0x7f07d3ed51a9 in MessageLoop::Run() includes/src/ipc/chromium/src/base/message_loop.cc:299
    #85 0x7f07debb84fd in XRE_InitChildProcess(int, char**, XREChildData const*) includes/src/toolkit/xre/nsEmbedFunctions.cpp:691:34
    #86 0x4ee965 in content_process_main includes/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #87 0x4ee965 in main includes/src/browser/app/nsBrowserApp.cpp:280
    #88 0x7f07f1d1382f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #89 0x41dfe8 in _start (firefox+0x41dfe8)
Flags: in-testsuite?
Crash Signature: [@ nsINode::Length const ]
Priority: -- → P1
Assignee: nobody → m_kato
Comment on attachment 8938769 [details]
Bug 1425091 - Part 1. Should check whether CreateNode is failure.

https://reviewboard.mozilla.org/r/209308/#review215180
Attachment #8938769 - Flags: review?(masayuki) → review+
Comment on attachment 8938770 [details]
Bug 1425091 - Part 2. Add crashtest.

https://reviewboard.mozilla.org/r/209310/#review215182
Attachment #8938770 - Flags: review?(masayuki) → review+
Comment on attachment 8938772 [details]
Bug 1425091 - Part 4. EditorBase::MoveNode should check valid node that has parent node.

https://reviewboard.mozilla.org/r/209314/#review215186
Attachment #8938772 - Flags: review?(masayuki) → review+
Comment on attachment 8938771 [details]
Bug 1425091 - Part 3. SelAdjCreateNode and SelAdjInsertNode should check whether valid point.

https://reviewboard.mozilla.org/r/209312/#review215188

Just avoiding assertion, this is okay because you might request to uplift them.

However, I don't think that this is right approach to fix the bug. If DOM tree is changed by a mutation observer in EditorBase::CreateNode(), we need to decide what we should do.  There are some possible approaches:

1. Quitting the action with succeeded code. I think that throwing an exception is not assumed by any web apps which uses mutation observer and I guess that other browsers don't do it.  So, perhaps, we need new succeeded code which caller can distinguish with if mutation observer overwrites the edit operation.
2. Set selection next to the created node even if it's moved to different container. I guess that this case is NOT expected by users.
3. Set selection to next to the expected previous child of new node if new node is moved to different container or just removed from the tree.

Anyway, I don't know if #2 and #3 are reasonable because mutation observer may want to set selection by itself. Then, we shouldn't touch the selection. However, I don't know how other browsers do it in this case.
Attachment #8938771 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/5a346a696bc7
Part 1. Should check whether CreateNode is failure. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/6301157475d8
Part 2. Add crashtest. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/d05e5fbf2956
Part 3. SelAdjCreateNode and SelAdjInsertNode should check whether valid point. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/d46a837e1cfa
Part 4. EditorBase::MoveNode should check valid node that has parent node. r=masayuki
We're building the first Fx58 RC build next week and there's no signs of this crash signature in the wild. Calling this wontfix for 58, but feel free to set the status to affected and nominate the patches for approval if you feel strongly otherwise.
Blocks: 1415062
Flags: in-testsuite? → in-testsuite+
Version: 59 Branch → 58 Branch
You need to log in before you can comment on or make changes to this bug.