Bug 177808 – REGRESSION(r222697): [GTK] Crash in WebCore::SelectionRangeData::apply

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Created attachment 322512 [details]
Backtrace

To reproduce, visit https://expired.badssl.com/ and click the Technical Information expander. In trunk it crashes 100%. Backtrace attached.

[Michael Catanzaro]

Another reproducer is to run TestWebViewEditor, it will crash 100% (in debug builds):

#0  0x00007fc9ae285646 in std::__atomic_base<unsigned char>::compare_exchange_weak (__m2=std::memory_order_acquire, __m1=std::memory_order_acquire, 
    __i2=1 '\001', __i1=@0x7ffd4b9639ab: 0 '\000', this=0x0)
    at /usr/include/c++/7/bits/atomic_base.h:434
#1  std::__atomic_base<unsigned char>::compare_exchange_weak (
    __m=std::memory_order_acquire, __i2=1 '\001', __i1=<optimized out>, 
    this=0x0) at /usr/include/c++/7/bits/atomic_base.h:456
#2  WTF::Atomic<unsigned char>::compareExchangeWeak (this=0x0, 
    expected=0 '\000', desired=1 '\001', order=std::memory_order_acquire)
    at ../../Source/WTF/wtf/Atomics.h:87
#3  0x00007fc9ae2850a6 in WTF::LockAlgorithm<unsigned char, (unsigned char)1, (unsigned char)2>::lockFastAssumingZero (lock=...)
    at ../../Source/WTF/wtf/LockAlgorithm.h:46
#4  0x00007fc9ae284cd2 in WTF::LockBase::lock (this=0x0)
    at ../../Source/WTF/wtf/Lock.h:62
#5  0x00007fc9ae28608e in std::lock_guard<WTF::Lock>::lock_guard (
    this=0x7ffd4b963a68, __m=...) at /usr/include/c++/7/bits/std_mutex.h:162
#6  0x00007fc9ae31e0fa in WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::String>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::String> >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::String, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::String> >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::invalidateIterators (
    this=0x7ffd4b963c78) at ../../Source/WTF/wtf/HashTable.h:1389
#7  0x00007fc9ae31e088 in WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::String>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::String> >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::String, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::String> >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::~HashTable (
    this=0x7ffd4b963c78, __in_chrg=<optimized out>)
    at ../../Source/WTF/wtf/HashTable.h:359
#8  0x00007fc9ae31dfda in WTF::HashMap<WTF::String, WTF::String, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::String> >::~HashMap (
    this=0x7ffd4b963c78, __in_chrg=<optimized out>)
    at ../../Source/WTF/wtf/HashMap.h:36
#9  0x00007fc9ae54cf6a in WebCore::SelectionData::~SelectionData (
    this=0x7ffd4b963c10, __in_chrg=<optimized out>)
    at ../../Source/WebCore/platform/gtk/SelectionData.h:29
#10 0x00007fc9aeefd4c3 in WebCore::SelectionRangeData::apply (
    this=0x7fc94ecf03f0, newSelection=..., 
    blockRepaintMode=WebCore::SelectionRangeData::RepaintMode::NewXOROld)
    at ../../Source/WebCore/rendering/SelectionRangeData.cpp:261
#11 0x00007fc9aeefc0da in WebCore::SelectionRangeData::set (
    this=0x7fc94ecf03f0, selection=..., 
    blockRepaintMode=WebCore::SelectionRangeData::RepaintMode::NewXOROld)
    at ../../Source/WebCore/rendering/SelectionRangeData.cpp:169
#12 0x00007fc9b0089d3e in WebCore::FrameSelection::updateAppearance (
    this=0x7fc9929be640)
    at ../../Source/WebCore/editing/FrameSelection.cpp:2117
#13 0x00007fc9b0081700 in WebCore::FrameSelection::updateAndRevealSelection (
    this=0x7fc9929be640, intent=...)
    at ../../Source/WebCore/editing/FrameSelection.cpp:400
#14 0x00007fc9b00815d6 in WebCore::FrameSelection::setSelection (
    this=0x7fc9929be640, selection=..., options=6, intent=..., 
    align=WebCore::FrameSelection::AlignCursorOnScrollIfNeeded, 
    granularity=WebCore::CharacterGranularity)
    at ../../Source/WebCore/editing/FrameSelection.cpp:369
#15 0x00007fc9b0080405 in WebCore::FrameSelection::moveTo (
    this=0x7fc9929be640, base=..., extent=..., affinity=WebCore::DOWNSTREAM, 
    userTriggered=WebCore::NotUserTriggered)
    at ../../Source/WebCore/editing/FrameSelection.cpp:178
#16 0x00007fc9b047a840 in WebCore::DOMSelection::setBaseAndExtent (
    this=0x7fc9929f9aa0, baseNode=0x7fc9929bf750, baseOffset=0, 
    extentNode=0x7fc9929bf750, extentOffset=1)
    at ../../Source/WebCore/page/DOMSelection.cpp:215
#17 0x00007fc9b047bbcf in WebCore::DOMSelection::selectAllChildren (
    this=0x7fc9929f9aa0, node=...)
    at ../../Source/WebCore/page/DOMSelection.cpp:428
#18 0x00007fc9b0a6ab19 in WebCore::jsDOMSelectionPrototypeFunctionSelectAllChildrenBody (state=0x7ffd4b964440, castedThis=0x7fc94b8d84e0, throwScope=...)
    at DerivedSources/WebCore/JSDOMSelection.cpp:477
#19 0x00007fc9b0a74858 in WebCore::IDLOperation<WebCore::JSDOMSelection>::call<WebCore::jsDOMSelectionPrototypeFunctionSelectAllChildrenBody> (state=..., 
    operationName=0x7fc9b2a3555f "selectAllChildren")
    at ../../Source/WebCore/bindings/js/JSDOMOperation.h:53
#20 0x00007fc9b0a6ab47 in WebCore::jsDOMSelectionPrototypeFunctionSelectAllChildren (state=0x7ffd4b964440) at DerivedSources/WebCore/JSDOMSelection.cpp:483
#21 0x00007fc950d2d028 in ?? ()
#22 0x00007ffd4b9644b0 in ?? ()
#23 0x00007fc9a4ea226a in llint_entry ()
   from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME/lib/libjavascriptcoregtk-4.0.so.18

The problem appears to be a name collision between WebCore::SelectionData and WebCore::SelectionData. Yes, those are the same names. I don't understand how exactly it's happening, but it seems the destructor for a Source/WebCore/platform/gtk/SelectionData.h SelectionData is being called on a Source/WebCore/rendering/SelectionRangeData.cpp SelectionData. Yikes.

[Michael Catanzaro]

This is probably to blame for the huge amount of the API and layout test crashes we're seeing on the debug bot right now.

[Michael Catanzaro]

Fix is trivial, but note that TestWebViewEditor will still crash after this due to bug #151654.

[Michael Catanzaro]

Created attachment 324957 [details]
Patch

[Carlos Garcia Campos]

Adding rev info, so that I know I don't need to merge this in stable branch.

[Carlos Garcia Campos]

Comment on attachment 324957 [details]
Patch

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

> Source/WebCore/rendering/SelectionRangeData.cpp:48
> +namespace { // See bug #177808.
> +
>  struct SelectionData {

Since this is only used internally in this file, maybe we can just rename it, SelectionInfo, SelectionContext or whatever. Another possibility would be to move gtk SelectionData to PAL, but it depends on URL and Image.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #6)
> Comment on attachment 324957 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324957&action=review
> 
> > Source/WebCore/rendering/SelectionRangeData.cpp:48
> > +namespace { // See bug #177808.
> > +
> >  struct SelectionData {
> 
> Since this is only used internally in this file, maybe we can just rename
> it, SelectionInfo, SelectionContext or whatever. 

There's no need for that: SelectionData is a fine name, and classes declared in source files should be in an anonymous namespace anyway (for exactly this reason). Renaming GTK's SelectionData to PasteboardSelectionData might make sense, though.

> Another possibility would
> be to move gtk SelectionData to PAL, but it depends on URL and Image.

Yes, all the pasteboard code should be moved down to PAL. But I don't want to work on that today.

[WebKit Commit Bot]

Comment on attachment 324957 [details]
Patch

Clearing flags on attachment: 324957

Committed r224087: <https://trac.webkit.org/changeset/224087>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.