Changeset 202838 in webkit

Timestamp:

Jul 5, 2016 4:09:51 PM (8 years ago)

Author:

ggaren@apple.com

Message:

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

Reviewed by Saam Barati.

• 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 (and
operationCallArityCheck does not 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.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• ftl/FTLLink.cpp (modified) (2 diffs)
• tests/stress/arity-check-ftl-throw.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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 Saam Barati.
+
+        * 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 (and
+        operationCallArityCheck does not 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>
 

trunk/Source/JavaScriptCore/ftl/FTLLink.cpp

@@ -138 +138 @@
         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();
@@ -165 +168 @@
         }
         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)));