NEW220340
[GTK] UI process crash in WebKit::IconDatabase::iconIDForIconURL 
https://bugs.webkit.org/show_bug.cgi?id=220340
Summary [GTK] UI process crash in WebKit::IconDatabase::iconIDForIconURL
Michael Catanzaro
Reported 2021-01-05 14:31:32 PST
Looks like we may still have safety issues in IconDatabase. Maybe we should be using mutexes to guard our use of WebCore::SQLiteStatement objects? (Maybe even the WebCore::SQLiteDatabase, though that doesn't seem to be the problem here?) All sqlite objects *should* be safe to use cross-threads since we assume sqlite uses serialized threading mode, documented at https://www.sqlite.org/threadsafe.html. But I'm not sure it's really true. (I remember we discovered sqlite3_initialize() was not actually threadsafe in the past, contrary to its documentation, see bug #143245.) And it sure looks like our WebCore wrappers, like WebCore::SQLiteStatement, are not safe. So we may need to lock these. Program terminated with signal SIGSEGV, Segmentation fault. #0 __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:383 383 VMOVU (%rsi), %VEC(4) [Current thread is 1 (Thread 0x7f76167fa700 (LWP 13))] (gdb) bt full #0 __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:383 #1 0x00007f7668a46f6b in memcpy (__len=1540, __src=0xbab0000a64695f80, __dest=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34 nAlloc = 1540 nByte = 1540 iLimit = <optimized out> flags = <optimized out> #2 sqlite3VdbeMemSetStr (pMem=pMem@entry=0x7f75fc00f5c0, z=z@entry=0xbab0000a64695f80 <error: Cannot access memory at address 0xbab0000a64695f80>, n=n@entry=1540, enc=enc@entry=2 '\002', xDel=xDel@entry=0xffffffffffffffff) at sqlite3.c:11213 nAlloc = 1540 nByte = 1540 iLimit = <optimized out> flags = <optimized out> #3 0x00007f7668a47b42 in bindText (encoding=<optimized out>, xDel=0xffffffffffffffff, nData=1540, zData=0xbab0000a64695f80, i=<optimized out>, pStmt=0x7f75fc00e900) at sqlite3.c:84167 pVar = 0x7f75fc00f5c0 rc = <optimized out> p = 0x7f75fc00e900 rc = 0 #4 bindText (pStmt=0x7f75fc00e900, i=<optimized out>, zData=0xbab0000a64695f80, nData=1540, xDel=0xffffffffffffffff, encoding=<optimized out>) at sqlite3.c:18615 p = 0x7f75fc00e900 rc = 0 #5 0x00007f766c518184 in WebCore::SQLiteStatement::bindText(int, WTF::String const&) (this=0x7f765edca048, index=index@entry=1, text=...) at DerivedSources/ForwardingHeaders/wtf/text/StringImpl.h:280 upconvertedCharacters = {m_upconvertedCharacters = {<WTF::VectorBuffer<char16_t, 32, WTF::FastMalloc>> = {<WTF::VectorBufferBase<char16_t, WTF::FastMalloc>> = {m_buffer = 0x7f76167f9700 u"\x2e60ﰀ罵", m_capacity = 32, m_size = 0}, m_inlineBuffer = {{__data = "`.", __align = {<No data fields>}}, {__data = <incomplete sequence \374>, __align = {<No data fields>}}, {__data = "u\177", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = <incomplete sequence \351>, __align = {<No data fields>}}, {__data = <incomplete sequence \374>, __align = {<No data fields>}}, {__data = "u\177", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "`.", __align = {<No data fields>}}, {__data = <incomplete sequence \374>, __align = {<No data fields>}}, {__data = "u\177", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\311", <incomplete sequence \340>, __align = {<No data fields>}}, {__data = "\242h", __align = {<No data fields>}}, {__data = "v\177", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}, {__data = "\324/", __align = {<No data fields>}}, {__data = "\033n", __align = {<No data fields>}}, {__data = "v\177", __align = {<No data fields>}}, {__data = "\000", __align = {<No data fields>}}}}, <No data fields>}, m_characters = 0xbab0000a64695f80 <error: Cannot access memory at address 0xbab0000a64695f80>} anyCharacter = 0 u'\000' characters = <optimized out> #6 0x00007f766af81634 in WebKit::IconDatabase::iconIDForIconURL(WTF::String const&, bool&) (this=0x7f765edd1000, iconURL=..., expired=@0x7f76167f9817: false) at /usr/include/c++/10.2.0/bits/unique_ptr.h:421 result = <optimized out> --Type <RET> for more, q to quit, c to continue without paging--c #7 0x00007f766af850f8 in operator() (__closure=0x7f74a0e9cc10) at ../Source/WebKit/UIProcess/API/glib/IconDatabase.cpp:560 expired = false canWriteToDatabase = <optimized out> iconID = {<WTF::constexpr_Optional_base<long>> = {init_ = false, storage_ = {dummy_ = 0 '\000', value_ = 0}}, <No data fields>} iconData = {<WTF::VectorBuffer<char, 0, WTF::FastMalloc>> = {<WTF::VectorBufferBase<char, WTF::FastMalloc>> = {m_buffer = 0x0, m_capacity = 0, m_size = 0}, <No data fields>}, <No data fields>} iconURL = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <optimized out>, m_ptr = 0x7f7615c35f18}} this = 0x7f765edd1000 completionHandler = {m_function = {m_callableWrapper = std::unique_ptr<WTF::Detail::CallableWrapperBase<void, WTF::RefPtr<_cairo_surface, WTF::DumbPtrTraits<_cairo_surface> >&&>> = {get() = 0x7f74a0ed8bd0}}} timestamp = {m_value = 1609883696.515743} allowDatabaseWrite = WebKit::IconDatabase::AllowDatabaseWrite::Yes pageURL = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <optimized out>, m_ptr = 0x7f74a0d638b8}} protectedThis = {static isRef = <optimized out>, m_ptr = 0x7f765edd1000} #8 WTF::Detail::CallableWrapper<WebKit::IconDatabase::loadIconForPageURL(const WTF::String&, WebKit::IconDatabase::AllowDatabaseWrite, WTF::CompletionHandler<void(WTF::RefPtr<_cairo_surface>&&)>&&)::<lambda()>, void>::call(void) (this=0x7f74a0e9cc08) at DerivedSources/ForwardingHeaders/wtf/Function.h:52 #9 0x00007f766a103973 in WTF::Function<void ()>::operator()() const (this=<synthetic pointer>) at ../Source/WTF/wtf/Function.h:80 didSuspendFunctions = false #10 WTF::RunLoop::performWork() (this=0x7f765edcf000) at ../Source/WTF/wtf/RunLoop.cpp:119 didSuspendFunctions = false #11 0x00007f766a1512cd in operator() (userData=<optimized out>, __closure=0x0) at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:80 #12 _FUN(gpointer) () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:82 #13 0x00007f766a151bf3 in operator() (__closure=0x0, userData=0x7f765edcf000, callback=0x7f766a1512c0 <_FUN(gpointer)>, source=0x7f75fc001b30) at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:53 name = 0x7f75fc0018e0 "[WebKit] RunLoop work" runLoopSource = @0x7f75fc001b30: {source = {callback_data = 0x7f75fc001fd0, callback_funcs = 0x7f766e53a280 <g_source_callback_funcs>, source_funcs = 0x7f766a4d1f20 <WTF::RunLoop::s_runLoopSourceFunctions>, ref_count = 3, context = 0x7f75fc000b60, priority = 100, flags = 35, source_id = 1, poll_fds = 0x0, prev = 0x0, next = 0x0, name = 0x7f75fc0018e0 "[WebKit] RunLoop work", priv = 0x7f75fc001c00}, runLoop = 0x7f765edcf000} returnValue = <optimized out> #14 _FUN(GSource*, GSourceFunc, gpointer) () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:56 #15 0x00007f766e45b4af in g_main_dispatch (context=0x7f75fc000b60) at ../glib/gmain.c:3337 dispatch = 0x7f766a151ba0 <_FUN(GSource*, GSourceFunc, gpointer)> prev_source = 0x0 begin_time_nsec = 0 was_in_call = 0 user_data = 0x7f765edcf000 callback = 0x7f766a1512c0 <_FUN(gpointer)> cb_funcs = <optimized out> cb_data = 0x7f75fc001fd0 need_destroy = <optimized out> source = 0x7f75fc001b30 current = 0x7f75fc002d40 i = 0 __func__ = "g_main_dispatch" #16 g_main_context_dispatch (context=0x7f75fc000b60) at ../glib/gmain.c:4055 #17 0x00007f766e45b858 in g_main_context_iterate (context=0x7f75fc000b60, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4131 max_priority = 100 timeout = 0 some_ready = 1 nfds = <optimized out> allocated_nfds = <optimized out> fds = 0x7f75fc002c90 #18 0x00007f766e45bb73 in g_main_loop_run (loop=loop@entry=0x7f75fc001b10) at ../glib/gmain.c:4329 __func__ = "g_main_loop_run" #19 0x00007f766a151d50 in WTF::RunLoop::run() () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:108 runLoop = @0x7f765edcf000: {<WTF::FunctionDispatcher> = {<WTF::ThreadSafeRefCounted<WTF::FunctionDispatcher, (WTF::DestructionThread)0>> = {<WTF::ThreadSafeRefCountedBase> = {m_refCount = {<std::__atomic_base<unsigned int>> = {static _S_alignment = 4, _M_i = 2}, static is_always_lock_free = true}}, <No data fields>}, _vptr.FunctionDispatcher = 0x7f766a49e678 <vtable for WTF::RunLoop+16>}, m_currentIteration = {m_start = 10, m_end = 52, m_buffer = {<WTF::VectorBufferBase<WTF::Function<void()>, WTF::FastMalloc>> = {m_buffer = 0x7f750b480bd0, m_capacity = 54, m_size = 0}, <No data fields>}}, m_nextIterationLock = {static isHeldBit = 1 '\001', static hasParkedBit = 2 '\002', m_byte = {value = {<std::__atomic_base<unsigned char>> = {static _S_alignment = 1, _M_i = 0 '\000'}, static is_always_lock_free = true}}}, m_nextIteration = {m_start = 0, m_end = 0, m_buffer = {<WTF::VectorBufferBase<WTF::Function<void()>, WTF::FastMalloc>> = {m_buffer = 0x0, m_capacity = 0, m_size = 0}, <No data fields>}}, m_isFunctionDispatchSuspended = false, m_hasSuspendedFunctions = false, static s_runLoopSourceFunctions = {prepare = 0x0, check = 0x0, dispatch = 0x7f766a151ba0 <_FUN(GSource*, GSourceFunc, gpointer)>, finalize = 0x0, closure_callback = 0x0, closure_marshal = 0x0}, m_mainContext = {m_ptr = 0x7f75fc000b60}, m_mainLoops = {<WTF::VectorBuffer<WTF::GRefPtr<_GMainLoop>, 0, WTF::FastMalloc>> = {<WTF::VectorBufferBase<WTF::GRefPtr<_GMainLoop>, WTF::FastMalloc>> = {m_buffer = 0x7f765edce000, m_capacity = 16, m_size = 1}, <No data fields>}, <No data fields>}, m_source = {m_ptr = 0x7f75fc001b30}, m_observers = {m_set = {m_impl = {static smallMaxLoadNumerator = <optimized out>, static smallMaxLoadDenominator = <optimized out>, static largeMaxLoadNumerator = <optimized out>, static largeMaxLoadDenominator = <optimized out>, static maxSmallTableCapacity = <optimized out>, static minLoad = <optimized out>, static tableSizeOffset = <optimized out>, static tableSizeMaskOffset = <optimized out>, static keyCountOffset = <optimized out>, static deletedCountOffset = <optimized out>, static metadataSize = <optimized out>, m_table = 0x0}}}} mainContext = 0x7f75fc000b60 innermostLoop = 0x7f75fc001b10 nestedMainLoop = <optimized out> #20 0x00007f766a10513d in WTF::Function<void ()>::operator()() const (this=<synthetic pointer>) at ../Source/WTF/wtf/Function.h:80 function = {m_callableWrapper = std::unique_ptr<WTF::Detail::CallableWrapperBase<void>> = {get() = 0x7f765edf5300}} #21 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (newThreadContext=0x7f765edec1b0) at ../Source/WTF/wtf/Threading.cpp:171 function = {m_callableWrapper = std::unique_ptr<WTF::Detail::CallableWrapperBase<void>> = {get() = 0x7f765edf5300}} #22 0x00007f766a153f7d in WTF::wtfThreadEntryPoint(void*) (context=<optimized out>) at ../Source/WTF/wtf/posix/ThreadingPOSIX.cpp:213 #23 0x00007f766840f4d2 in start_thread (arg=<optimized out>) at pthread_create.c:477 ret = <optimized out> pd = <optimized out> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140145160333056, -7816065943494804149, 140727170437838, 140727170437839, 140145160330048, 8396800, 7884656778560322891, 7884415288411995467}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = 0 #24 0x00007f766e1c22e3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Attachments
Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2021-01-05 14:33:39 PST
Also I'm nervous that we create WebCore::SQLiteStatements on WorkQueue threads without guarding their initialization. We should be using std::call_once to ensure we don't accidentally wind up creating two. (But again, that seems unlikely to result in this particular crash.)
Note You need to log in before you can comment on or make changes to this bug.