[Webkit-unassigned] [Bug 173407] New: WTF::StringImpl::copyChars segfaults when built with GCC 7

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 15 02:04:00 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=173407

            Bug ID: 173407
           Summary: WTF::StringImpl::copyChars segfaults when built with
                    GCC 7
           Product: WebKit
           Version: Other
          Hardware: Unspecified
                OS: Linux
            Status: NEW
          Severity: Major
          Priority: P2
         Component: Web Template Framework
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: webkit.org at the-compiler.org

I haven't tested this with WebKit trunk so far, only with https://github.com/annulen/webkit and WebKit2GTK 2.16.3 - but since the code in trunk looks the same, I'm guessing this applies too.

After updating my packages on Archlinux, I noticed a lot of segfaults when navigating to pages, or e.g. when submitting a comment on Reddit.

I was able to minimize it to this example which crashes when run in jsc:

    s = 'xxxxxxxxxxxxxxAxxxxxxxxxxxxxxxxxxxxA–'; s.replace(/A/g, 'b')

(note the non-ASCII – at the end)

Stack (with the original failure on reddit, with QtWebKit):

    #0  0x00007fffed413b65 in WTF::StringImpl::copyChars<char16_t>(char16_t*, char16_t const*, unsigned int) (destination=0x7ffefa9e0250 u"<p ", source=0x7fff1baf0256 u"<p class=\"parent\"><a name=\"dircqkb\"></a></p><div class=\"midcol likes\" ><div class=\"arrow upmod login-required access-required\" data-event-action=\"upvote\" role=\"button\""..., numCharacters=20) at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/WTF/wtf/text/StringImpl.h:634
    #1  0x00007fffed420926 in JSC::jsSpliceSubstringsWithSeparators (separatorCount=251, separators=<optimized out>, rangeCount=<optimized out>, substringRanges=<optimized out>, source=..., sourceVal=<optimized out>, exec=0x7fffffffc6f0, this=<optimized out>)
        at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:431
    #2  0x00007fffed420926 in JSC::replaceUsingRegExpSearch (replaceValue=..., replacementString=..., callType=<optimized out>, callData=..., searchValue=..., string=<optimized out>, exec=<optimized out>)
        at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:666
    #3  0x00007fffed420926 in JSC::replaceUsingRegExpSearch (replaceValue=..., searchValue=..., string=<optimized out>, exec=<optimized out>) at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:708
    #4  0x00007fffed420926 in JSC::replace(JSC::ExecState*, JSC::JSString*, JSC::JSValue, JSC::JSValue) (replaceValue=..., searchValue=..., string=<optimized out>, exec=<optimized out>) at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:829
    #5  0x00007fffed420926 in JSC::replace(JSC::ExecState*, JSC::JSValue, JSC::JSValue, JSC::JSValue) (replaceValue=..., searchValue=..., thisValue=..., exec=<optimized out>) at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:841
    #6  0x00007fffed420926 in JSC::stringProtoFuncReplace(JSC::ExecState*) (exec=<optimized out>) at /tmp/makepkg/qt5-webkit-ng-debug/src/webkit-qtwebkit-5.212.0-alpha/Source/JavaScriptCore/runtime/StringPrototype.cpp:846
    #7  0x00007fff1bfff0c8 in  ()
    #8  0x00007fffffffc7b0 in  ()
    #9  0x00007fffed5c930d in llint_entry () at /usr/lib/libQt5WebKit.so.5

Stack (with jsc and WebKit2GTK, release build):

    #0  0x00007ffff7ac2c95 in void WTF::StringImpl::copyChars<char16_t>(char16_t*, char16_t const*, unsigned int) () at /usr/lib/libjavascriptcoregtk-4.0.so.18
    #1  0x00007ffff7ac1d5e in JSC::stringProtoFuncReplaceUsingRegExp(JSC::ExecState*) ()
        at /usr/lib/libjavascriptcoregtk-4.0.so.18
    #2  0x00007fffb21ff028 in  ()
    #3  0x00007fffffffcfc0 in  ()
    #4  0x00007ffff77a4e42 in  () at /usr/lib/libjavascriptcoregtk-4.0.so.18
    #5  0x0000000000000000 in  ()

So I looked into the implementation of WTF::StringImpl::copyChars:

    template <typename T> static void copyChars(T* destination, const T* source, unsigned numCharacters)
    {
        if (numCharacters == 1) {
            *destination = *source;
            return;
        }

        if (numCharacters <= s_copyCharsInlineCutOff) {
            unsigned i = 0;
#if (CPU(X86) || CPU(X86_64))
            const unsigned charsPerInt = sizeof(uint32_t) / sizeof(T);

            if (numCharacters > charsPerInt) {
                unsigned stopCount = numCharacters & ~(charsPerInt - 1);

                const uint32_t* srcCharacters = reinterpret_cast<const uint32_t*>(source);  // aliasing
                uint32_t* destCharacters = reinterpret_cast<uint32_t*>(destination);  // aliasing
                for (unsigned j = 0; i < stopCount; i += charsPerInt, ++j)
                    destCharacters[j] = srcCharacters[j];   // CRASH
            }
#endif
            for (; i < numCharacters; ++i)
                destination[i] = source[i];
        } else
            memcpy(destination, source, numCharacters * sizeof(T));
    }

Removing the optimized #if block made things run fine. With some help from people in the #gcc IRC channel, I figured out that WebKit violates strict aliasing rules with the casts marked above (as T is an uint16_t, incompatible with uint32_t):

https://www.cocoawithlove.com/2008/04/using-pointers-to-recast-in-c-is-bad.html
http://dbp-consulting.com/tutorials/StrictAliasing.html

WebKit should be built with -fno-strict-aliasing though, and at least with QtWebKit I verified this was the case. From what I understand, that should make it possible to do this kind of thing, yet with GCC 7.1.1 I get a crash there.

I haven't looked at the generated assembler yet (and not sure I'll get to it anytime soon), but this might be a GCC 7 bug?

But even if it's fixed there, I suppose WebKit should work around it in some way. I wonder whether the optimized inline copyChars is really needed nowadays (it was added in 2011, https://github.com/annulen/webkit/commit/0cb990be04a30870abb2a01e7f9fe5eea08b25a9 ) - I'd expect an inline memcpy() to generate equal if not better code nowadays, no?

Downstream QtWebKit bug where I investigated this: https://github.com/annulen/webkit/issues/562

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170615/5c397241/attachment-0001.html>


More information about the webkit-unassigned mailing list