Bug 41281 – HTML5 Regression: Crash in insert()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Tony Gentilcore]

http://build.chromium.org/buildbot/waterfall.fyi/builders/WebKit%20Reliability/builds/387/steps/reliability:%20partial%20result%20of%20current%20build/logs/stdio

REGRESSION: NEW crash stack traces found
--------------------
Repro information:
Unfiltered URL: http://www.freshersworld.com/

Stack trace:
chrome_2580000!WebCore::HTMLDocumentParser::insert+0x78 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 228]
chrome_2580000!WebCore::Document::write+0x46 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2032]
chrome_2580000!WebCore::V8HTMLDocument::writeCallback+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\custom\v8htmldocumentcustom.cpp @ 115]
chrome_2580000!v8::internal::HandleApiCallHelper<0>+0x197 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 971]
chrome_2580000!v8::internal::Builtin_HandleApiCall+0xf [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 988]
chrome_2580000!v8::internal::Invoke+0xc2 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 96]
chrome_2580000!v8::internal::Execution::Call+0x26 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 121]
chrome_2580000!v8::Script::Run+0x157 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\api.cc @ 1247]
chrome_2580000!WebCore::V8Proxy::runScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 452]
chrome_2580000!WebCore::V8Proxy::evaluate+0x16b [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 404]
chrome_2580000!WebCore::ScriptController::evaluate+0x111 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\scriptcontroller.cpp @ 242]
chrome_2580000!WebCore::ScriptController::executeScript+0x8c [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\scriptcontrollerbase.cpp @ 62]
chrome_2580000!WebCore::HTMLScriptRunner::executeScript+0x48 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 160]
chrome_2580000!WebCore::HTMLScriptRunner::runScript+0x1a4 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 277]
chrome_2580000!WebCore::HTMLScriptRunner::execute+0x13 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 187]
chrome_2580000!WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder+0x92 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 153]
chrome_2580000!WebCore::HTMLDocumentParser::pumpTokenizer+0xb0 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 180]
chrome_2580000!WebCore::HTMLDocumentParser::insert+0x68 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 227]
chrome_2580000!WebCore::Document::write+0x46 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2032]
chrome_2580000!WebCore::V8HTMLDocument::writeCallback+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\custom\v8htmldocumentcustom.cpp @ 115]
chrome_2580000!v8::internal::HandleApiCallHelper<0>+0x197 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 971]
chrome_2580000!v8::internal::Builtin_HandleApiCall+0xf [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 988]
WARNING: Frame IP not in any known module. Following frames may be wrong.
0x61d010e
0x66ba8d2
chrome_2580000!v8::internal::Invoke+0xc2 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 96]
chrome_2580000!v8::internal::Execution::Call+0x26 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 121]
chrome_2580000!v8::Script::Run+0x157 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\api.cc @ 1247]
chrome_2580000!WebCore::V8Proxy::runScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 452]
chrome_2580000!WebCore::V8Proxy::evaluate+0x16b [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 404]
chrome_2580000!WebCore::ScriptController::evaluate+0x111 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\scriptcontroller.cpp @ 242]
chrome_2580000!WebCore::ScriptController::executeScript+0x8c [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\scriptcontrollerbase.cpp @ 62]
chrome_2580000!WebCore::HTMLScriptRunner::executeScript+0x48 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 160]
chrome_2580000!WebCore::HTMLScriptRunner::executePendingScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 141]
chrome_2580000!WebCore::HTMLScriptRunner::executeParsingBlockingScripts+0x50 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 207]
chrome_2580000!WebCore::HTMLDocumentParser::executeScriptsWaitingForStylesheets+0x24 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 395]
chrome_2580000!WebCore::Document::removePendingSheet+0x3a [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2623]
chrome_2580000!WebCore::HTMLLinkElement::sheetLoaded+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmllinkelement.cpp @ 355]
chrome_2580000!WebCore::CSSStyleSheet::checkLoaded+0x31 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\css\cssstylesheet.cpp @ 214]
chrome_2580000!WebCore::HTMLLinkElement::setCSSStyleSheet+0x329 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmllinkelement.cpp @ 340]
chrome_2580000!WebCore::CachedCSSStyleSheet::checkNotify+0x70 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\cachedcssstylesheet.cpp @ 116]
chrome_2580000!WebCore::CachedCSSStyleSheet::data+0x13a [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\cachedcssstylesheet.cpp @ 106]
chrome_2580000!WebCore::Loader::Host::didFinishLoading+0xd4 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\loader.cpp @ 407]
chrome_2580000!WebCore::SubresourceLoader::didFinishLoading+0x27 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\subresourceloader.cpp @ 196]
chrome_2580000!WebCore::ResourceLoader::didFinishLoading+0x7 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\resourceloader.cpp @ 444]
chrome_2580000!webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest+0x18c [c:\b\slave\webkit-rel-reliability-builder\build\src\webkit\glue\weburlloader_impl.cc @ 583]
chrome_2580000!ResourceDispatcher::OnRequestComplete+0x8e [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 469]
chrome_2580000!IPC::MessageWithTuple<Tuple3<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >::Dispatch<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>+0x5b [c:\b\slave\webkit-rel-reliability-builder\build\src\ipc\ipc_message_utils.h @ 1044]
chrome_2580000!ResourceDispatcher::DispatchMessageW+0xa1 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 536]
chrome_2580000!ResourceDispatcher::OnMessageReceived+0x287 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 302]
chrome_2580000!ChildThread::OnMessageReceived+0x1a [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\child_thread.cc @ 124]
chrome_2580000!RunnableMethod<`anonymous namespace'::JobTracker,void (__thiscall A0x926de479::JobTracker::*)(std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &),Tuple1<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > > >::Run+0x17 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\task.h @ 323]
chrome_2580000!MessageLoop::RunTask+0x10d [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 341]
chrome_2580000!MessageLoop::DoWork+0x12f [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 460]
chrome_2580000!base::MessagePumpDefault::Run+0x117 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_pump_default.cc @ 50]
chrome_2580000!MessageLoop::RunInternal+0x92 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 214]
chrome_2580000!MessageLoop::Run+0x5a [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 165]
chrome_2580000!RendererMain+0x33f [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\renderer\renderer_main.cc @ 294]
chrome_2580000!ChromeMain+0xab9 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\chrome_dll_main.cc @ 760]
chrome!MainDllLoader::Launch+0x191 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\client_util.cc @ 257]
chrome!wWinMain+0x97 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\chrome_exe_main.cc @ 47]
chrome!__tmainCRTStartup+0x176 [f:\dd\vctools\crt_bld\self_x86\crt\src\crt0.c @ 324]
kernel32!RegisterWaitForInputIdle+0x49

[Adam Barth]

Tony, are you investigating this crash?

@eseidel: How do we get into a situation in which executeScriptsWaitingForStylesheets is called?  After that, it looks like two nested calls to document.write.

[Tony Gentilcore]

(In reply to comment #1)
> Tony, are you investigating this crash?

Sorry, I can't look into it until after lunch today. If it is still there, I'll tackle it then.

> 
> @eseidel: How do we get into a situation in which executeScriptsWaitingForStylesheets is called?  After that, it looks like two nested calls to document.write.

[Adam Barth]

Ok.  I'll take a look.

[Adam Barth]

Well, I can create this call stack using the following test case:

== repro.html ==

<link rel="stylesheet" href="http://webblaze.org/">
<body>
1
<script src="ext.js"></script>
5
Done!

== ext.js ==

document.write(2);
document.write(" <script>document.write(3)</script> ");
document.write(4);

However, nothing bad happens.  My theory is as follows: instead of 3, we do something that tricks endIfDelayed into actually calling end(), which deletes the HTMLDocumentParser, which causes the decrement of the script nesting level to be twiddling unallocated memory.  I'm pretty sure there's a bug on that line of code, so I'm going to post a patch to fix it.

[Adam Barth]

Created attachment 59907 [details]
Patch

[Alexey Proskuryakov]

This is just a crasher bug, not a site being incompatible with HTML5, so this shouldn't block bug 41115.

[Eric Seidel (no email)]

Comment on attachment 59907 [details]
Patch

I definitely like this change.

WebCore/html/HTMLDocumentParser.h:112
 +      bool shouldDelayEnd() const { return inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume(); }
Seems sightly silly to put this in the header.  Generally the .cpp is better/cleaner.

I was sad when I broke write into insert and append because the code sharing previously seemed good.  It's still good.  But I wonder how it could be better.

[WebKit Commit Bot]

Comment on attachment 59907 [details]
Patch

Clearing flags on attachment: 59907

Committed r62033: <http://trac.webkit.org/changeset/62033>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

> WebCore/html/HTMLDocumentParser.h:112
>  +      bool shouldDelayEnd() const { return inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume(); }
> Seems sightly silly to put this in the header.  Generally the .cpp is better/cleaner.

Yeah, I don't know if we have a consistent way of making those decisions.

[WebKit Review Bot]

http://trac.webkit.org/changeset/62033 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/62032
http://trac.webkit.org/changeset/62033
http://trac.webkit.org/changeset/62034
http://trac.webkit.org/changeset/62031

[jobslink]

Created attachment 432324 [details]
JobsLink,  a true pioneer & India’s most comprehensive online career portal that provides a wide range of opportunities for job seekers to stride with their career.

JobsLink,  a true pioneer & India’s most comprehensive online career portal that provides a wide range of opportunities for job seekers to stride with their career.
 Find and apply for the latest job offers in private, government and international with JobsLink online career portal. JobsLink has taken the initiative to cater to the needs of the graduated community globally. JobsLink stands as the best online career portal.


 https://www.jobslink.in/

[mmenterprises]

MME, a true pioneer & India’s most comprehensive online career portal that provides a wide range of opportunities for job seekers to stride with their career. https://mmenterprises.co.in/