Bug 162139 – REGRESSION(r194387): Crash on github.com in IntlDateTimeFormat::resolvedOptions in C locale

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Archlinux Bug]

Visiting any page on github.com with 2.13.92 result in segmentation fault once the page is mostly rendered.
Build arguments

cmake -DPORT=GTK -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_SKIP_RPATH=ON -DCMAKE_INSTALL_PREFIX=/usr \
        -DLIB_INSTALL_DIR=/usr/lib -DLIBEXEC_INSTALL_DIR=/usr/lib/webkit2gtk-4.0 \
        -DENABLE_GTKDOC=OFF -DENABLE_MEDIA_SOURCE=ON -DENABLE_VIDEO_TRACK=ON -DENABLE_VIDEO=ON

[Michael Catanzaro]

We can't reproduce it, got a backtrace?

[Archlinux Bug]

It crashes whole browser on Wayland, but only crashes tab on X11.

locale
LANG=C
LC_CTYPE="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_COLLATE="C"
LC_MONETARY="C"
LC_MESSAGES="C"
LC_PAPER="C"
LC_NAME="C"
LC_ADDRESS="C"
LC_TELEPHONE="C"
LC_MEASUREMENT="C"
LC_IDENTIFICATION="C"
LC_ALL=C


Process 1817 (WebKitWebProces) dumped core.
                                                
                                                Stack trace of thread 1817:
                                                #0  0x00007f9f6577b4b1 _ZN3JSC18IntlDateTimeFormat15resolvedOptionsERNS_9ExecStateE (libjavascriptcoregtk-4.0.so.18)
                                                #1  0x00007f9f6577ed09 _ZN3JSCL46IntlDateTimeFormatPrototypeFuncResolvedOptionsEPNS_9ExecStateE (libjavascriptcoregtk-4.0.so.
                                                #2  0x00007f9f03fff028 n/a (n/a)
Sep 19 23:07:05 systemd-coredump[1874]: 



Process 1796 (epiphany) dumped core.
                                                
                                                Stack trace of thread 1796:
                                                #0  0x00007f2d0bcac23e _ZNK6WebKit17WaylandCompositor6Buffer4sizeEv (libwebkit2gtk-4.0.so.37)
                                                #1  0x00007f2d0bcac913 _ZN6WebKit17WaylandCompositor7Surface25prepareTextureForPaintingERjRN7WebCore7IntSizeE (libwebkit2gtk-
                                                #2  0x00007f2d0bca4f65 _ZN6WebKit30AcceleratedBackingStoreWayland5paintEP6_cairoRKN7WebCore7IntRectE (libwebkit2gtk-4.0.so.37
                                                #3  0x00007f2d0bc8c114 _ZL21webkitWebViewBaseDrawP10_GtkWidgetP6_cairo (libwebkit2gtk-4.0.so.37)
                                                #4  0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #5  0x00007f2d0ad84260 gtk_container_propagate_draw (libgtk-3.so.0)
                                                #6  0x00007f2d0ad84342 n/a (libgtk-3.so.0)
                                                #7  0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #8  0x00007f2d0ad84260 gtk_container_propagate_draw (libgtk-3.so.0)
                                                #9  0x00007f2d0ae7c200 n/a (libgtk-3.so.0)
                                                #10 0x00007f2d0ad8930d n/a (libgtk-3.so.0)
                                                #11 0x00007f2d0ad8df70 n/a (libgtk-3.so.0)
                                                #12 0x00007f2d0ae7c011 n/a (libgtk-3.so.0)
                                                #13 0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #14 0x00007f2d0ad84260 gtk_container_propagate_draw (libgtk-3.so.0)
                                                #15 0x00007f2d0ad84342 n/a (libgtk-3.so.0)
                                                #16 0x00007f2d0ad38fa4 n/a (libgtk-3.so.0)
                                                #17 0x00007f2d0ad8930d n/a (libgtk-3.so.0)
                                                #18 0x00007f2d0ad8df70 n/a (libgtk-3.so.0)
                                                #19 0x00007f2d0ad3b941 n/a (libgtk-3.so.0)
                                                #20 0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #21 0x00007f2d0ad84260 gtk_container_propagate_draw (libgtk-3.so.0)
                                                #22 0x00007f2d0ae6e2ba n/a (libgtk-3.so.0)
                                                #23 0x00007f2d0ad8930d n/a (libgtk-3.so.0)
                                                #24 0x00007f2d0ad8df70 n/a (libgtk-3.so.0)
                                                #25 0x00007f2d0ad3d074 n/a (libgtk-3.so.0)
                                                #26 0x00007f2d0ad8df70 n/a (libgtk-3.so.0)
                                                #27 0x00007f2d0ae70a0c n/a (libgtk-3.so.0)
                                                #28 0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #29 0x00007f2d0ad84260 gtk_container_propagate_draw (libgtk-3.so.0)
                                                #30 0x00007f2d0ad84342 n/a (libgtk-3.so.0)
                                                #31 0x00007f2d0af9ea11 n/a (libgtk-3.so.0)
                                                #32 0x00007f2d0af910ab n/a (libgtk-3.so.0)
                                                #33 0x00007f2d0af91589 gtk_widget_send_expose (libgtk-3.so.0)
                                                #34 0x00007f2d0ae43925 gtk_main_do_event (libgtk-3.so.0)
                                                #35 0x00007f2d0a97a785 n/a (libgdk-3.so.0)
                                                #36 0x00007f2d0a989478 n/a (libgdk-3.so.0)
                                                #37 0x00007f2d0a98a5cc n/a (libgdk-3.so.0)
                                                #38 0x00007f2d0a98a783 n/a (libgdk-3.so.0)
                                                #39 0x00007f2d092ca1d4 n/a (libgobject-2.0.so.0)
                                                #40 0x00007f2d092e490d g_signal_emit_valist (libgobject-2.0.so.0)
                                                #41 0x00007f2d092e4fff g_signal_emit (libgobject-2.0.so.0)
                                                #42 0x00007f2d0a982711 n/a (libgdk-3.so.0)
                                                #43 0x00007f2d0a970ae8 n/a (libgdk-3.so.0)
                                                #44 0x00007f2d08112703 n/a (libglib-2.0.so.0)
                                                #45 0x00007f2d08111c8a g_main_context_dispatch (libglib-2.0.so.0)
                                                #46 0x00007f2d08112040 n/a (libglib-2.0.so.0)
                                                #47 0x00007f2d081120ec g_main_context_iteration (libglib-2.0.so.0)
                                                #48 0x00007f2d095b9d2d g_application_run (libgio-2.0.so.0)
                                                #49 0x00000000004023eb main (epiphany)
                                                #50 0x00007f2d07d4a291 __libc_start_main (libc.so.6)
                                                #51 0x000000000040290a _start (epiphany)

[Michael Catanzaro]

Please, use 'coredumpctl gdb' and then 'bt full' to get a good backtrace. You also need to install debuginfo for GLib and GTK+, or build them yourself with debuginfo if Arch doesn't provide any.

Since it's crashing the UI process under Wayland, it would be interesting to see a backtrace of that process as well; a web process crash should never crash the UI process, so there are multiple bugs here.

[Archlinux Bug]

It crashes if locale is set to C. 
There is no crash if I start using LC_ALL=en_US.UTF-8 epiphany. And adding a check in IntlDateTimeFormat also fixes the issue.

if(!m_locale.isEmpty() ) {
   options->putDirect(vm, vm.propertyNames->locale, jsNontrivialString(&exec, m_locale));
}

[Michael Catanzaro]

OK, most likely a regression from "[INTL] Implement Intl.DateTimeFormat.prototype.resolvedOptions ()"

[Carlos Garcia Campos]

Created attachment 289442 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 289442 [details]
Patch

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

> LayoutTests/js/intl-invalid-locale-crash.html:10
> +if (window.internals) {
> +    window.internals.setUserPreferredLanguages(["a"]);

Please add a comment to indicate that you're doing this to ensure the script gets run with an invalid locale.

> Source/JavaScriptCore/runtime/IntlCollator.cpp:261
> +        throwTypeError(&state, scope, ASCIILiteral("failed to initialize Collator"));

Wouldn't it be good to indicate why? "failed to initialize Collator due to invalid locale"?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:976
> +    for (size_t i = 0; languageList[i]; ++i) {
> +        // Dot not propagate the C locale to WebCore.

"Do not"

And since you changed the line above anyway, I would change size_t to auto as well.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:-979
> -    WebCore::languageDidChange();

What, why dropping this? This looks like an accident. Please don't commit without responding to this.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:491
> +    // When using the C locale, en-US should be used as default.

I would copy/paste this entire block to add a test for POSIX locale as well.

[Carlos Garcia Campos]

(In reply to comment #8)
> Comment on attachment 289442 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289442&action=review
> 
> > LayoutTests/js/intl-invalid-locale-crash.html:10
> > +if (window.internals) {
> > +    window.internals.setUserPreferredLanguages(["a"]);
> 
> Please add a comment to indicate that you're doing this to ensure the script
> gets run with an invalid locale.

Ah, ok, I assumed the test name was enough.

> > Source/JavaScriptCore/runtime/IntlCollator.cpp:261
> > +        throwTypeError(&state, scope, ASCIILiteral("failed to initialize Collator"));
> 
> Wouldn't it be good to indicate why? "failed to initialize Collator due to
> invalid locale"?

Ok.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:976
> > +    for (size_t i = 0; languageList[i]; ++i) {
> > +        // Dot not propagate the C locale to WebCore.
> 
> "Do not"

Oops.

> And since you changed the line above anyway, I would change size_t to auto
> as well.

What would auto deduce for 0? int? I prefer to be explicit here, we want an unsigned type.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:-979
> > -    WebCore::languageDidChange();
> 
> What, why dropping this? This looks like an accident. Please don't commit
> without responding to this.

No, but I forgot to comment about this in the changelog, WebCore::overrideUserPreferredLanguages() already calls languageDidChange()), so we were calling all the observers twice.

> > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:491
> > +    // When using the C locale, en-US should be used as default.
> 
> I would copy/paste this entire block to add a test for POSIX locale as well.

Good point.

[Carlos Garcia Campos]

I'll submit a new patch for landing, but I would like to hear from any JSC developer before, could someone confirm this approach is ok?

[Carlos Garcia Campos]

Created attachment 289551 [details]
Patch for landing

[Michael Catanzaro]

(In reply to comment #9)
> What would auto deduce for 0? int? I prefer to be explicit here, we want an
> unsigned type.

Oops, probably

[Carlos Garcia Campos]

Committed r206295: <http://trac.webkit.org/changeset/206295>