Bug 35425 – [GTK] plugins/setProperty.html fails on 64bit Release

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philippe Normand]

I can reproduce the crash only when executing all the "plugins" tests:

(gdb) bt
#0  malloc_consolidate (av=0x7fc5458bfe40) at malloc.c:5089
#1  0x00007fc5455cd509 in _int_malloc (av=0x7fc5458bfe40, bytes=33) at malloc.c:4338
#2  0x00007fc5455cf82e in *__GI___libc_malloc (bytes=8192) at malloc.c:3638
#3  0x00007fc54681e983 in IA__g_malloc (n_bytes=8192) at gmem.c:131
#4  0x00007fc54ade393c in WebCore::openCallback(_GObject*, _GAsyncResult*, void*) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/staging/Release/.libs/libwebkit-1.0.so.2
#5  0x00007fc547379458 in complete_in_idle_cb_for_thread (_data=<value optimized out>) at gsimpleasyncresult.c:653
#6  0x00007fc546815c80 in g_main_dispatch (context=0x1d69a40) at gmain.c:1960
#7  IA__g_main_context_dispatch (context=0x1d69a40) at gmain.c:2513
#8  0x00007fc546819ab8 in g_main_context_iterate (context=0x1d69a40, block=<value optimized out>, 
    dispatch=<value optimized out>, self=<value optimized out>) at gmain.c:2591
#9  0x00007fc546819ffd in IA__g_main_loop_run (loop=0x1d6f520) at gmain.c:2799
#10 0x00007fc5486f9287 in IA__gtk_main () at gtkmain.c:1219
#11 0x000000000040f14f in runTest(std::string const&) ()
#12 0x000000000040f8d2 in main ()

Not trivial to debug in gdb as this is a release build :( Will skip the test for now.

[Philippe Normand]

A git bisect revealed that r54786 is the culprit :) Gustavo, what do you think?

[Philippe Normand]

The same commit makes  plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream.html crash too:

(gdb) bt
#0  0x0000000001d626e0 in ?? ()
#1  0x00007f64296cf8ce in WebCore::ScriptController::clearScriptObjects() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#2  0x00007f6429915f30 in WebCore::FrameLoader::clear(bool, bool, bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#3  0x00007f6429917cae in WebCore::FrameLoader::begin(WebCore::KURL const&, bool, WebCore::SecurityOrigin*) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#4  0x00007f642991838e in WebCore::FrameLoader::receivedFirstData() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#5  0x00007f6429918634 in WebCore::FrameLoader::setEncoding(WebCore::String const&, bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#6  0x00007f642991a8d8 in WebCore::FrameLoader::finishedLoadingDocument(WebCore::DocumentLoader*) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#7  0x00007f6429902078 in WebCore::DocumentLoader::finishedLoading() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#8  0x00007f6429918db4 in WebCore::FrameLoader::finishedLoading() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#9  0x00007f642992b627 in WebCore::MainResourceLoader::didFinishLoading() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#10 0x00007f642992e147 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#11 0x00007f642992e566 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#12 0x00007f64299359c6 in WebCore::PolicyChecker::continueAfterContentPolicy(WebCore::PolicyAction) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#13 0x00007f6429c9ff71 in WebKit::FrameLoaderClient::dispatchDecidePolicyForMIMEType(void (WebCore::PolicyChecker::*)(WebCore::PolicyAction), WebCore::String const&, WebCore::ResourceRequest const&) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#14 0x00007f642992fd68 in WebCore::MainResourceLoader::didReceiveResponse(WebCore::ResourceResponse const&) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#15 0x00007f642992beb2 in WebCore::MainResourceLoader::handleEmptyLoad(WebCore::KURL const&, bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#16 0x00007f642992cc84 in WebCore::MainResourceLoader::loadNow(WebCore::ResourceRequest&) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#17 0x00007f642992ecc0 in WebCore::MainResourceLoader::load(WebCore::ResourceRequest const&, WebCore::SubstituteData const&)
    () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#18 0x00007f64299023ed in WebCore::DocumentLoader::startLoadingMainResource(unsigned long) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#19 0x00007f6429912c55 in WebCore::FrameLoader::continueLoadAfterWillSubmitForm() ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#20 0x00007f642991e3c0 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#21 0x00007f642991e3ed in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#22 0x00007f6429934a38 in WebCore::PolicyCallback::call(bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#23 0x00007f64299363cf in WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#24 0x00007f6429c9f987 in WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction(void (WebCore::PolicyChecker::*)(WebCore::PolicyAction), WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>) () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#25 0x00007f6429936f2b in WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, void (*)(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool), void*) () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#26 0x00007f642991e79a in WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>) () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#27 0x00007f642991e928 in WebCore::FrameLoader::load(WebCore::DocumentLoader*) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#28 0x00007f642991ea4b in WebCore::FrameLoader::load(WebCore::ResourceRequest const&, WebCore::SubstituteData const&, bool)
    () from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#29 0x00007f642991eb36 in WebCore::FrameLoader::load(WebCore::ResourceRequest const&, bool) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#30 0x00007f6429cac97d in webkit_web_frame_load_uri ()
---Type <return> to continue, or q <return> to quit---
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkit-1.0.so.2
#31 0x000000000040f029 in runTest(std::string const&) ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/staging/Release/Programs/DumpRenderTree
#32 0x000000000040f702 in main ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/staging/Release/Programs/DumpRenderTree

[Gustavo Noronha (kov)]

(In reply to comment #1)
> A git bisect revealed that r54786 is the culprit :) Gustavo, what do you think?

ouch; I think that fix may be just exposing the bug, I'll have to try and debug it

[Gustavo Noronha (kov)]

(In reply to comment #2)
> The same commit makes 
> plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream.html crash
> too:

That is funny. This code doesn't seem to go through the parseDataUrl code path.

[Gustavo Noronha (kov)]

Created attachment 49752 [details]
proposed fix

[Gustavo Noronha (kov)]

For the curious: it turns out that the invoke call that is done by handleCallback may fail, and return before it has initialized the browserResult variant to anything that makes sense. As it happens, in 64 bits Linux the variable ended up being considered to be of type string, but obviously failed to have a correctly malloc'ed string as the data, so free was called in unrelated memory, causing the corruption.

As to why my fix to parseDataUrl exposed the problem, the URL notification was the cause for the call that calls invoke. Since we were not setting an URL in the response, no notification happened.

[Eric Seidel (no email)]

Comment on attachment 49752 [details]
proposed fix

I wonder how many other cases of unconditional release we have?

[Gustavo Noronha (kov)]

Created attachment 49754 [details]
only release the variant when invoke succeeds

Applying the fix to all places where it's needed.

[Eric Seidel (no email)]

Comment on attachment 49754 [details]
only release the variant when invoke succeeds

OK.

[Gustavo Noronha (kov)]

Landed as r55392.