Attachment #282821 for bug #159439

View Details Raw Unified Return to Bug 159439



2016-07-05  Geoffrey Garen  <ggaren@apple.com>

        Crash @ bankofamerica.com, University of Vienna
        https://bugs.webkit.org/show_bug.cgi?id=159439

        Reviewed by NOBODY (OOPS!).

        * ftl/FTLLink.cpp:
        (JSC::FTL::link): Do check for stack overflow in the arity mismatch thunk
        because it can happen. Don't store a CallSiteIndex because we haven't
        stored a CodeBlock yet, and our stack frame is not fully constructed,
        so it would be an error for any client to try to load this value.

        * tests/stress/arity-check-ftl-throw.js: Added. New test case for stressing
        a stack overflow with arity mismatch. Sadly, after hours of fiddling, I
        can't seem to get this to fail in trunk. Still, it's good to have some
        more testing in this area.

2016-07-05  Benjamin Poulain  <bpoulain@apple.com>

        [JSC] The prototype cycle checks throws the wrong error type



            CCallHelpers::TrustedImm32(codeBlock->numParameters())));
        jit.emitFunctionPrologue();
        jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
        jit.store32(
            CCallHelpers::TrustedImm32(CallSiteIndex(0).bits()),
            CCallHelpers::tagFor(JSStack::ArgumentCount));
        jit.storePtr(GPRInfo::callFrameRegister, &vm.topCallFrame);
        CCallHelpers::Call callArityCheck = jit.call();

        auto noException = jit.branch32(CCallHelpers::AboveOrEqual, GPRInfo::returnValueGPR, CCallHelpers::TrustedImm32(0));
        jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
        jit.move(CCallHelpers::TrustedImmPtr(jit.vm()), GPRInfo::argumentGPR0);
        jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
        CCallHelpers::Call callLookupExceptionHandlerFromCallerFrame = jit.call();
        jit.jumpToExceptionHandler();
        noException.link(&jit);

#if !ASSERT_DISABLED
        // FIXME: need to make this call register with exception handling somehow. This is
        // part of a bigger problem: FTL should be able to handle exceptions.
        // https://bugs.webkit.org/show_bug.cgi?id=113622
        // Until then, use a JIT ASSERT.
        jit.load64(vm.addressOfException(), GPRInfo::regT1);
        jit.jitAssertIsNull(GPRInfo::regT1);
#endif

        jit.move(GPRInfo::returnValueGPR, GPRInfo::argumentGPR0);
        jit.emitFunctionEpilogue();
        mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::argumentGPR0));

            return;
        }
        linkBuffer->link(callArityCheck, codeBlock->m_isConstructor ? operationConstructArityCheck : operationCallArityCheck);
        linkBuffer->link(callLookupExceptionHandlerFromCallerFrame, lookupExceptionHandlerFromCallerFrame);
        linkBuffer->link(callArityFixup, FunctionPtr((vm.getCTIStub(arityFixupGenerator)).code().executableAddress()));
        linkBuffer->link(mainPathJumps, CodeLocationLabel(bitwise_cast<void*>(state.generatedFunction)));




// Require lots of arguments so that arity fixup will need a lot of stack, making
// it prone to stack overflow.
var script = "";
for (var i = 0; i < 128; ++i)
    script += "dummy, "
script += "dummy";
var g = new Function(script, "return arguments;"); // Ensure that arguments are observed.

function f(recursionCount)
{
    if (!recursionCount)
        return;

    // Use too few arguments to force arity fixup.
    g();

    f(--recursionCount);
}

noInline(g);
noInline(f);

// Ensure that f and g get optimized.
for (var i = 0; i < 1000000; ++i) {
    // Recurse once to ensure profiling along all control flow paths.
    f(1);
}

try {
    // Recurse enough times to trigger a stack overflow exception.
    f(1000000);
} catch(e) {
    if (! (e instanceof RangeError))
        throw "bad value for e";
}

Return to Bug 159439

Lines 137-155 void link(State& state) Source/JavaScriptCore/ftl/FTLLink.cpp_sec1
- Source/JavaScriptCore/ftl/FTLLink.cpp -7 / +11 lines
137	CCallHelpers::TrustedImm32(codeBlock->numParameters())));	137	CCallHelpers::TrustedImm32(codeBlock->numParameters())));
138	jit.emitFunctionPrologue();	138	jit.emitFunctionPrologue();
139	jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);	139	jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
140	jit.store32(
141	CCallHelpers::TrustedImm32(CallSiteIndex(0).bits()),
142	CCallHelpers::tagFor(JSStack::ArgumentCount));
143	jit.storePtr(GPRInfo::callFrameRegister, &vm.topCallFrame);	140	jit.storePtr(GPRInfo::callFrameRegister, &vm.topCallFrame);
144	CCallHelpers::Call callArityCheck = jit.call();	141	CCallHelpers::Call callArityCheck = jit.call();
		142
		143	auto noException = jit.branch32(CCallHelpers::AboveOrEqual, GPRInfo::returnValueGPR, CCallHelpers::TrustedImm32(0));
		144	jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
		145	jit.move(CCallHelpers::TrustedImmPtr(jit.vm()), GPRInfo::argumentGPR0);
		146	jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
		147	CCallHelpers::Call callLookupExceptionHandlerFromCallerFrame = jit.call();
		148	jit.jumpToExceptionHandler();
		149	noException.link(&jit);
		150
145	#if !ASSERT_DISABLED	151	#if !ASSERT_DISABLED
146	// FIXME: need to make this call register with exception handling somehow. This is
147	// part of a bigger problem: FTL should be able to handle exceptions.
148	// https://bugs.webkit.org/show_bug.cgi?id=113622
149	// Until then, use a JIT ASSERT.
150	jit.load64(vm.addressOfException(), GPRInfo::regT1);	152	jit.load64(vm.addressOfException(), GPRInfo::regT1);
151	jit.jitAssertIsNull(GPRInfo::regT1);	153	jit.jitAssertIsNull(GPRInfo::regT1);
152	#endif	154	#endif
		155
153	jit.move(GPRInfo::returnValueGPR, GPRInfo::argumentGPR0);	156	jit.move(GPRInfo::returnValueGPR, GPRInfo::argumentGPR0);
154	jit.emitFunctionEpilogue();	157	jit.emitFunctionEpilogue();
155	mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::argumentGPR0));	158	mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::argumentGPR0));
Lines 164-169 void link(State& state) Source/JavaScriptCore/ftl/FTLLink.cpp_sec2
164	return;	167	return;
165	}	168	}
166	linkBuffer->link(callArityCheck, codeBlock->m_isConstructor ? operationConstructArityCheck : operationCallArityCheck);	169	linkBuffer->link(callArityCheck, codeBlock->m_isConstructor ? operationConstructArityCheck : operationCallArityCheck);
		170	linkBuffer->link(callLookupExceptionHandlerFromCallerFrame, lookupExceptionHandlerFromCallerFrame);
167	linkBuffer->link(callArityFixup, FunctionPtr((vm.getCTIStub(arityFixupGenerator)).code().executableAddress()));	171	linkBuffer->link(callArityFixup, FunctionPtr((vm.getCTIStub(arityFixupGenerator)).code().executableAddress()));
168	linkBuffer->link(mainPathJumps, CodeLocationLabel(bitwise_cast<void*>(state.generatedFunction)));	172	linkBuffer->link(mainPathJumps, CodeLocationLabel(bitwise_cast<void*>(state.generatedFunction)));
169		173

Lines 1-3 Source/JavaScriptCore/ChangeLog_sec1
- Source/JavaScriptCore/ChangeLog +18 lines
		1	2016-07-05 Geoffrey Garen <ggaren@apple.com>
		2
		3	Crash @ bankofamerica.com, University of Vienna
		4	https://bugs.webkit.org/show_bug.cgi?id=159439
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	* ftl/FTLLink.cpp:
		9	(JSC::FTL::link): Do check for stack overflow in the arity mismatch thunk
		10	because it can happen. Don't store a CallSiteIndex because we haven't
		11	stored a CodeBlock yet, and our stack frame is not fully constructed,
		12	so it would be an error for any client to try to load this value.
		13
		14	* tests/stress/arity-check-ftl-throw.js: Added. New test case for stressing
		15	a stack overflow with arity mismatch. Sadly, after hours of fiddling, I
		16	can't seem to get this to fail in trunk. Still, it's good to have some
		17	more testing in this area.
		18
1	2016-07-05 Benjamin Poulain <bpoulain@apple.com>	19	2016-07-05 Benjamin Poulain <bpoulain@apple.com>
2		20
3	[JSC] The prototype cycle checks throws the wrong error type	21	[JSC] The prototype cycle checks throws the wrong error type

Line 0 Source/JavaScriptCore/tests/stress/arity-check-ftl-throw.js_sec1
- Source/JavaScriptCore/tests/stress/arity-check-ftl-throw.js +35 lines
	1	// Require lots of arguments so that arity fixup will need a lot of stack, making
	2	// it prone to stack overflow.
	3	var script = "";
	4	for (var i = 0; i < 128; ++i)
	5	script += "dummy, "
	6	script += "dummy";
	7	var g = new Function(script, "return arguments;"); // Ensure that arguments are observed.
	8
	9	function f(recursionCount)
	10	{
	11	if (!recursionCount)
	12	return;
	13
	14	// Use too few arguments to force arity fixup.
	15	g();
	16
	17	f(--recursionCount);
	18	}
	19
	20	noInline(g);
	21	noInline(f);
	22
	23	// Ensure that f and g get optimized.
	24	for (var i = 0; i < 1000000; ++i) {
	25	// Recurse once to ensure profiling along all control flow paths.
	26	f(1);
	27	}
	28
	29	try {
	30	// Recurse enough times to trigger a stack overflow exception.
	31	f(1000000);
	32	} catch(e) {
	33	if (! (e instanceof RangeError))
	34	throw "bad value for e";
	35	}