Bug 274964 – [WPE][GTK] g_variant_builder_add_value: assertion 'GVSB(builder)->offset < GVSB(builder)->max_items' failed when encoding session state

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

Created attachment 471555 [details]
Full backtrace

My Epiphany Tech Preview (WebKitGTK 2.45.3) UI process just crashed twice here:

#0  g_logv (log_domain=0x7f0de3b9100e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7f0d373ff480)
    at ../glib/gmessages.c:1277
#1  0x00007f0de3b37a93 in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at ../glib/gmessages.c:1315
#2  0x00007f0de3b7b7a1 in g_variant_builder_add (builder=0x7f0d373ff750, format_string=<optimized out>) at ../glib/gvariant.c:5647
#3  0x00007f0dde5e2f8c in encodeHTTPBody (sessionBuilder=0x7f0d373ff750, httpBody=<optimized out>)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:130
#4  encodeFrameState (sessionBuilder=sessionBuilder@entry=0x7f0d373ff750, frameState=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:196
#5  0x00007f0dde5e10c5 in encodePageState (sessionBuilder=0x7f0d373ff750, pageState=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:213
#6  encodeBackForwardListItemState (sessionBuilder=0x7f0d373ff750, item=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:221
#7  encodeBackForwardListState (sessionBuilder=0x7f0d373ff750, backForwardListState=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:229
#8  encodeSessionState (sessionState=...) at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:243
#9  webkit_web_view_session_state_serialize (state=0x7f0dce078120)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp:523
#10 0x00007f0de3d53c38 in write_tab (tab=0x5566306e02c0, writer=0x7f0cd8019870) at ../src/ephy-session.c:751
#11 write_tab (writer=0x7f0cd8019870, tab=0x5566306e02c0) at ../src/ephy-session.c:702
#12 0x00007f0de3d54fc9 in write_ephy_window (window=<optimized out>, writer=<optimized out>) at ../src/ephy-session.c:830
#13 save_session_sync (task=0x55663075c170 [GTask], source_object=<optimized out>, task_data=<optimized out>, cancellable=<optimized out>)
    at ../src/ephy-session.c:954
#14 0x00007f0de399c4d6 in g_task_thread_pool_thread (thread_data=0x55663075c170, pool_data=<optimized out>) at ../gio/gtask.c:1583
#15 0x00007f0de3b5eb82 in g_thread_pool_thread_proxy (data=<optimized out>) at ../glib/gthreadpool.c:336
#16 0x00007f0de3b5dfe9 in g_thread_proxy (data=0x7f0dd0002310) at ../glib/gthread.c:835
#17 0x00007f0de293ce39 in start_thread (arg=<optimized out>) at pthread_create.c:444
#18 0x00007f0de29c49c4 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100

It's a g_critical(), and playing with gdb I was able to print the error message:

g_variant_builder_add_value: assertion 'GVSB(builder)->offset < GVSB(builder)->max_items' failed when encoding session state

It happened after I added two attachments to bug #274802 and might be related to the presence of those attachments in the back/forward list, but I'm not sure. I'm also not sure whether it's a GLib bug or a WebKit bug.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #0)
> It happened after I added two attachments to bug #274802 and might be
> related to the presence of those attachments in the back/forward list

Now it happened again after reporting this bug. Maybe any bug submission is enough to trigger it?

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
> Now it happened again after reporting this bug. Maybe any bug submission is
> enough to trigger it?

Ah no, it must have happened because I attached the "full backtrace" attachment to this bug.

[Michael Catanzaro]

OK, found a reproducer! Create an empty file and attempt to attach it to this bug report. Fill in the description field and click Submit. Bugzilla will display an error message complaining that you're not allowed to attach an empty file. Then the UI process will crash.

(In MiniBrowser it will be just a critical instead of a crash; set G_DEBUG=fatal-criticals to make it crash and get a backtrace.)

[Michael Catanzaro]

So the crash occurs here when encoding the HTTPBody object, specifically the file data:

static inline void encodeHTTPBody(GVariantBuilder* sessionBuilder, const HTTPBody& httpBody)
{
    g_variant_builder_open(sessionBuilder, G_VARIANT_TYPE("(sa" HTTP_BODY_ELEMENT_TYPE_STRING_V1 ")"));
    g_variant_builder_add(sessionBuilder, "s", httpBody.contentType.utf8().data());
    g_variant_builder_open(sessionBuilder, G_VARIANT_TYPE("a" HTTP_BODY_ELEMENT_TYPE_STRING_V1));
    g_variant_builder_open(sessionBuilder, G_VARIANT_TYPE(HTTP_BODY_ELEMENT_TYPE_STRING_V1));
    for (const auto& element : httpBody.elements) {
        g_variant_builder_add(sessionBuilder, "u", toHTMLBodyElementType(element.data.index())); // <-- crash is here

Here we are just encoding the integer index corresponding to the HTMLBodyElementType that we're about to encode. In this case, the index is 1, indicating HTMLBodyElementType::File. The variant element.data contains WebKit::HTTPBody::Element::FileData corresponding to the Bugzilla attachment; in this case, "/run/user/1000/doc/bf7e311e/Empty Document".

But that doesn't actually matter. The crash is occurring because the state of the GVariantBuilder is determined to be already messed up at the beginning of the call to g_variant_builder_add(). Playing with gdb, I found the offset is 7 and max_items is 7. We crash since 7 is not less than 7. Something went wrong in the previous call to either g_variant_builder_add() or g_variant_builder_open().

[Michael Catanzaro]

The problem is the g_variant_builder_open()/close() for constructing an HTTP_BODY_ELEMENT_TYPE_STRING_V1 should be on the inside of the for loop, not the outside. It breaks if there are ever more than 1 HTTP body elements.

[Michael Catanzaro]

Pull request: https://github.com/WebKit/WebKit/pull/29377

[EWS]

Committed 279649@main (86fbb6c7dafe): <https://commits.webkit.org/279649@main>

Reviewed commits have been landed. Closing PR #29377 and removing active labels.