Bug 126174 – jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit is failing in cStack

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

Probably a varargs issue.

[Filip Pizlo]

It looks like this fails even in the LLInt.  Still investigating...

[Filip Pizlo]

This looks like an exception throwing, unwinding, and stack overflow bug.

All engines crash on the tests for stack overflow, where you apply with a ridiculous argument count.

[Mark Lam]

Here's a reduced test case that reproduces this issue (or likely the root cause of this issue):

function foo() {
    try {
        foo();
    } catch (e) {
        throw e;
    }
}
foo();

[Mark Lam]

Created attachment 220024 [details]
the patch.

[Mark Lam]

Created attachment 220025 [details]
the patch.

[Mark Lam]

When we do a stack check in a function prologue, the activation object in the frame hasn't been set yet. The test failures came from the stack unwinding code trying to tear off the frame to a non-existant activation object. Since we haven't entered the function yet and the frame is technically not fully "pushed" yet, we can throw i.e. start the unwinding from the caller frame instead. This fixes the issue.

Landed in r161084 on the jsCStack branch: <http://trac.webkit.org/r161084>.

[Michael Saboff]

With the span size ASSERT disabled in ConservativeRoots.cpp, function-apply-aliased.js.layout-no-cjit (and the other instances of function-apply-aliased.js) fail after timing out:

jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: Timed out after 169.000000 seconds!
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 1   0x10188adc0 WTFCrash
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 2   0x1011be960 jscmain(int, char**)
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 3   0x1018da288 WTF::threadEntryPoint(void*)
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 4   0x1018db038 WTF::wtfThreadEntryPoint(void*)
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 5   0x7fff8c879899 _pthread_body
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 6   0x7fff8c87972a _pthread_struct_init
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: 7   0x7fff8c87dfc9 thread_start
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: test_script_6882: line 2: 57151 Segmentation fault: 11  "$@" ../../../../.vm/JavaScriptCore.framework/Resources/jsc --enableConcurrentJIT\=false resources/standalone-pre.js function-apply-aliased.js resources/standalone-post.js
jsc-layout-tests.yaml/js/script-tests/function-apply-aliased.js.layout-no-cjit: ERROR: Unexpected exit code: 139

[Mark Lam]

(In reply to comment #7)
> With the span size ASSERT disabled in ConservativeRoots.cpp, function-apply-aliased.js.layout-no-cjit (and the other instances of function-apply-aliased.js) fail after timing out:

That is because the JSStack size used to be 512K.  Now it is is 63M.  I’m now working on adding a VM option to cap the C stack size, and we’ll cap the stack size to 4M for jsc and tests.  With that, the tests won’t time out.

[Filip Pizlo]

(In reply to comment #8)
> (In reply to comment #7)
> > With the span size ASSERT disabled in ConservativeRoots.cpp, function-apply-aliased.js.layout-no-cjit (and the other instances of function-apply-aliased.js) fail after timing out:
> 
> That is because the JSStack size used to be 512K.  Now it is is 63M.  I’m now working on adding a VM option to cap the C stack size, and we’ll cap the stack size to 4M for jsc and tests.  With that, the tests won’t time out.

OK - we need to have an *actual* story for this.  We can't have a WebKit user getting a spin every time that some program overflows stack.

Maybe you should have limits on how much stack JS is allowed to use?  As we talked previously, you'll have to have a story for how this will work in case of VM reentry.

[Mark Lam]

(In reply to comment #9)
> Maybe you should have limits on how much stack JS is allowed to use?  As we talked previously, you'll have to have a story for how this will work in case of VM reentry.

The work to use a separate limit for the JS stack is at https://bugs.webkit.org/show_bug.cgi?id=126320.

[Geoffrey Garen]

Comment on attachment 220025 [details]
the patch.

r=me

[Mark Lam]

Review status updated in r162014: <http://trac.webkit.org/r162014>.