Bug 51734 – [Gtk] null ptr crash in DumpRenderTreeSupportGtk::getInnerText

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Abhishek Arya]

fast/multicol/span/double-merge-anonymous-block-crash.html

Skipping until i do more analysis of this seemingly null ptr crash. Weird, it does not crash on any other platform.

[Martin Robinson]

There are stack traces at http://webkit-bots.igalia.com. Here is the trace for this crash:

Thread 1 (Thread 14962):
#0  0x00007f2be3aa5a5f in WebCore::Node::document (this=0x0) at ../../WebCore/dom/Node.h:350
#1  0x00007f2be3cf87da in WebCore::Element::innerText (this=0x0) at ../../WebCore/dom/Element.cpp:1502
#2  0x00007f2be45033b1 in DumpRenderTreeSupportGtk::getInnerText (frame=0x1527000) at ../../WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:156
#3  0x0000000000419b28 in dumpFramesAsText (frame=0x1527000) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:253
#4  0x000000000041a879 in dump () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:509
#5  0x000000000041b2fc in webViewLoadFinished (view=0x1506010, frame=0x1527000) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:738
#6  0x00007f2be62b4d1e in g_closure_invoke (closure=0x158d240, return_value=0x0, n_param_values=2, param_values=0x7f2b48a5b730, invocation_hint=0x7fffd29d4fe0) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gobject/gclosure.c:766
#7  0x00007f2be62cdd99 in signal_emit_unlocked_R (node=0x14a9330, detail=<value optimized out>, instance=<value optimized out>, emission_return=<value optimized out>, instance_and_params=<value optimized out>) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gobject/gsignal.c:3252
#8  0x00007f2be62cf516 in g_signal_emit_valist (instance=0x1506010, signal_id=<value optimized out>, detail=0, var_args=0x7fffd29d5200) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gobject/gsignal.c:2983
#9  0x00007f2be62cf812 in g_signal_emit_by_name (instance=0x1506010, detailed_signal=<value optimized out>) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gobject/gsignal.c:3077
#10 0x00007f2be450eae6 in WebKit::FrameLoaderClient::postProgressFinishedNotification (this=0x14cda80) at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:386
#11 0x00007f2be3ff220a in WebCore::ProgressTracker::finalProgressComplete (this=0x1513a20) at ../../WebCore/loader/ProgressTracker.cpp:153
#12 0x00007f2be3ff20b7 in WebCore::ProgressTracker::progressCompleted (this=0x1513a20, frame=0x14c38e0) at ../../WebCore/loader/ProgressTracker.cpp:132
#13 0x00007f2be3fbeb22 in WebCore::FrameLoader::checkLoadCompleteForThisFrame (this=0x14c3938) at ../../WebCore/loader/FrameLoader.cpp:2393
#14 0x00007f2be3fbf1b2 in WebCore::FrameLoader::recursiveCheckLoadComplete (this=0x14c3938) at ../../WebCore/loader/FrameLoader.cpp:2501
#15 0x00007f2be3fbf264 in WebCore::FrameLoader::checkLoadComplete (this=0x14c3938) at ../../WebCore/loader/FrameLoader.cpp:2514
#16 0x00007f2be3fbdd3a in WebCore::FrameLoader::finishedLoading (this=0x14c3938) at ../../WebCore/loader/FrameLoader.cpp:2162
#17 0x00007f2be3fed769 in WebCore::MainResourceLoader::didFinishLoading (this=0x7f2b48d9be90, finishTime=0) at ../../WebCore/loader/MainResourceLoader.cpp:457
#18 0x00007f2be3ff9833 in WebCore::ResourceLoader::didFinishLoading (this=0x7f2b48d9be90, finishTime=0) at ../../WebCore/loader/ResourceLoader.cpp:437
#19 0x00007f2be44da163 in WebCore::closeCallback (source=0x1cf5700, res=0x1d45860) at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:813
#20 0x00007f2be634faad in async_ready_close_callback_wrapper (source_object=0x1cf5700, res=0x1d45860, user_data=0x0) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gio/ginputstream.c:484
#21 0x00007f2be6361cd8 in complete_in_idle_cb_for_thread (_data=<value optimized out>) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/gio/gsimpleasyncresult.c:813
#22 0x00007f2be1aaa0b2 in g_main_dispatch (context=0x14a0ee0) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/glib/gmain.c:2440
#23 g_main_context_dispatch (context=0x14a0ee0) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/glib/gmain.c:3013
#24 0x00007f2be1aae778 in g_main_context_iterate (context=0x14a0ee0, block=<value optimized out>, dispatch=<value optimized out>, self=<value optimized out>) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/glib/gmain.c:3091
#25 0x00007f2be1aaec85 in g_main_loop_run (loop=0x7f2b48d7ef70) at /home/kov/debian/tmp/glib2.0-2.27.5.2010128/glib/gmain.c:3299
#26 0x00007f2be2c06657 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#27 0x000000000041afb5 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:655
#28 0x000000000041a68a in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:469
#29 0x000000000041c58b in main (argc=2, argv=0x7fffd29d62e8) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1096

[Abhishek Arya]

Martin, i compiled a gtk checkout. This testcase does not crash in GtkLauncher. This looks like a issue specific to DumpRenderTreeSupportGtk. Can anyone from gtk team fix this. The reason for the crash probably is that anonymous blocks are essentially null nodes. So, it does look like there is a null check needed at line 145.

CString DumpRenderTreeSupportGtk::getInnerText(WebKitWebFrame* frame)
132	{
133	    g_return_val_if_fail(WEBKIT_IS_WEB_FRAME(frame), CString(""));
134	
135	    Frame* coreFrame = core(frame);
136	    if (!coreFrame)
137	        return CString("");
138	
139	    FrameView* view = coreFrame->view();
140	
141	    if (view && view->layoutPending())
142	        view->layout();
143	
144	    Element* documentElement = coreFrame->document()->documentElement();
145	    return documentElement->innerText().utf8();

ccing [e-mail] who wrote this code.

[Antonio Gomes]

Looking ...

[Antonio Gomes]

Back to it.

Interesting:

The code in DumpRenderThreeSupportGtk::getInnerText that crashes now is essentially the same as it was in webkit_web_frame_get_inner_text, before DRTSupportGtk exits.

-    Element* documentElement = coreFrame->document()->documentElement();
-    String string =  documentElement->innerText();
-    return g_strdup(string.utf8().data());

and now

+    Element* documentElement = coreFrame->document()->documentElement();
+    return documentElement->innerText().utf8();

Any idea why the crash manifests only now?

[Martin Robinson]

(In reply to comment #4)
> +    Element* documentElement = coreFrame->document()->documentElement();
> +    return documentElement->innerText().utf8();
> 
> Any idea why the crash manifests only now?

I think the test is relatively new. It was added in
http://trac.webkit.org/changeset/74781

[Antonio Gomes]

Patch seem to be passing locally (after unskipping), release r75454:

$ WEBKITOUTPUTDIR=`pwd`/WebKitBuild/Gtk/Release run-webkit-tests --gtk fast/multicol/ --ignore-metrics
Running build-dumprendertree
Running tests from /home/agomes/Devel/webkit/webkit/LayoutTests
Testing 49 test cases.
fast/multicol ...................................
fast/multicol/span ..............
1.56s total testing time

all 49 test cases succeeded


@mrobinson can you reproduce the crash?

[Martin Robinson]

(In reply to comment #6)
> @mrobinson can you reproduce the crash?

I cannot reproduce the crash, though to run the test I think you have to run it explicitly, since it's skipped:

env -i xvfb-run ./Tools/Scripts/run-webkit-tests  --gtk --debug fast/multicol/span/double-merge-anonymous-block-crash.html

This was specifically crashing on the debug build, by the way.

[Antonio Gomes]

(In reply to comment #6)
> Patch seem to be passing locally (after unskipping), release r75454:

*Test* seems to be passing locally (after unskipping) ...

> I cannot reproduce the crash, though to run the test I think you have to run it explicitly, since it's skipped:
> 
> env -i xvfb-run ./Tools/Scripts/run-webkit-tests  --gtk --debug fast/multicol/span/double-merge-anonymous-block-crash.html
> 
> This was specifically crashing on the debug build, by the way.

I tried debug but repeating each test execution 20 times with 100 interactions. No crash...

I just do not want to add a null check w/out seeing the crash :( It should not be null from what I saw in the test html source anyways.

I will try the exactly command you pasted from home. Thanks for testing!

[Antonio Gomes]

Martin unskipped and bots seem happy: http://trac.webkit.org/changeset/75513