Diff
Modified: trunk/JSTests/ChangeLog (204361 => 204362)
--- trunk/JSTests/ChangeLog 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/JSTests/ChangeLog 2016-08-10 23:45:05 UTC (rev 204362)
@@ -1,3 +1,22 @@
+2016-08-10 Michael Saboff <msab...@apple.com>
+
+ Baseline GetByVal and PutByVal for cache ID stubs need to handle exceptions
+ https://bugs.webkit.org/show_bug.cgi?id=160749
+
+ Reviewed by Filip Pizlo.
+
+ New test that causes baseline GetByValWithCachedId and PutByValWithCachedId
+ stubs to be generated and then throws exceptions for those stub to handle
+ to verify that they are properly handled.
+
+ * stress/regress-160749.js: Added.
+ (testCachedGetByVal.):
+ (testCachedGetByVal.get for):
+ (testCachedGetByVal):
+ (testCachedPutByVal.):
+ (testCachedPutByVal.set for):
+ (testCachedPutByVal):
+
2016-08-10 Mark Lam <mark....@apple.com>
DFG's flushForTerminal() needs to add PhantomLocals for bytecode live locals.
Added: trunk/JSTests/stress/regress-160749.js (0 => 204362)
--- trunk/JSTests/stress/regress-160749.js (rev 0)
+++ trunk/JSTests/stress/regress-160749.js 2016-08-10 23:45:05 UTC (rev 204362)
@@ -0,0 +1,91 @@
+// Regression test for 160749. This test should not exit with an error or crash.
+// Check that the Baseline JIT GetByValWithCacheId and PutByValWithCahcedId stubs properly handle exceptions.
+
+function testCachedGetByVal()
+{
+ o = { };
+ o['a'] = 42;
+
+ let result = 0;
+ let loopCount = 100000;
+ let interationToChange = 90000;
+ let expectedResult = 42 * interationToChange;
+ let exceptions = 0;
+ let expectedExceptions = loopCount - interationToChange;
+
+ for (let i = 0; i < loopCount; i++) {
+ if (i == interationToChange) {
+ Object.defineProperty(o, "a", {
+ enumerable: true,
+ get: function() { throw "error"; return 100; }
+ });
+ }
+
+ for (let v in o) {
+ try {
+ result += o[v.toString()];
+ } catch(e) {
+ if (e == "error")
+ exceptions++;
+ else
+ throw "Got wrong exception \"" + e + "\"";
+ }
+ }
+ }
+
+ if (result != expectedResult)
+ throw "Expected a result of " + expectedResult + ", but got " + result;
+ if (exceptions != expectedExceptions)
+ throw "1 Expected " + expectedExceptions + " exceptions, but got " + exceptions;
+}
+
+noDFG(testCachedGetByVal);
+
+function testCachedPutByVal()
+{
+ o = { };
+ o['a'] = 0;
+
+ let result = 0;
+ let loopCount = 100000;
+ let iterationToChange = 90000;
+ let exceptions = 0;
+ let expectedExceptions = loopCount - iterationToChange;
+
+ for (let i = 0; i < loopCount; i++) {
+ if (i == iterationToChange) {
+ result = o.a;
+ Object.defineProperty(o, "_a", {
+ enumerable: false,
+ value: -1
+ });
+ Object.defineProperty(o, "a", {
+ enumerable: true,
+ set: function(v) { throw "error"; o._a = v; }
+ });
+ }
+
+ for (let v in o) {
+ try {
+ o[v.toString()] = i + 1;
+ } catch(e) {
+ if (e == "error")
+ exceptions++;
+ else
+ throw "Got wrong exception \"" + e + "\"";
+ }
+ }
+ }
+
+ if (result != iterationToChange)
+ throw "Expected a result of " + result + ", but got " + o.a;
+ if (o._a != -1)
+ throw "Expected o._b to -1, but it is " + o._a;
+ if (exceptions != expectedExceptions)
+ throw "Expected " + expectedExceptions + " exceptions, but got " + exceptions;
+}
+
+noDFG(testCachedPutByVal);
+
+testCachedGetByVal();
+testCachedPutByVal();
Modified: trunk/Source/_javascript_Core/ChangeLog (204361 => 204362)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-10 23:45:05 UTC (rev 204362)
@@ -1,3 +1,40 @@
+2016-08-10 Michael Saboff <msab...@apple.com>
+
+ Baseline GetByVal and PutByVal for cache ID stubs need to handle exceptions
+ https://bugs.webkit.org/show_bug.cgi?id=160749
+
+ Reviewed by Filip Pizlo.
+
+ We were emitting "callOperation()" calls in emitGetByValWithCachedId() and
+ emitPutByValWithCachedId() without linking the exception checks created by the
+ code emitted. This manifested itself in various ways depending on the processor.
+ This is due to what the destination is for an unlinked branch. On X86, an unlinked
+ branch goes tot he next instructions. On ARM64, we end up with an infinite loop
+ as we branch to the same instruction. On ARM we branch to 0 as the branch is to
+ an absolute address of 0.
+
+ Now we save the exception handler address for the original generated function and
+ link the exception cases for these by-val stubs to this handler.
+
+ * bytecode/ByValInfo.h:
+ (JSC::ByValInfo::ByValInfo): Added the address of the exception handler we should
+ link to.
+
+ * jit/JIT.cpp:
+ (JSC::JIT::link): Compute the linked exception handler address and pass it to
+ the ByValInfo constructor.
+ (JSC::JIT::privateCompileExceptionHandlers): Make sure that we generate the
+ exception handler if we have any by-val handlers.
+
+ * jit/JIT.h:
+ Added a label for the exception handler. We'll link this later for the
+ by value handlers.
+
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::privateCompileGetByValWithCachedId):
+ (JSC::JIT::privateCompilePutByValWithCachedId):
+ Link exception branches to the exception handler for the main function.
+
2016-08-10 Mark Lam <mark....@apple.com>
DFG's flushForTerminal() needs to add PhantomLocals for bytecode live locals.
Modified: trunk/Source/_javascript_Core/bytecode/ByValInfo.h (204361 => 204362)
--- trunk/Source/_javascript_Core/bytecode/ByValInfo.h 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/Source/_javascript_Core/bytecode/ByValInfo.h 2016-08-10 23:45:05 UTC (rev 204362)
@@ -207,10 +207,11 @@
struct ByValInfo {
ByValInfo() { }
- ByValInfo(unsigned bytecodeIndex, CodeLocationJump notIndexJump, CodeLocationJump badTypeJump, JITArrayMode arrayMode, ArrayProfile* arrayProfile, int16_t badTypeJumpToDone, int16_t badTypeJumpToNextHotPath, int16_t returnAddressToSlowPath)
+ ByValInfo(unsigned bytecodeIndex, CodeLocationJump notIndexJump, CodeLocationJump badTypeJump, CodeLocationLabel exceptionHandler, JITArrayMode arrayMode, ArrayProfile* arrayProfile, int16_t badTypeJumpToDone, int16_t badTypeJumpToNextHotPath, int16_t returnAddressToSlowPath)
: bytecodeIndex(bytecodeIndex)
, notIndexJump(notIndexJump)
, badTypeJump(badTypeJump)
+ , exceptionHandler(exceptionHandler)
, arrayMode(arrayMode)
, arrayProfile(arrayProfile)
, badTypeJumpToDone(badTypeJumpToDone)
@@ -226,6 +227,7 @@
unsigned bytecodeIndex;
CodeLocationJump notIndexJump;
CodeLocationJump badTypeJump;
+ CodeLocationLabel exceptionHandler;
JITArrayMode arrayMode; // The array mode that was baked into the inline JIT code.
ArrayProfile* arrayProfile;
int16_t badTypeJumpToDone;
Modified: trunk/Source/_javascript_Core/jit/JIT.cpp (204361 => 204362)
--- trunk/Source/_javascript_Core/jit/JIT.cpp 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/Source/_javascript_Core/jit/JIT.cpp 2016-08-10 23:45:05 UTC (rev 204362)
@@ -725,27 +725,33 @@
for (unsigned i = m_putByIds.size(); i--;)
m_putByIds[i].finalize(patchBuffer);
- for (const auto& byValCompilationInfo : m_byValCompilationInfo) {
- PatchableJump patchableNotIndexJump = byValCompilationInfo.notIndexJump;
- CodeLocationJump notIndexJump = CodeLocationJump();
- if (Jump(patchableNotIndexJump).isSet())
- notIndexJump = CodeLocationJump(patchBuffer.locationOf(patchableNotIndexJump));
- CodeLocationJump badTypeJump = CodeLocationJump(patchBuffer.locationOf(byValCompilationInfo.badTypeJump));
- CodeLocationLabel doneTarget = patchBuffer.locationOf(byValCompilationInfo.doneTarget);
- CodeLocationLabel nextHotPathTarget = patchBuffer.locationOf(byValCompilationInfo.nextHotPathTarget);
- CodeLocationLabel slowPathTarget = patchBuffer.locationOf(byValCompilationInfo.slowPathTarget);
- CodeLocationCall returnAddress = patchBuffer.locationOf(byValCompilationInfo.returnAddress);
+ if (m_byValCompilationInfo.size()) {
+ CodeLocationLabel exceptionHandler = patchBuffer.locationOf(m_exceptionHandler);
- *byValCompilationInfo.byValInfo = ByValInfo(
- byValCompilationInfo.bytecodeIndex,
- notIndexJump,
- badTypeJump,
- byValCompilationInfo.arrayMode,
- byValCompilationInfo.arrayProfile,
- differenceBetweenCodePtr(badTypeJump, doneTarget),
- differenceBetweenCodePtr(badTypeJump, nextHotPathTarget),
- differenceBetweenCodePtr(returnAddress, slowPathTarget));
+ for (const auto& byValCompilationInfo : m_byValCompilationInfo) {
+ PatchableJump patchableNotIndexJump = byValCompilationInfo.notIndexJump;
+ CodeLocationJump notIndexJump = CodeLocationJump();
+ if (Jump(patchableNotIndexJump).isSet())
+ notIndexJump = CodeLocationJump(patchBuffer.locationOf(patchableNotIndexJump));
+ CodeLocationJump badTypeJump = CodeLocationJump(patchBuffer.locationOf(byValCompilationInfo.badTypeJump));
+ CodeLocationLabel doneTarget = patchBuffer.locationOf(byValCompilationInfo.doneTarget);
+ CodeLocationLabel nextHotPathTarget = patchBuffer.locationOf(byValCompilationInfo.nextHotPathTarget);
+ CodeLocationLabel slowPathTarget = patchBuffer.locationOf(byValCompilationInfo.slowPathTarget);
+ CodeLocationCall returnAddress = patchBuffer.locationOf(byValCompilationInfo.returnAddress);
+
+ *byValCompilationInfo.byValInfo = ByValInfo(
+ byValCompilationInfo.bytecodeIndex,
+ notIndexJump,
+ badTypeJump,
+ exceptionHandler,
+ byValCompilationInfo.arrayMode,
+ byValCompilationInfo.arrayProfile,
+ differenceBetweenCodePtr(badTypeJump, doneTarget),
+ differenceBetweenCodePtr(badTypeJump, nextHotPathTarget),
+ differenceBetweenCodePtr(returnAddress, slowPathTarget));
+ }
}
+
for (unsigned i = 0; i < m_callCompilationInfo.size(); ++i) {
CallCompilationInfo& compilationInfo = m_callCompilationInfo[i];
CallLinkInfo& info = *compilationInfo.callLinkInfo;
@@ -825,7 +831,8 @@
jumpToExceptionHandler();
}
- if (!m_exceptionChecks.empty()) {
+ if (!m_exceptionChecks.empty() || m_byValCompilationInfo.size()) {
+ m_exceptionHandler = label();
m_exceptionChecks.link(this);
copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
Modified: trunk/Source/_javascript_Core/jit/JIT.h (204361 => 204362)
--- trunk/Source/_javascript_Core/jit/JIT.h 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/Source/_javascript_Core/jit/JIT.h 2016-08-10 23:45:05 UTC (rev 204362)
@@ -943,6 +943,7 @@
JumpList m_exceptionChecks;
JumpList m_exceptionChecksWithCallFrameRollback;
+ Label m_exceptionHandler;
unsigned m_getByIdIndex;
unsigned m_putByIdIndex;
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (204361 => 204362)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2016-08-10 23:30:24 UTC (rev 204361)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2016-08-10 23:45:05 UTC (rev 204362)
@@ -1331,6 +1331,8 @@
patchBuffer.link(slowCases, CodeLocationLabel(MacroAssemblerCodePtr::createFromExecutableAddress(returnAddress.value())).labelAtOffset(byValInfo->returnAddressToSlowPath));
patchBuffer.link(fastDoneCase, byValInfo->badTypeJump.labelAtOffset(byValInfo->badTypeJumpToDone));
patchBuffer.link(slowDoneCase, byValInfo->badTypeJump.labelAtOffset(byValInfo->badTypeJumpToNextHotPath));
+ if (!m_exceptionChecks.empty())
+ patchBuffer.link(m_exceptionChecks, byValInfo->exceptionHandler);
for (const auto& callSite : m_calls) {
if (callSite.to)
@@ -1419,6 +1421,9 @@
LinkBuffer patchBuffer(*m_vm, *this, m_codeBlock);
patchBuffer.link(slowCases, CodeLocationLabel(MacroAssemblerCodePtr::createFromExecutableAddress(returnAddress.value())).labelAtOffset(byValInfo->returnAddressToSlowPath));
patchBuffer.link(doneCases, byValInfo->badTypeJump.labelAtOffset(byValInfo->badTypeJumpToDone));
+ if (!m_exceptionChecks.empty())
+ patchBuffer.link(m_exceptionChecks, byValInfo->exceptionHandler);
+
for (const auto& callSite : m_calls) {
if (callSite.to)
patchBuffer.link(callSite.from, FunctionPtr(callSite.to));
