Bug 121376 - REGRESSION(r155771): js/stack-overflow-arrity-catch.html is crashing on non-Mac platforms
Summary: REGRESSION(r155771): js/stack-overflow-arrity-catch.html is crashing on non-M...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: Gtk, Regression
Depends on:
Blocks:
 
Reported: 2013-09-14 23:07 PDT by Zan Dobersek
Modified: 2013-09-18 13:45 PDT (History)
5 users (show)

See Also:


Attachments
disassembly of the llint function, register dump and a full back trace on a debug build (x86-64) (313.74 KB, text/plain)
2013-09-17 14:22 PDT, Gustavo Noronha (kov)
no flags Details
Patch (1.65 KB, patch)
2013-09-17 18:20 PDT, Michael Saboff
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-09-14 23:07:05 PDT
The js/stack-overflow-arrity-catch.html layout test is crashing on non-Mac ports after r155771.
http://trac.webkit.org/changeset/155711

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=js%2Fstack-overflow-arrity-catch.html

Here's the relevant backtrace from a GTK debug build:
crash log for DumpRenderTree (pid 22832):
...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/Programs/D'.
Program terminated with signal 11, Segmentation fault.
#0  0x00007f2a52ca67f1 in llint_function_for_call_arity_check () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/.libs/libjavascriptcoregtk-3.0.so.0

...

Thread 1 (Thread 0x7f2a4207a900 (LWP 22832)):
#0  0x00007f2a52ca67f1 in llint_function_for_call_arity_check () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/.libs/libjavascriptcoregtk-3.0.so.0
#1  0x00007f2a0069cfa0 in ?? ()
#2  0x00000000023d9510 in ?? ()
#3  0x00007f29e294f8a8 in ?? ()
#4  0x00007f2a00000008 in ?? ()
#5  0x00007f2a0069cfa0 in ?? ()
#6  0x0000000001f11e10 in ?? ()
#7  0x00007fff38715630 in ?? ()
#8  0x00007f2a5287db80 in JSC::MacroAssemblerCodeRef::operator! (this=0x7f2a4dfdf118 <WebCore::JSDOMWindowBase::supportsRichSourceInfo(JSC::JSGlobalObject const*)>) at ../../Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:409
#9  0x00007f2a5287d4a8 in JSC::JITCode::execute (this=0x1f11e00, stack=0x1a6c9f8, callFrame=0x7f2a0069cfa0, vm=0x1a5b800) at ../../Source/JavaScriptCore/jit/JITCode.cpp:46
#10 0x00007f2a52868210 in JSC::Interpreter::execute (this=0x1a6c9e0, program=0x7f2a000fd7f0, callFrame=0x7f2a001ddbb0, thisObj=0x7f2a0021ffd8) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:882
#11 0x00007f2a5294d938 in JSC::evaluate (exec=0x7f2a001ddbb0, source=..., thisValue=..., returnedException=0x7fff38716350) at ../../Source/JavaScriptCore/runtime/Completion.cpp:83
#12 0x00007f2a4e00b726 in WebCore::JSMainThreadExecState::evaluate (exec=0x7f2a001ddbb0, source=..., thisValue=..., exception=0x7fff38716350) at ../../Source/WebCore/bindings/js/JSMainThreadExecState.h:61
#13 0x00007f2a4e03a66f in WebCore::ScriptController::evaluateInWorld (this=0x13ebfb0, sourceCode=..., world=0x1a70900) at ../../Source/WebCore/bindings/js/ScriptController.cpp:142
#14 0x00007f2a4e03a77e in WebCore::ScriptController::evaluate (this=0x13ebfb0, sourceCode=...) at ../../Source/WebCore/bindings/js/ScriptController.cpp:158
#15 0x00007f2a4e30321d in WebCore::ScriptElement::executeScript (this=0x20e7e58, sourceCode=...) at ../../Source/WebCore/dom/ScriptElement.cpp:315
#16 0x00007f2a4e517450 in WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent (this=0x1c55880, pendingScript=...) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:150
#17 0x00007f2a4e51729e in WebCore::HTMLScriptRunner::executeParsingBlockingScript (this=0x1c55880) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:122
#18 0x00007f2a4e51777c in WebCore::HTMLScriptRunner::executeParsingBlockingScripts (this=0x1c55880) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:201
#19 0x00007f2a4e5178be in WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad (this=0x1c55880, cachedScript=0x1c1a8b0) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:210
#20 0x00007f2a4e5041fb in WebCore::HTMLDocumentParser::notifyFinished (this=0x2105900, cachedResource=0x1c1a8b0) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:936
#21 0x00007f2a4e6bf3d6 in WebCore::CachedResource::checkNotify (this=0x1c1a8b0) at ../../Source/WebCore/loader/cache/CachedResource.cpp:369
#22 0x00007f2a4e6bf4ac in WebCore::CachedResource::finishLoading (this=0x1c1a8b0) at ../../Source/WebCore/loader/cache/CachedResource.cpp:385
#23 0x00007f2a4e6d353e in WebCore::CachedScript::finishLoading (this=0x1c1a8b0, data=0x1bd3a40) at ../../Source/WebCore/loader/cache/CachedScript.cpp:89
#24 0x00007f2a4e740384 in WebCore::SubresourceLoader::didFinishLoading (this=0x1c84b80, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:283
#25 0x00007f2a4e736ad1 in WebCore::ResourceLoader::didFinishLoading (this=0x1c84b80, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:489
#26 0x00007f2a4ef2c106 in WebCore::readCallback (asyncResult=0x1a76200, data=0x2032f80) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1329
#27 0x00007f2a4cd52eb8 in async_ready_callback_wrapper () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#28 0x00007f2a4cd7f22e in g_task_return_now () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#29 0x00007f2a4cd7f258 in complete_in_idle_cb () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#30 0x00007f2a4cba170c in g_idle_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#31 0x00007f2a4cb9efb1 in g_main_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#32 0x00007f2a4cb9fd08 in g_main_context_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#33 0x00007f2a4cb9fefa in g_main_context_iterate () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#34 0x00007f2a4cba0323 in g_main_loop_run () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#35 0x00007f2a4d4c9fcf in gtk_main () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgtk-3.so.0
#36 0x00000000004aeccb in runTest (inputLine=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:792
#37 0x00000000004ae39a in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:575
#38 0x00000000004b16e5 in main (argc=2, argv=0x7fff387174b8) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1531
Comment 1 Michael Saboff 2013-09-16 11:47:59 PDT
(In reply to comment #0)
> The js/stack-overflow-arrity-catch.html layout test is crashing on non-Mac ports after r155771.
> http://trac.webkit.org/changeset/155711
> 
> http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=js%2Fstack-overflow-arrity-catch.html
> 
> Here's the relevant backtrace from a GTK debug build:
> crash log for DumpRenderTree (pid 22832):
> ...
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> Core was generated by `/home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/Programs/D'.
> Program terminated with signal 11, Segmentation fault.
> #0  0x00007f2a52ca67f1 in llint_function_for_call_arity_check () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/.libs/libjavascriptcoregtk-3.0.so.0
> 
> ...
> 
> Thread 1 (Thread 0x7f2a4207a900 (LWP 22832)):
> #0  0x00007f2a52ca67f1 in llint_function_for_call_arity_check () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/.libs/libjavascriptcoregtk-3.0.so.0

Is it possible that someone could run this is a debugger and provide a register dump and disassembly of llint_function_for_call_arity_check showing the PC at the time of the crash?

I think we are dying while moving the stack frame after an arity mismatch, but it would be helpful to know exactly where.
Comment 2 Gustavo Noronha (kov) 2013-09-17 14:22:39 PDT
Created attachment 211941 [details]
disassembly of the llint function, register dump and a full back trace on a debug build (x86-64)

There you go, let me know if you need more info on this, I'm kov on IRC =)
Comment 3 Michael Saboff 2013-09-17 18:20:51 PDT
Created attachment 211961 [details]
Patch

This is the only issue I can think of at this point that might be causing this crash.  Please apply the attached patch and see if it fixes the problem.  If it fixes the issue, a review would be appreciated.  If it doesn't, I'll create a new defect and attach the patch to the new bug.
Comment 4 Zan Dobersek 2013-09-18 00:01:00 PDT
Yes, the patch fixes the problem. Thanks.
Comment 5 Oliver Hunt 2013-09-18 09:02:52 PDT
Comment on attachment 211961 [details]
Patch

Hmm, does grow() not have an assertion to ensure that grow() is actually growing?
Comment 6 Michael Saboff 2013-09-18 09:34:28 PDT
(In reply to comment #5)
> (From update of attachment 211961 [details])
> Hmm, does grow() not have an assertion to ensure that grow() is actually growing?

It does not.  grow() is an inline quick check that calls growSlowCase() if needed.  Most of the times we call grow(), it is more of check that we have space.  Consider a deep chain of calls that returns and then we have the arity check here.  There would be plenty of space because we aren't growing.
Comment 7 Michael Saboff 2013-09-18 09:50:16 PDT
Committed r156046: <http://trac.webkit.org/changeset/156046>
Comment 8 Oliver Hunt 2013-09-18 11:18:44 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 211961 [details] [details])
> > Hmm, does grow() not have an assertion to ensure that grow() is actually growing?
> 
> It does not.  grow() is an inline quick check that calls growSlowCase() if needed.  Most of the times we call grow(), it is more of check that we have space.  Consider a deep chain of calls that returns and then we have the arity check here.  There would be plenty of space because we aren't growing.

Do we not know what the base we're growing from is?
Comment 9 Michael Saboff 2013-09-18 13:45:31 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (From update of attachment 211961 [details] [details] [details])
> > > Hmm, does grow() not have an assertion to ensure that grow() is actually growing?
> > 
> > It does not.  grow() is an inline quick check that calls growSlowCase() if needed.  Most of the times we call grow(), it is more of check that we have space.  Consider a deep chain of calls that returns and then we have the arity check here.  There would be plenty of space because we aren't growing.
> 
> Do we not know what the base we're growing from is?

We have the base, but in the case of checking whether we need to grow we need to check the extent of the current stack allocation.  grow() does that.