Bug 229277 – Nullptr crash in TypingCommand::willAddTypingToOpenCommand via TypingCommand::deleteKeyPressed

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

Created attachment 435836 [details]
Test

e.g.

#0 0x14afd5252 in WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >::Ref(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> > const&)+0x42 (WebCore.framework/Versions/A/WebCore:x86_64+0xf57252)
#1 0x14afd5208 in WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >::Ref(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> > const&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0xf57208)
#2 0x14afd518e in WebCore::BoundaryPoint::BoundaryPoint(WebCore::BoundaryPoint const&)+0x1e (WebCore.framework/Versions/A/WebCore:x86_64+0xf5718e)
#3 0x14afd5168 in WebCore::BoundaryPoint::BoundaryPoint(WebCore::BoundaryPoint const&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0xf57168)
#4 0x14afd5901 in WebCore::SimpleRange::SimpleRange(WebCore::SimpleRange const&)+0x21 (WebCore.framework/Versions/A/WebCore:x86_64+0xf57901)
#5 0x14afd4c98 in WebCore::SimpleRange::SimpleRange(WebCore::SimpleRange const&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0xf56c98)
#6 0x14d9ac471 in WebCore::StaticRange::create(WebCore::SimpleRange const&)+0xc1 (WebCore.framework/Versions/A/WebCore:x86_64+0x392e471)
#7 0x14db91d7c in WebCore::TypingCommand::willAddTypingToOpenCommand(WebCore::TypingCommand::ETypingCommand, WebCore::TextGranularity, WTF::String const&, std::__1::optional<WebCore::SimpleRange> const&)+0x22c (WebCore.framework/Versions/A/WebCore:x86_64+0x3b13d7c)
#8 0x14db8cdee in WebCore::TypingCommand::deleteKeyPressed(WebCore::TextGranularity, bool)+0x8be (WebCore.framework/Versions/A/WebCore:x86_64+0x3b0edee)
#9 0x14db90f4e in WebCore::TypingCommand::doApply()+0x1be (WebCore.framework/Versions/A/WebCore:x86_64+0x3b12f4e)
#10 0x14da353c6 in WebCore::CompositeEditCommand::apply()+0x216 (WebCore.framework/Versions/A/WebCore:x86_64+0x39b73c6)
#11 0x14db8c27b in WebCore::TypingCommand::deleteKeyPressed(WebCore::Document&, unsigned int, WebCore::TextGranularity)+0x29b (WebCore.framework/Versions/A/WebCore:x86_64+0x3b0e27b)
#12 0x14db05dae in WebCore::executeDelete(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)+0xde (WebCore.framework/Versions/A/WebCore:x86_64+0x3a87dae)
#13 0x14dac808b in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const+0xdb (WebCore.framework/Versions/A/WebCore:x86_64+0x3a4a08b)
#14 0x14d72dc7e in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&)+0x14e (WebCore.framework/Versions/A/WebCore:x86_64+0x36afc7e)
#15 0x14ab2d8c8 in WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)+0x4f8 (WebCore.framework/Versions/A/WebCore:x86_64+0xaaf8c8)
#16 0x14ab2d330 in 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*)+0x100 (WebCore.framework/Versions/A/WebCore:x86_64+0xaaf330)
#17 0x14ab165a8 in WebCore::jsDocumentPrototypeFunction_execCommand(JSC::JSGlobalObject*, JSC::CallFrame*)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0xa985a8)
#18 0x3ae554a011d7  (<unknown module>)
#19 0x16806f8d6 in llint_entry+0x1b5c6 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xe318d6)
#20 0x168054108 in vmEntryToJavaScript+0xd7 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xe16108)
#21 0x169a75084 in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x5e4 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2837084)
#22 0x16a375b84 in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x64 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x3137b84)

<rdar://80333314>

[Ryosuke Niwa]

I can reproduce this with ASAN release build of WebKitTestRunner at r281219.

[Rob Buis]

Created attachment 436289 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 436289 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436289&action=review

> Source/WebCore/editing/TypingCommand.cpp:666
> +                if (auto range = deleteListSelection.value().firstRange()) {

I presume deleteListSelection is "none"?
It's confusing that CompositeEditCommand::shouldBreakOutOfEmptyListItem() returns optional<VisibleSelection>.
We normally use VisibleSelection.isNone() in these cases.
Given shouldBreakOutOfEmptyListItem() is only used in this function, I think we should go ahead & make that change
instead of returning checking for both std::nullopt and VisibleSelection being none.

[Rob Buis]

Created attachment 436770 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 436770 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436770&action=review

> Source/WebCore/editing/TypingCommand.cpp:667
> +            auto deleteListSelection = shouldBreakOutOfEmptyListItem();
> +            if (!deleteListSelection.isNone()) {
> +                if (willAddTypingToOpenCommand(DeleteKey, granularity, { }, deleteListSelection.firstRange())) {

Define the variable inside if like so:
if (auto deleteListSelection = shouldBreakOutOfEmptyListItem(); !deleteListSelection.isNone()) {

[Rob Buis]

Created attachment 436848 [details]
Patch

[EWS]

Committed r281795 (241132@main): <https://commits.webkit.org/241132@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436848 [details].