WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
226870
Nullptr crash in positionInParentBeforeNode via InsertTextCommand::doApply
https://bugs.webkit.org/show_bug.cgi?id=226870
Summary
Nullptr crash in positionInParentBeforeNode via InsertTextCommand::doApply
Ryosuke Niwa
Reported
2021-06-10 03:20:32 PDT
Created
attachment 431058
[details]
Test e.g. ASSERTION FAILED: ancestor ./dom/Position.cpp(1579) : WebCore::Position WebCore::positionInParentBeforeNode(WebCore::Node *) 1 0x1a999ee59 WTFCrash 2 0x18a8370db WTFCrashWithInfo(int, char const*, char const*, int) 3 0x18db1f9d0 WebCore::positionInParentBeforeNode(WebCore::Node*) 4 0x18dcbd546 WebCore::InsertTextCommand::doApply() 5 0x18dc32d05 WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::CompositeEditCommand, WTF::RawPtrTraits<WebCore::CompositeEditCommand> >&&, WebCore::VisibleSelection const&) 6 0x18dcfe7e4 WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool) 7 0x18dd209da WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const 8 0x18dcfe6aa void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&) 9 0x18dcfe5bc WebCore::TypingCommand::insertText(WTF::String const&, bool) 10 0x18dcfd084 WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool) 11 0x18dcfdc32 WebCore::TypingCommand::doApply() 12 0x18dc1e12c WebCore::CompositeEditCommand::apply() 13 0x18dceac43 WebCore::TextInsertionBaseCommand::applyTextInsertionCommand(WebCore::Frame*, WebCore::TextInsertionBaseCommand&, WebCore::VisibleSelection const&, WebCore::VisibleSelection const&) 14 0x18dcfcf77 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType) 15 0x18dcfccf3 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, unsigned int, WebCore::TypingCommand::TextCompositionType) 16 0x18dca4e44 WebCore::executeInsertText(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) 17 0x18dc7cfbb WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 18 0x18d985e45 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 19 0x18b242004 WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) 20 0x18b24196c long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) 21 0x18b22a524 WebCore::jsDocumentPrototypeFunction_execCommand(JSC::JSGlobalObject*, JSC::CallFrame*) 22 0x26d010e011d8 23 0x1a9fa671a llint_entry 24 0x1a9f84440 vmEntryToJavaScript 25 0x1aaea8d4b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 26 0x1aaea94ac JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 27 0x1ab2774ad JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 28 0x1ab27758f JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 29 0x1ab277872 JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 30 0x18d306fee WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 31 0x18d326fdb WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) <>ASSERTION FAILED: ancestor ./dom/Position.cpp(1579) : WebCore::Position WebCore::positionInParentBeforeNode(WebCore::Node *) 1 0x1a999ee59 WTFCrash 2 0x18a8370db WTFCrashWithInfo(int, char const*, char const*, int) 3 0x18db1f9d0 WebCore::positionInParentBeforeNode(WebCore::Node*) 4 0x18dcbd546 WebCore::InsertTextCommand::doApply() 5 0x18dc32d05 WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::CompositeEditCommand, WTF::RawPtrTraits<WebCore::CompositeEditCommand> >&&, WebCore::VisibleSelection const&) 6 0x18dcfe7e4 WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool) 7 0x18dd209da WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const 8 0x18dcfe6aa void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&) 9 0x18dcfe5bc WebCore::TypingCommand::insertText(WTF::String const&, bool) 10 0x18dcfd084 WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool) 11 0x18dcfdc32 WebCore::TypingCommand::doApply() 12 0x18dc1e12c WebCore::CompositeEditCommand::apply() 13 0x18dceac43 WebCore::TextInsertionBaseCommand::applyTextInsertionCommand(WebCore::Frame*, WebCore::TextInsertionBaseCommand&, WebCore::VisibleSelection const&, WebCore::VisibleSelection const&) 14 0x18dcfcf77 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType) 15 0x18dcfccf3 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, unsigned int, WebCore::TypingCommand::TextCompositionType) 16 0x18dca4e44 WebCore::executeInsertText(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) 17 0x18dc7cfbb WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 18 0x18d985e45 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 19 0x18b242004 WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) 20 0x18b24196c long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) 21 0x18b22a524 WebCore::jsDocumentPrototypeFunction_execCommand(JSC::JSGlobalObject*, JSC::CallFrame*) 22 0x26d010e011d8 23 0x1a9fa671a llint_entry 24 0x1a9f84440 vmEntryToJavaScript 25 0x1aaea8d4b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 26 0x1aaea94ac JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 27 0x1ab2774ad JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 28 0x1ab27758f JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 29 0x1ab277872 JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 30 0x18d306fee WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 31 0x18d326fdb WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) <
rdar://78607686
>
Attachments
Test
(378 bytes, text/html)
2021-06-10 03:20 PDT
,
Ryosuke Niwa
no flags
Details
Patch
(881 bytes, patch)
2021-06-15 07:23 PDT
,
Frédéric Wang (:fredw)
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(4.38 KB, patch)
2021-06-15 22:16 PDT
,
Frédéric Wang (:fredw)
rniwa
: review+
Details
Formatted Diff
Diff
Patch
(4.37 KB, patch)
2021-06-15 23:05 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2021-06-10 03:21:10 PDT
Reproduced with debug build of WebKitTestRunner at
r278627
.
Frédéric Wang (:fredw)
Comment 2
2021-06-15 07:23:25 PDT
Created
attachment 431435
[details]
Patch From a quick debugging, the <summary> element is detached from the tree in the testcase. Here is a tentative patch that fixes the assert.
Ryosuke Niwa
Comment 3
2021-06-15 14:23:10 PDT
Comment on
attachment 431435
[details]
Patch Please add the test for this. Also, the actual selection isn't orphaned here, is it?
Frédéric Wang (:fredw)
Comment 4
2021-06-15 21:27:33 PDT
(In reply to Ryosuke Niwa from
comment #3
)
> Please add the test for this.
Will do.
> Also, the actual selection isn't orphaned here, is it?
I guess you mean endingSelection().isOrphan()? Yes, it became orphan here too (see reverse debugging below). Do you mean we should rely on that method instead? (rr) bt #0 0x00007feb1ace29f7 in CRASH_WITH_INFO(...) () at WTF/Headers/wtf/Assertions.h:744 #1 0x00007feb2162f5b7 in WebCore::positionInParentBeforeNode(WebCore::Node*) (node=0x60c0002a7fc0) at ../../Source/WebCore/dom/Position.cpp:1579 #2 0x00007feb218d622b in WebCore::InsertTextCommand::doApply() (this= 0x61200006f340) at ../../Source/WebCore/editing/InsertTextCommand.cpp:177 ... (rr) reverse-f (rr) p showTree(node) *SUMMARY 0x60c0002a7fc0 (renderer (nil)) #document-fragment 0x61200007e940 (renderer (nil)) (needs style recalc) (child needs style recalc) DIV 0x60c0002a8080 (renderer (nil)) SLOT 0x60d000065fc0 (renderer (nil)) BODY 0x60c0002a82c0 (renderer (nil)) Q 0x60c0002a8380 (renderer (nil)) $1 = void (rr) watch -l node->m_parentNode (rr) rc (rr) delete (rr) reverse-f (rr) bt #0 0x00007feb2128da05 in WebCore::ContainerNode::removeBetween(WebCore::Node*, WebCore::Node*, WebCore::Node&) (this=0x60c0002a6a00, previousChild=0x0, nextChild=0x0, oldChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:655 #1 0x00007feb2129b9a2 in WebCore::ContainerNode::removeNodeWithScriptAssertion(WebCore::Node&, WebCore::ContainerNode::ChildChange::Source) (this=0x60c0002a6a00, childToRemove=..., source=WebCore::ContainerNode::ChildChange::Source::API) at ../../Source/WebCore/dom/ContainerNode.cpp:181 #2 0x00007feb2128d355 in WebCore::ContainerNode::removeChild(WebCore::Node&) (this=0x60c0002a6a00, oldChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:614 ... #15 0x00007feb218d5f30 in WebCore::InsertTextCommand::doApply() (this=0x61200006f340) at ../../Source/WebCore/editing/InsertTextCommand.cpp:143 ... (rr) b Source/WebCore/editing/InsertTextCommand.cpp:143 Breakpoint 2 at 0x7feb218d5f06: file ../../Source/WebCore/editing/InsertTextCommand.cpp, line 143. (rr) rc Continuing. 143 deleteSelection(false, true, true, false, false); (rr) p endingSelection().isNoneOrOrphaned() $2 = false (rr) p endingSelection().isOrphan() $3 = false (rr) p endingSelection().start().isOrphan() $4 = false (rr) p endingSelection().end().isOrphan() $5 = false (rr) n 147 if (endingSelection().isNone()) (rr) p endingSelection().isNoneOrOrphaned() $6 = true (rr) p endingSelection().isOrphan() $7 = true (rr) p endingSelection().start().isOrphan() $8 = true (rr) p endingSelection().end().isOrphan() $9 = true
Ryosuke Niwa
Comment 5
2021-06-15 21:31:26 PDT
no, I mean FrameSelection
Frédéric Wang (:fredw)
Comment 6
2021-06-15 22:04:47 PDT
(In reply to Ryosuke Niwa from
comment #5
)
> no, I mean FrameSelection
OK, yes after deleteSelection, endingSelection().isOrphan() is true but document().selection().selection().isOrphan() is false.
Frédéric Wang (:fredw)
Comment 7
2021-06-15 22:16:23 PDT
Created
attachment 431515
[details]
Patch
Ryosuke Niwa
Comment 8
2021-06-15 22:43:11 PDT
(In reply to Frédéric Wang (:fredw) from
comment #6
)
> (In reply to Ryosuke Niwa from
comment #5
) > > no, I mean FrameSelection > > OK, yes after deleteSelection, endingSelection().isOrphan() is true but > document().selection().selection().isOrphan() is false.
Cool.
Ryosuke Niwa
Comment 9
2021-06-15 22:44:18 PDT
Comment on
attachment 431515
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=431515&action=review
> LayoutTests/ChangeLog:6 > + Add regression test.
This should appear after "reviewed by" line.
> Source/WebCore/ChangeLog:7 > + deleteSelection call In InsertTextCommand::doApply() can make the endingSelection() orphan. > + If that happens, exit early to avoid dereferencing a nullptr pointer later.
This long description should appear below "reviewed by" line while the bug title & URL should continue to appear above it.
Frédéric Wang (:fredw)
Comment 10
2021-06-15 22:58:45 PDT
Comment on
attachment 431515
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=431515&action=review
>> LayoutTests/ChangeLog:6 >> + Add regression test. > > This should appear after "reviewed by" line.
Oops, I thought about it but got confused with the 'Test:' thing that is put after... Will fix.
Frédéric Wang (:fredw)
Comment 11
2021-06-15 23:05:44 PDT
Created
attachment 431521
[details]
Patch
EWS
Comment 12
2021-06-16 06:36:28 PDT
Committed
r278930
(
238861@main
): <
https://commits.webkit.org/238861@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 431521
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug