Bug 177808

Summary: REGRESSION(r222697): [GTK] Crash in WebCore::SelectionRangeData::apply
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, darin, mcatanzaro, zalan
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 168219    
Attachments:
Description Flags
Backtrace
none
Patch none

Description Michael Catanzaro 2017-10-03 06:06:38 PDT
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.
Comment 1 Michael Catanzaro 2017-10-25 20:50:43 PDT
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.
Comment 2 Michael Catanzaro 2017-10-25 21:44:43 PDT
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.
Comment 3 Michael Catanzaro 2017-10-25 21:50:44 PDT
Fix is trivial, but note that TestWebViewEditor will still crash after this due to bug #151654.
Comment 4 Michael Catanzaro 2017-10-25 21:57:12 PDT
Created attachment 324957 [details]
Patch
Comment 5 Carlos Garcia Campos 2017-10-25 22:52:14 PDT
Adding rev info, so that I know I don't need to merge this in stable branch.
Comment 6 Carlos Garcia Campos 2017-10-25 22:56:15 PDT
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.
Comment 7 Michael Catanzaro 2017-10-26 03:56:10 PDT
(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.
Comment 8 WebKit Commit Bot 2017-10-27 00:04:54 PDT
Comment on attachment 324957 [details]
Patch

Clearing flags on attachment: 324957

Committed r224087: <https://trac.webkit.org/changeset/224087>
Comment 9 WebKit Commit Bot 2017-10-27 00:04:56 PDT
All reviewed patches have been landed.  Closing bug.